[v5,02/11] eal: add new secure free function

Although internally rte_free does poison the buffer in most
cases, it is useful to have function that explicitly does
this to avoid any security issues.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eal/common/rte_malloc.c  | 30 ++++++++++++++++++++++++------
 lib/eal/include/rte_malloc.h | 18 ++++++++++++++++++
 lib/eal/version.map          |  3 +++
 3 files changed, 45 insertions(+), 6 deletions(-)
  

On 2025/2/12 1:35, Stephen Hemminger wrote:
> Although internally rte_free does poison the buffer in most
> cases, it is useful to have function that explicitly does
> this to avoid any security issues.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  lib/eal/common/rte_malloc.c  | 30 ++++++++++++++++++++++++------
>  lib/eal/include/rte_malloc.h | 18 ++++++++++++++++++
>  lib/eal/version.map          |  3 +++
>  3 files changed, 45 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/eal/common/rte_malloc.c b/lib/eal/common/rte_malloc.c
> index 3eed4d4be6..c9e0f4724f 100644
> --- a/lib/eal/common/rte_malloc.c
> +++ b/lib/eal/common/rte_malloc.c
> @@ -15,6 +15,7 @@
>  #include <rte_eal_memconfig.h>
>  #include <rte_common.h>
>  #include <rte_spinlock.h>
> +#include <rte_string_fns.h>
>  
>  #include <eal_trace_internal.h>
>  
> @@ -27,27 +28,44 @@
>  
>  
>  /* Free the memory space back to heap */
> -static void
> -mem_free(void *addr, const bool trace_ena)
> +static inline void
> +mem_free(void *addr, const bool trace_ena, bool zero)
>  {
> +	struct malloc_elem *elem;
> +
>  	if (trace_ena)
>  		rte_eal_trace_mem_free(addr);
>  
> -	if (addr == NULL) return;
> -	if (malloc_heap_free(malloc_elem_from_data(addr)) < 0)
> +	if (addr == NULL)
> +		return;
> +
> +	elem = malloc_elem_from_data(addr);
> +	if (zero) {
> +		size_t data_len = elem->size - MALLOC_ELEM_OVERHEAD;

this will make rte_malloc know the layout of malloc-elem.
Prefer to add extra paramter, e.g. malloc_heap_free(elem, bool zero)

> +
> +		rte_memset_sensitive(addr, 0, data_len);
> +	}
> +
> +	if (malloc_heap_free(elem) < 0)
>  		EAL_LOG(ERR, "Error: Invalid memory");
>  }
>  
>  void
>  rte_free(void *addr)
>  {
> -	mem_free(addr, true);
> +	mem_free(addr, true, false);
> +}
> +
> +void
> +rte_free_sensitive(void *addr)
> +{
> +	mem_free(addr, true, true);
>  }
>  
>  void
>  eal_free_no_trace(void *addr)
>  {
> -	mem_free(addr, false);
> +	mem_free(addr, false, false);
>  }
>  
>  static void *
> diff --git a/lib/eal/include/rte_malloc.h b/lib/eal/include/rte_malloc.h
> index c8836de67c..d472ebb7db 100644
> --- a/lib/eal/include/rte_malloc.h
> +++ b/lib/eal/include/rte_malloc.h
> @@ -51,6 +51,24 @@ struct rte_malloc_socket_stats {
>  void
>  rte_free(void *ptr);
>  
> +
> +/**
> + * Frees the memory space pointed to by the provided pointer
> + * and guarantees it will be zero'd before reuse.
> + *
> + * This pointer must have been returned by a previous call to
> + * rte_malloc(), rte_zmalloc(), rte_calloc() or rte_realloc(). The behaviour of
> + * rte_free() is undefined if the pointer does not match this requirement.

Suggest add notice: The value may be cleared twice, which affects the performance.

> + *
> + * If the pointer is NULL, the function does nothing.
> + *
> + * @param ptr
> + *   The pointer to memory to be freed.
> + */
> +__rte_experimental
> +void
> +rte_free_sensitive(void *ptr);

one line is OK.
void rte_free_sensitive(void *ptr);

> +
>  /**
>   * This function allocates memory from the huge-page area of memory. The memory
>   * is not cleared. In NUMA systems, the memory allocated resides on the same
> diff --git a/lib/eal/version.map b/lib/eal/version.map
> index a20c713eb1..fa67ff44d5 100644
> --- a/lib/eal/version.map
> +++ b/lib/eal/version.map
> @@ -398,6 +398,9 @@ EXPERIMENTAL {
>  	# added in 24.11
>  	rte_bitset_to_str;
>  	rte_lcore_var_alloc;
> +
> +	# added in 25.03
> +	rte_free_sensitive;
>  };
>  
>  INTERNAL {
  

On Wed, 12 Feb 2025 10:01:13 +0800
fengchengwen <fengchengwen@huawei.com> wrote:

> On 2025/2/12 1:35, Stephen Hemminger wrote:
> > Although internally rte_free does poison the buffer in most
> > cases, it is useful to have function that explicitly does
> > this to avoid any security issues.
> > 
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> >  lib/eal/common/rte_malloc.c  | 30 ++++++++++++++++++++++++------
> >  lib/eal/include/rte_malloc.h | 18 ++++++++++++++++++
> >  lib/eal/version.map          |  3 +++
> >  3 files changed, 45 insertions(+), 6 deletions(-)
> > 
> > diff --git a/lib/eal/common/rte_malloc.c b/lib/eal/common/rte_malloc.c
> > index 3eed4d4be6..c9e0f4724f 100644
> > --- a/lib/eal/common/rte_malloc.c
> > +++ b/lib/eal/common/rte_malloc.c
> > @@ -15,6 +15,7 @@
> >  #include <rte_eal_memconfig.h>
> >  #include <rte_common.h>
> >  #include <rte_spinlock.h>
> > +#include <rte_string_fns.h>
> >  
> >  #include <eal_trace_internal.h>
> >  
> > @@ -27,27 +28,44 @@
> >  
> >  
> >  /* Free the memory space back to heap */
> > -static void
> > -mem_free(void *addr, const bool trace_ena)
> > +static inline void
> > +mem_free(void *addr, const bool trace_ena, bool zero)
> >  {
> > +	struct malloc_elem *elem;
> > +
> >  	if (trace_ena)
> >  		rte_eal_trace_mem_free(addr);
> >  
> > -	if (addr == NULL) return;
> > -	if (malloc_heap_free(malloc_elem_from_data(addr)) < 0)
> > +	if (addr == NULL)
> > +		return;
> > +
> > +	elem = malloc_elem_from_data(addr);
> > +	if (zero) {
> > +		size_t data_len = elem->size - MALLOC_ELEM_OVERHEAD;  
> 
> this will make rte_malloc know the layout of malloc-elem.
> Prefer to add extra paramter, e.g. malloc_heap_free(elem, bool zero)

Don't understand, these are functions inside the rte_malloc implementation file.
The layout of malloc_elem is already known here and nothing visible in API or ABI is changing.
>   
> > +
> > +/**
> > + * Frees the memory space pointed to by the provided pointer
> > + * and guarantees it will be zero'd before reuse.
> > + *
> > + * This pointer must have been returned by a previous call to
> > + * rte_malloc(), rte_zmalloc(), rte_calloc() or rte_realloc(). The behaviour of
> > + * rte_free() is undefined if the pointer does not match this requirement.  
> 
> Suggest add notice: The value may be cleared twice, which affects the performance.

That could easily change with a little work, this is only for crypto keys
so performance doesn't matter.

> > + *
> > + * If the pointer is NULL, the function does nothing.
> > + *
> > + * @param ptr
> > + *   The pointer to memory to be freed.
> > + */
> > +__rte_experimental
> > +void
> > +rte_free_sensitive(void *ptr);  
> 
> one line is OK.
> void rte_free_sensitive(void *ptr);

Yes it could be on one line, and more compact is my preferred style.
But other functions in this file and DPDK style guide say it should be on its own line.