[dpdk-dev] [RFC] mem: poison memory when freed

[dpdk-dev] [RFC] mem: poison memory when freed

[Stephen Hemminger]

DPDK malloc library allows broken programs to work because
the semantics of zmalloc and malloc are the same.

This patch changes to a more secure model which will catch
(and crash) programs that reuse memory already freed.

This supersedes earlier changes to zero memory on free and
avoid zeroing memory in zmalloc.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_eal/common/malloc_elem.c | 5 ++++-
 lib/librte_eal/common/rte_malloc.c  | 6 +++++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/lib/librte_eal/common/malloc_elem.c b/lib/librte_eal/common/malloc_elem.c
index efcb82677198..62cc0b385c0c 100644
--- a/lib/librte_eal/common/malloc_elem.c
+++ b/lib/librte_eal/common/malloc_elem.c
@@ -23,6 +23,8 @@
 #include "malloc_elem.h"
 #include "malloc_heap.h"
 
+#define MALLOC_POISON	       0x6b		     /**< Free memory. */
+
 size_t
 malloc_elem_find_max_iova_contig(struct malloc_elem *elem, size_t align)
 {
@@ -531,7 +533,8 @@ malloc_elem_free(struct malloc_elem *elem)
 	/* decrease heap's count of allocated elements */
 	elem->heap->alloc_count--;
 
-	memset(ptr, 0, data_len);
+	/* poison memory */
+	memset(ptr, MALLOC_POISON, data_len);
 
 	return elem;
 }
diff --git a/lib/librte_eal/common/rte_malloc.c b/lib/librte_eal/common/rte_malloc.c
index b51a6d111bde..b33c936fd491 100644
--- a/lib/librte_eal/common/rte_malloc.c
+++ b/lib/librte_eal/common/rte_malloc.c
@@ -70,7 +70,11 @@ rte_malloc(const char *type, size_t size, unsigned align)
 void *
 rte_zmalloc_socket(const char *type, size_t size, unsigned align, int socket)
 {
-	return rte_malloc_socket(type, size, align, socket);
+	void *ptr = rte_malloc_socket(type, size, align, socket);
+
+	if (ptr != NULL)
+		memset(ptr, 0, size);
+	return ptr;
 }
 
 /*
-- 
2.18.0

Re: [dpdk-dev] [RFC] mem: poison memory when freed

[Burakov, Anatoly]

On 18-Jul-18 10:44 PM, Stephen Hemminger wrote:
> DPDK malloc library allows broken programs to work because
> the semantics of zmalloc and malloc are the same.
> 
> This patch changes to a more secure model which will catch
> (and crash) programs that reuse memory already freed.
> 
> This supersedes earlier changes to zero memory on free and
> avoid zeroing memory in zmalloc.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---

I would be a bit wary of introducing this change without prior 
announcement. Currently, rte_malloc'd memory is semantically identical 
to zmalloc'd memory, which means there may be code that relies on this 
behavior, even though it's technically incorrect.

How about a deprecation notice, and do this in 18.11?

-- 
Thanks,
Anatoly

Re: [dpdk-dev] [RFC] mem: poison memory when freed

[Bruce Richardson]

On Thu, Jul 19, 2018 at 10:03:55AM +0100, Burakov, Anatoly wrote:
> On 18-Jul-18 10:44 PM, Stephen Hemminger wrote:
> > DPDK malloc library allows broken programs to work because
> > the semantics of zmalloc and malloc are the same.
> > 
> > This patch changes to a more secure model which will catch
> > (and crash) programs that reuse memory already freed.
> > 
> > This supersedes earlier changes to zero memory on free and
> > avoid zeroing memory in zmalloc.
> > 
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> 
> I would be a bit wary of introducing this change without prior announcement.
> Currently, rte_malloc'd memory is semantically identical to zmalloc'd
> memory, which means there may be code that relies on this behavior, even
> though it's technically incorrect.
> 
> How about a deprecation notice, and do this in 18.11?
> 
The question I have is, how much is this going to slow down calls to
zmalloc, particularly those on application startup? The advantage of the
previous scheme was that for applications with large memory footprints, we
were able to give them their allocations quickly, and had no zeroing
overhead unless blocks of memory were continually being allocated and
freed. With this, the startup time of some apps could be badly impacted.
Perhaps we should make this a runtime debug option.

/Bruce

Re: [dpdk-dev] [RFC] mem: poison memory when freed

[Burakov, Anatoly]

On 18-Jul-18 10:44 PM, Stephen Hemminger wrote:
> DPDK malloc library allows broken programs to work because
> the semantics of zmalloc and malloc are the same.
> 
> This patch changes to a more secure model which will catch
> (and crash) programs that reuse memory already freed.
> 
> This supersedes earlier changes to zero memory on free and
> avoid zeroing memory in zmalloc.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>   lib/librte_eal/common/malloc_elem.c | 5 ++++-
>   lib/librte_eal/common/rte_malloc.c  | 6 +++++-
>   2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_eal/common/malloc_elem.c b/lib/librte_eal/common/malloc_elem.c
> index efcb82677198..62cc0b385c0c 100644
> --- a/lib/librte_eal/common/malloc_elem.c
> +++ b/lib/librte_eal/common/malloc_elem.c
> @@ -23,6 +23,8 @@
>   #include "malloc_elem.h"
>   #include "malloc_heap.h"
>   
> +#define MALLOC_POISON	       0x6b		     /**< Free memory. */
> +
>   size_t
>   malloc_elem_find_max_iova_contig(struct malloc_elem *elem, size_t align)
>   {
> @@ -531,7 +533,8 @@ malloc_elem_free(struct malloc_elem *elem)
>   	/* decrease heap's count of allocated elements */
>   	elem->heap->alloc_count--;
>   
> -	memset(ptr, 0, data_len);
> +	/* poison memory */
> +	memset(ptr, MALLOC_POISON, data_len);

Looking at the merits of this patch, this is not quite enough. We also 
join adjacent malloc elements and erase their headers, so you will also 
need to adjust memsets in malloc_elem_join_adjacent_free().

>   
>   	return elem;
>   }
> diff --git a/lib/librte_eal/common/rte_malloc.c b/lib/librte_eal/common/rte_malloc.c
> index b51a6d111bde..b33c936fd491 100644
> --- a/lib/librte_eal/common/rte_malloc.c
> +++ b/lib/librte_eal/common/rte_malloc.c
> @@ -70,7 +70,11 @@ rte_malloc(const char *type, size_t size, unsigned align)
>   void *
>   rte_zmalloc_socket(const char *type, size_t size, unsigned align, int socket)
>   {
> -	return rte_malloc_socket(type, size, align, socket);
> +	void *ptr = rte_malloc_socket(type, size, align, socket);
> +
> +	if (ptr != NULL)
> +		memset(ptr, 0, size);
> +	return ptr;
>   }
>   
>   /*
> 


-- 
Thanks,
Anatoly