[PATCH] eal/malloc: fix malloc cookie check

[PATCH] eal/malloc: fix malloc cookie check

[Xueming Li]

From: xuemingl <xuemingl@mellanox.com>

DPDK uses it's own memory management, few regular memory profiler tool
support DPDK now. Malloc cookie check provides limited memory corruption
check, better than nothing. This patch fixes the following:
1. Replace broken generated configuration macro RTE_LIBRTE_MALLOC_DEBUG
with RTE_MALLOC_DEBUG
2. Fix malloc size calculation when RTE_MALLOC_DEBUG cookie check
enabled.

>From real test, it IS very helpful to detect memory corruption.

A better designed DPDK application should quit nicely with resoure
releasing so that all allocated memory could be checked against cookie.

Fixes: af75078 ("first public release")
Cc: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>

Signed-off-by: Xueming Li <xuemingl@mellanox.com>
---
 lib/librte_eal/common/malloc_elem.c |  8 ++++----
 lib/librte_eal/common/malloc_elem.h |  4 ++--
 test/test/test_malloc.c             | 10 +++++-----
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/lib/librte_eal/common/malloc_elem.c b/lib/librte_eal/common/malloc_elem.c
index 150769057..889dffd21 100644
--- a/lib/librte_eal/common/malloc_elem.c
+++ b/lib/librte_eal/common/malloc_elem.c
@@ -275,14 +275,14 @@ malloc_elem_free(struct malloc_elem *elem)
 		return -1;
 
 	rte_spinlock_lock(&(elem->heap->lock));
-	size_t sz = elem->size - sizeof(*elem);
+	size_t sz = elem->size - sizeof(*elem) - MALLOC_ELEM_TRAILER_LEN;
 	uint8_t *ptr = (uint8_t *)&elem[1];
 	struct malloc_elem *next = RTE_PTR_ADD(elem, elem->size);
 	if (next->state == ELEM_FREE){
 		/* remove from free list, join to this one */
 		elem_free_list_remove(next);
 		join_elem(elem, next);
-		sz += sizeof(*elem);
+		sz += (sizeof(*elem) + MALLOC_ELEM_TRAILER_LEN);
 	}
 
 	/* check if previous element is free, if so join with it and return,
@@ -291,8 +291,8 @@ malloc_elem_free(struct malloc_elem *elem)
 	if (elem->prev != NULL && elem->prev->state == ELEM_FREE) {
 		elem_free_list_remove(elem->prev);
 		join_elem(elem->prev, elem);
-		sz += sizeof(*elem);
-		ptr -= sizeof(*elem);
+		sz += (sizeof(*elem) + MALLOC_ELEM_TRAILER_LEN);
+		ptr -= (sizeof(*elem) + MALLOC_ELEM_TRAILER_LEN);
 		elem = elem->prev;
 	}
 	malloc_elem_free_list_insert(elem);
diff --git a/lib/librte_eal/common/malloc_elem.h b/lib/librte_eal/common/malloc_elem.h
index f04b2d1e4..ce39129d9 100644
--- a/lib/librte_eal/common/malloc_elem.h
+++ b/lib/librte_eal/common/malloc_elem.h
@@ -53,13 +53,13 @@ struct malloc_elem {
 	volatile enum elem_state state;
 	uint32_t pad;
 	size_t size;
-#ifdef RTE_LIBRTE_MALLOC_DEBUG
+#ifdef RTE_MALLOC_DEBUG
 	uint64_t header_cookie;         /* Cookie marking start of data */
 	                                /* trailer cookie at start + size */
 #endif
 } __rte_cache_aligned;
 
-#ifndef RTE_LIBRTE_MALLOC_DEBUG
+#ifndef RTE_MALLOC_DEBUG
 static const unsigned MALLOC_ELEM_TRAILER_LEN = 0;
 
 /* dummy function - just check if pointer is non-null */
diff --git a/test/test/test_malloc.c b/test/test/test_malloc.c
index 013fd4407..5558acda4 100644
--- a/test/test/test_malloc.c
+++ b/test/test/test_malloc.c
@@ -108,7 +108,7 @@ test_align_overlap_per_lcore(__attribute__((unused)) void *arg)
 		}
 		for(j = 0; j < 1000 ; j++) {
 			if( *(char *)p1 != 0) {
-				printf("rte_zmalloc didn't zero"
+				printf("rte_zmalloc didn't zero "
 				       "the allocated memory\n");
 				ret = -1;
 			}
@@ -180,7 +180,7 @@ test_reordered_free_per_lcore(__attribute__((unused)) void *arg)
 		}
 		for(j = 0; j < 1000 ; j++) {
 			if( *(char *)p1 != 0) {
-				printf("rte_zmalloc didn't zero"
+				printf("rte_zmalloc didn't zero "
 				       "the allocated memory\n");
 				ret = -1;
 			}
@@ -293,7 +293,7 @@ test_multi_alloc_statistics(void)
 	struct rte_malloc_socket_stats pre_stats, post_stats ,first_stats, second_stats;
 	size_t size = 2048;
 	int align = 1024;
-#ifndef RTE_LIBRTE_MALLOC_DEBUG
+#ifndef RTE_MALLOC_DEBUG
 	int trailer_size = 0;
 #else
 	int trailer_size = RTE_CACHE_LINE_SIZE;
@@ -623,7 +623,7 @@ test_rte_malloc_validate(void)
 	const size_t request_size = 1024;
 	size_t allocated_size;
 	char *data_ptr = rte_malloc(NULL, request_size, RTE_CACHE_LINE_SIZE);
-#ifdef RTE_LIBRTE_MALLOC_DEBUG
+#ifdef RTE_MALLOC_DEBUG
 	int retval;
 	char *over_write_vals = NULL;
 #endif
@@ -645,7 +645,7 @@ test_rte_malloc_validate(void)
 	if (allocated_size < request_size)
 		err_return();
 
-#ifdef RTE_LIBRTE_MALLOC_DEBUG
+#ifdef RTE_MALLOC_DEBUG
 
 	/****** change the header to be bad */
 	char save_buf[64];
-- 
2.13.3

Re: [PATCH] eal/malloc: fix malloc cookie check

[Sergio Gonzalez Monroy]

Hi Xueming,

I think I have understood what you are doing and why the change is 
required, but
I believe we can improve the commit message.

It might be even a good idea to split the patches:
1. RTE_MALLOC_DEBUG fix
2. malloc_elem_free fix


On 23/08/2017 03:29, Xueming Li wrote:
> From: xuemingl <xuemingl@mellanox.com>

'From: ..' not needed.

> DPDK uses it's own memory management, few regular memory profiler tool
> support DPDK now. Malloc cookie check provides limited memory corruption
> check, better than nothing. This patch fixes the following:
> 1. Replace broken generated configuration macro RTE_LIBRTE_MALLOC_DEBUG
> with RTE_MALLOC_DEBUG
> 2. Fix malloc size calculation when RTE_MALLOC_DEBUG cookie check
> enabled.

For the 2nd change, you should mention what is the problem.
The issue is that malloc_elem_free is clearing (setting to 0) the 
trailer cookie when RTE_MALLOC_DEBUG is enabled.

You could also add that you would trigger this issue if the elem being 
freed does not join the next elem.

Thanks,
Sergio

>  From real test, it IS very helpful to detect memory corruption.
>
> A better designed DPDK application should quit nicely with resoure
> releasing so that all allocated memory could be checked against cookie.
>
> Fixes: af75078 ("first public release")
> Cc: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
>
> Signed-off-by: Xueming Li <xuemingl@mellanox.com>
> ---
>   lib/librte_eal/common/malloc_elem.c |  8 ++++----
>   lib/librte_eal/common/malloc_elem.h |  4 ++--
>   test/test/test_malloc.c             | 10 +++++-----
>   3 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/lib/librte_eal/common/malloc_elem.c b/lib/librte_eal/common/malloc_elem.c
> index 150769057..889dffd21 100644
> --- a/lib/librte_eal/common/malloc_elem.c
> +++ b/lib/librte_eal/common/malloc_elem.c
> @@ -275,14 +275,14 @@ malloc_elem_free(struct malloc_elem *elem)
>   		return -1;
>   
>   	rte_spinlock_lock(&(elem->heap->lock));
> -	size_t sz = elem->size - sizeof(*elem);
> +	size_t sz = elem->size - sizeof(*elem) - MALLOC_ELEM_TRAILER_LEN;
>   	uint8_t *ptr = (uint8_t *)&elem[1];
>   	struct malloc_elem *next = RTE_PTR_ADD(elem, elem->size);
>   	if (next->state == ELEM_FREE){
>   		/* remove from free list, join to this one */
>   		elem_free_list_remove(next);
>   		join_elem(elem, next);
> -		sz += sizeof(*elem);
> +		sz += (sizeof(*elem) + MALLOC_ELEM_TRAILER_LEN);
>   	}
>   
>   	/* check if previous element is free, if so join with it and return,
> @@ -291,8 +291,8 @@ malloc_elem_free(struct malloc_elem *elem)
>   	if (elem->prev != NULL && elem->prev->state == ELEM_FREE) {
>   		elem_free_list_remove(elem->prev);
>   		join_elem(elem->prev, elem);
> -		sz += sizeof(*elem);
> -		ptr -= sizeof(*elem);
> +		sz += (sizeof(*elem) + MALLOC_ELEM_TRAILER_LEN);
> +		ptr -= (sizeof(*elem) + MALLOC_ELEM_TRAILER_LEN);
>   		elem = elem->prev;
>   	}
>   	malloc_elem_free_list_insert(elem);
> diff --git a/lib/librte_eal/common/malloc_elem.h b/lib/librte_eal/common/malloc_elem.h
> index f04b2d1e4..ce39129d9 100644
> --- a/lib/librte_eal/common/malloc_elem.h
> +++ b/lib/librte_eal/common/malloc_elem.h
> @@ -53,13 +53,13 @@ struct malloc_elem {
>   	volatile enum elem_state state;
>   	uint32_t pad;
>   	size_t size;
> -#ifdef RTE_LIBRTE_MALLOC_DEBUG
> +#ifdef RTE_MALLOC_DEBUG
>   	uint64_t header_cookie;         /* Cookie marking start of data */
>   	                                /* trailer cookie at start + size */
>   #endif
>   } __rte_cache_aligned;
>   
> -#ifndef RTE_LIBRTE_MALLOC_DEBUG
> +#ifndef RTE_MALLOC_DEBUG
>   static const unsigned MALLOC_ELEM_TRAILER_LEN = 0;
>   
>   /* dummy function - just check if pointer is non-null */
> diff --git a/test/test/test_malloc.c b/test/test/test_malloc.c
> index 013fd4407..5558acda4 100644
> --- a/test/test/test_malloc.c
> +++ b/test/test/test_malloc.c
> @@ -108,7 +108,7 @@ test_align_overlap_per_lcore(__attribute__((unused)) void *arg)
>   		}
>   		for(j = 0; j < 1000 ; j++) {
>   			if( *(char *)p1 != 0) {
> -				printf("rte_zmalloc didn't zero"
> +				printf("rte_zmalloc didn't zero "
>   				       "the allocated memory\n");
>   				ret = -1;
>   			}
> @@ -180,7 +180,7 @@ test_reordered_free_per_lcore(__attribute__((unused)) void *arg)
>   		}
>   		for(j = 0; j < 1000 ; j++) {
>   			if( *(char *)p1 != 0) {
> -				printf("rte_zmalloc didn't zero"
> +				printf("rte_zmalloc didn't zero "
>   				       "the allocated memory\n");
>   				ret = -1;
>   			}
> @@ -293,7 +293,7 @@ test_multi_alloc_statistics(void)
>   	struct rte_malloc_socket_stats pre_stats, post_stats ,first_stats, second_stats;
>   	size_t size = 2048;
>   	int align = 1024;
> -#ifndef RTE_LIBRTE_MALLOC_DEBUG
> +#ifndef RTE_MALLOC_DEBUG
>   	int trailer_size = 0;
>   #else
>   	int trailer_size = RTE_CACHE_LINE_SIZE;
> @@ -623,7 +623,7 @@ test_rte_malloc_validate(void)
>   	const size_t request_size = 1024;
>   	size_t allocated_size;
>   	char *data_ptr = rte_malloc(NULL, request_size, RTE_CACHE_LINE_SIZE);
> -#ifdef RTE_LIBRTE_MALLOC_DEBUG
> +#ifdef RTE_MALLOC_DEBUG
>   	int retval;
>   	char *over_write_vals = NULL;
>   #endif
> @@ -645,7 +645,7 @@ test_rte_malloc_validate(void)
>   	if (allocated_size < request_size)
>   		err_return();
>   
> -#ifdef RTE_LIBRTE_MALLOC_DEBUG
> +#ifdef RTE_MALLOC_DEBUG
>   
>   	/****** change the header to be bad */
>   	char save_buf[64];

[PATCH v2 1/2] eal/malloc: fix RTE malloc debug macro

[Xueming Li]

This patch replaces broken macro RTE_LIBRTE_MALLOC_DEBUG with
RTE_MALLOC_DEBUG.

Fixes: af75078fece3 ("first public release")

Cc: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
Signed-off-by: Xueming Li <xuemingl@mellanox.com>
---
 lib/librte_eal/common/malloc_elem.h |  4 ++--
 test/test/test_malloc.c             | 10 +++++-----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/librte_eal/common/malloc_elem.h b/lib/librte_eal/common/malloc_elem.h
index f04b2d1e4..ce39129d9 100644
--- a/lib/librte_eal/common/malloc_elem.h
+++ b/lib/librte_eal/common/malloc_elem.h
@@ -53,13 +53,13 @@ struct malloc_elem {
 	volatile enum elem_state state;
 	uint32_t pad;
 	size_t size;
-#ifdef RTE_LIBRTE_MALLOC_DEBUG
+#ifdef RTE_MALLOC_DEBUG
 	uint64_t header_cookie;         /* Cookie marking start of data */
 	                                /* trailer cookie at start + size */
 #endif
 } __rte_cache_aligned;
 
-#ifndef RTE_LIBRTE_MALLOC_DEBUG
+#ifndef RTE_MALLOC_DEBUG
 static const unsigned MALLOC_ELEM_TRAILER_LEN = 0;
 
 /* dummy function - just check if pointer is non-null */
diff --git a/test/test/test_malloc.c b/test/test/test_malloc.c
index 013fd4407..5558acda4 100644
--- a/test/test/test_malloc.c
+++ b/test/test/test_malloc.c
@@ -108,7 +108,7 @@ test_align_overlap_per_lcore(__attribute__((unused)) void *arg)
 		}
 		for(j = 0; j < 1000 ; j++) {
 			if( *(char *)p1 != 0) {
-				printf("rte_zmalloc didn't zero"
+				printf("rte_zmalloc didn't zero "
 				       "the allocated memory\n");
 				ret = -1;
 			}
@@ -180,7 +180,7 @@ test_reordered_free_per_lcore(__attribute__((unused)) void *arg)
 		}
 		for(j = 0; j < 1000 ; j++) {
 			if( *(char *)p1 != 0) {
-				printf("rte_zmalloc didn't zero"
+				printf("rte_zmalloc didn't zero "
 				       "the allocated memory\n");
 				ret = -1;
 			}
@@ -293,7 +293,7 @@ test_multi_alloc_statistics(void)
 	struct rte_malloc_socket_stats pre_stats, post_stats ,first_stats, second_stats;
 	size_t size = 2048;
 	int align = 1024;
-#ifndef RTE_LIBRTE_MALLOC_DEBUG
+#ifndef RTE_MALLOC_DEBUG
 	int trailer_size = 0;
 #else
 	int trailer_size = RTE_CACHE_LINE_SIZE;
@@ -623,7 +623,7 @@ test_rte_malloc_validate(void)
 	const size_t request_size = 1024;
 	size_t allocated_size;
 	char *data_ptr = rte_malloc(NULL, request_size, RTE_CACHE_LINE_SIZE);
-#ifdef RTE_LIBRTE_MALLOC_DEBUG
+#ifdef RTE_MALLOC_DEBUG
 	int retval;
 	char *over_write_vals = NULL;
 #endif
@@ -645,7 +645,7 @@ test_rte_malloc_validate(void)
 	if (allocated_size < request_size)
 		err_return();
 
-#ifdef RTE_LIBRTE_MALLOC_DEBUG
+#ifdef RTE_MALLOC_DEBUG
 
 	/****** change the header to be bad */
 	char save_buf[64];
-- 
2.13.3

[PATCH v2 2/2] eal/malloc: fix RTE malloc element free

[Xueming Li]

malloc_elem_free() is clearing(setting to 0) the trailer cookie when
RTE_MALLOC_DEBUG is enabled. In case of joining free neighbor element,
part of joined memory is not getting cleared due to missing the length
of trailer cookie in the middle.

This patch fixes calculation of free memory length to be cleared in
malloc_elem_free() by including trailer cookie.

Fixes: af75078fece3 ("first public release")

Cc: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
Signed-off-by: Xueming Li <xuemingl@mellanox.com>
---
 lib/librte_eal/common/malloc_elem.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/librte_eal/common/malloc_elem.c b/lib/librte_eal/common/malloc_elem.c
index 150769057..889dffd21 100644
--- a/lib/librte_eal/common/malloc_elem.c
+++ b/lib/librte_eal/common/malloc_elem.c
@@ -275,14 +275,14 @@ malloc_elem_free(struct malloc_elem *elem)
 		return -1;
 
 	rte_spinlock_lock(&(elem->heap->lock));
-	size_t sz = elem->size - sizeof(*elem);
+	size_t sz = elem->size - sizeof(*elem) - MALLOC_ELEM_TRAILER_LEN;
 	uint8_t *ptr = (uint8_t *)&elem[1];
 	struct malloc_elem *next = RTE_PTR_ADD(elem, elem->size);
 	if (next->state == ELEM_FREE){
 		/* remove from free list, join to this one */
 		elem_free_list_remove(next);
 		join_elem(elem, next);
-		sz += sizeof(*elem);
+		sz += (sizeof(*elem) + MALLOC_ELEM_TRAILER_LEN);
 	}
 
 	/* check if previous element is free, if so join with it and return,
@@ -291,8 +291,8 @@ malloc_elem_free(struct malloc_elem *elem)
 	if (elem->prev != NULL && elem->prev->state == ELEM_FREE) {
 		elem_free_list_remove(elem->prev);
 		join_elem(elem->prev, elem);
-		sz += sizeof(*elem);
-		ptr -= sizeof(*elem);
+		sz += (sizeof(*elem) + MALLOC_ELEM_TRAILER_LEN);
+		ptr -= (sizeof(*elem) + MALLOC_ELEM_TRAILER_LEN);
 		elem = elem->prev;
 	}
 	malloc_elem_free_list_insert(elem);
-- 
2.13.3

Re: [PATCH v2 1/2] eal/malloc: fix RTE malloc debug macro

[Stephen Hemminger]

On Fri,  8 Sep 2017 22:50:54 +0800
Xueming Li <xuemingl@mellanox.com> wrote:

> -				printf("rte_zmalloc didn't zero"
> +				printf("rte_zmalloc didn't zero "
>  				       "the allocated memory\n");

Please don't break error messages onto two lines.
It makes it harder to use tools like grep and google searches to find where message is located.

Checkpatch and other tools make an exception for long lines in quoted strings.

Re: [PATCH v2 1/2] eal/malloc: fix RTE malloc debug macro

[Xueming(Steven) Li]

Thanks, updated in v3.

> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Saturday, September 9, 2017 12:04 AM
> To: Xueming(Steven) Li <xuemingl@mellanox.com>
> Cc: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>;
> dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 1/2] eal/malloc: fix RTE malloc debug macro
> 
> On Fri,  8 Sep 2017 22:50:54 +0800
> Xueming Li <xuemingl@mellanox.com> wrote:
> 
> > -				printf("rte_zmalloc didn't zero"
> > +				printf("rte_zmalloc didn't zero "
> >  				       "the allocated memory\n");
> 
> Please don't break error messages onto two lines.
> It makes it harder to use tools like grep and google searches to find where
> message is located.
> 
> Checkpatch and other tools make an exception for long lines in quoted strings.

[PATCH v3 1/2] eal/malloc: fix RTE malloc debug macro

[Xueming Li]

This patch replaces broken macro RTE_LIBRTE_MALLOC_DEBUG with
RTE_MALLOC_DEBUG.

Fixes: af75078fece3 ("first public release")

Cc: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
Signed-off-by: Xueming Li <xuemingl@mellanox.com>
---
 lib/librte_eal/common/malloc_elem.h |  4 ++--
 test/test/test_malloc.c             | 12 +++++-------
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/lib/librte_eal/common/malloc_elem.h b/lib/librte_eal/common/malloc_elem.h
index f04b2d1e4..ce39129d9 100644
--- a/lib/librte_eal/common/malloc_elem.h
+++ b/lib/librte_eal/common/malloc_elem.h
@@ -53,13 +53,13 @@ struct malloc_elem {
 	volatile enum elem_state state;
 	uint32_t pad;
 	size_t size;
-#ifdef RTE_LIBRTE_MALLOC_DEBUG
+#ifdef RTE_MALLOC_DEBUG
 	uint64_t header_cookie;         /* Cookie marking start of data */
 	                                /* trailer cookie at start + size */
 #endif
 } __rte_cache_aligned;
 
-#ifndef RTE_LIBRTE_MALLOC_DEBUG
+#ifndef RTE_MALLOC_DEBUG
 static const unsigned MALLOC_ELEM_TRAILER_LEN = 0;
 
 /* dummy function - just check if pointer is non-null */
diff --git a/test/test/test_malloc.c b/test/test/test_malloc.c
index 013fd4407..cee6469d8 100644
--- a/test/test/test_malloc.c
+++ b/test/test/test_malloc.c
@@ -108,8 +108,7 @@ test_align_overlap_per_lcore(__attribute__((unused)) void *arg)
 		}
 		for(j = 0; j < 1000 ; j++) {
 			if( *(char *)p1 != 0) {
-				printf("rte_zmalloc didn't zero"
-				       "the allocated memory\n");
+				printf("rte_zmalloc didn't zero the allocated memory\n");
 				ret = -1;
 			}
 		}
@@ -180,8 +179,7 @@ test_reordered_free_per_lcore(__attribute__((unused)) void *arg)
 		}
 		for(j = 0; j < 1000 ; j++) {
 			if( *(char *)p1 != 0) {
-				printf("rte_zmalloc didn't zero"
-				       "the allocated memory\n");
+				printf("rte_zmalloc didn't zero the allocated memory\n");
 				ret = -1;
 			}
 		}
@@ -293,7 +291,7 @@ test_multi_alloc_statistics(void)
 	struct rte_malloc_socket_stats pre_stats, post_stats ,first_stats, second_stats;
 	size_t size = 2048;
 	int align = 1024;
-#ifndef RTE_LIBRTE_MALLOC_DEBUG
+#ifndef RTE_MALLOC_DEBUG
 	int trailer_size = 0;
 #else
 	int trailer_size = RTE_CACHE_LINE_SIZE;
@@ -623,7 +621,7 @@ test_rte_malloc_validate(void)
 	const size_t request_size = 1024;
 	size_t allocated_size;
 	char *data_ptr = rte_malloc(NULL, request_size, RTE_CACHE_LINE_SIZE);
-#ifdef RTE_LIBRTE_MALLOC_DEBUG
+#ifdef RTE_MALLOC_DEBUG
 	int retval;
 	char *over_write_vals = NULL;
 #endif
@@ -645,7 +643,7 @@ test_rte_malloc_validate(void)
 	if (allocated_size < request_size)
 		err_return();
 
-#ifdef RTE_LIBRTE_MALLOC_DEBUG
+#ifdef RTE_MALLOC_DEBUG
 
 	/****** change the header to be bad */
 	char save_buf[64];
-- 
2.13.3

[PATCH v3 2/2] eal/malloc: fix RTE malloc element free

[Xueming Li]

malloc_elem_free() is clearing(setting to 0) the trailer cookie when
RTE_MALLOC_DEBUG is enabled. In case of joining free neighbor element,
part of joined memory is not getting cleared due to missing the length
of trailer cookie in the middle.

This patch fixes calculation of free memory length to be cleared in
malloc_elem_free() by including trailer cookie.

Fixes: af75078fece3 ("first public release")

Cc: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
Signed-off-by: Xueming Li <xuemingl@mellanox.com>
---
 lib/librte_eal/common/malloc_elem.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/librte_eal/common/malloc_elem.c b/lib/librte_eal/common/malloc_elem.c
index 150769057..889dffd21 100644
--- a/lib/librte_eal/common/malloc_elem.c
+++ b/lib/librte_eal/common/malloc_elem.c
@@ -275,14 +275,14 @@ malloc_elem_free(struct malloc_elem *elem)
 		return -1;
 
 	rte_spinlock_lock(&(elem->heap->lock));
-	size_t sz = elem->size - sizeof(*elem);
+	size_t sz = elem->size - sizeof(*elem) - MALLOC_ELEM_TRAILER_LEN;
 	uint8_t *ptr = (uint8_t *)&elem[1];
 	struct malloc_elem *next = RTE_PTR_ADD(elem, elem->size);
 	if (next->state == ELEM_FREE){
 		/* remove from free list, join to this one */
 		elem_free_list_remove(next);
 		join_elem(elem, next);
-		sz += sizeof(*elem);
+		sz += (sizeof(*elem) + MALLOC_ELEM_TRAILER_LEN);
 	}
 
 	/* check if previous element is free, if so join with it and return,
@@ -291,8 +291,8 @@ malloc_elem_free(struct malloc_elem *elem)
 	if (elem->prev != NULL && elem->prev->state == ELEM_FREE) {
 		elem_free_list_remove(elem->prev);
 		join_elem(elem->prev, elem);
-		sz += sizeof(*elem);
-		ptr -= sizeof(*elem);
+		sz += (sizeof(*elem) + MALLOC_ELEM_TRAILER_LEN);
+		ptr -= (sizeof(*elem) + MALLOC_ELEM_TRAILER_LEN);
 		elem = elem->prev;
 	}
 	malloc_elem_free_list_insert(elem);
-- 
2.13.3

Re: [PATCH v3 2/2] eal/malloc: fix RTE malloc element free

[Sergio Gonzalez Monroy]

On 09/09/2017 08:33, Xueming Li wrote:
> malloc_elem_free() is clearing(setting to 0) the trailer cookie when
> RTE_MALLOC_DEBUG is enabled. In case of joining free neighbor element,
> part of joined memory is not getting cleared due to missing the length
> of trailer cookie in the middle.
>
> This patch fixes calculation of free memory length to be cleared in
> malloc_elem_free() by including trailer cookie.
>
> Fixes: af75078fece3 ("first public release")
>
> Cc: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> Signed-off-by: Xueming Li <xuemingl@mellanox.com>
> ---

Acked-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>

Re: [PATCH v3 1/2] eal/malloc: fix RTE malloc debug macro

[Sergio Gonzalez Monroy]

On 09/09/2017 08:33, Xueming Li wrote:
> This patch replaces broken macro RTE_LIBRTE_MALLOC_DEBUG with
> RTE_MALLOC_DEBUG.
>
> Fixes: af75078fece3 ("first public release")
>
> Cc: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> Signed-off-by: Xueming Li <xuemingl@mellanox.com>
> ---

Acked-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>

Re: [PATCH v3 2/2] eal/malloc: fix RTE malloc element free

[Thomas Monjalon]

13/09/2017 14:06, Sergio Gonzalez Monroy:
> On 09/09/2017 08:33, Xueming Li wrote:
> > malloc_elem_free() is clearing(setting to 0) the trailer cookie when
> > RTE_MALLOC_DEBUG is enabled. In case of joining free neighbor element,
> > part of joined memory is not getting cleared due to missing the length
> > of trailer cookie in the middle.
> >
> > This patch fixes calculation of free memory length to be cleared in
> > malloc_elem_free() by including trailer cookie.
> >
> > Fixes: af75078fece3 ("first public release")
> >
> > Cc: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> > Signed-off-by: Xueming Li <xuemingl@mellanox.com>
> 
> Acked-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>

Applied, thanks