[PATCH 1/2] lib/graph: avoid memset(NULL, 0, 0)

[PATCH 1/2] lib/graph: avoid memset(NULL, 0, 0)

[Marat Khalili]

This was flagged by undefined behaviour sanitizer: memset should not be
called with NULL first argument. (memset requires first argument to be
pointer to a memory object, so passing NULL may result in an undefined
behaviour including among other things optimizer potentially removing
code paths depending on stat->xstat_count being NULL.)

Sanitizer message:

    lib/graph/graph_stats.c:473:2: runtime error: null pointer passed as
    argument 1, which is declared to never be null

To fix the issue add a check that stat->xstat_count is not NULL before
the call.

Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
---
 lib/graph/graph_stats.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/graph/graph_stats.c b/lib/graph/graph_stats.c
index eac73cbf71..57cd72e7cc 100644
--- a/lib/graph/graph_stats.c
+++ b/lib/graph/graph_stats.c
@@ -470,7 +470,9 @@ cluster_node_arregate_stats(struct cluster_node *cluster, bool dispatch)
 	uint64_t *xstat;
 	uint8_t i;
 
-	memset(stat->xstat_count, 0, sizeof(uint64_t) * stat->xstat_cntrs);
+	if (stat->xstat_count != NULL)
+		memset(stat->xstat_count, 0,
+			sizeof(uint64_t) * stat->xstat_cntrs);
 	for (count = 0; count < cluster->nb_nodes; count++) {
 		node = cluster->nodes[count];
 
-- 
2.43.0

[PATCH 2/2] lib/graph: rte_malloc for cache-aligned structs

[Marat Khalili]

This was flagged by undefined behaviour sanitizer: struct
rte_graph_cluster_stats is declared as `__rte_cache_aligned` but was
allocated using stdlib realloc which caused misaligned allocation. More
than one test needs to be executed in series in order to reproduce the
problem using graph_autotest, e.g:

    app/dpdk-test --no-huge --no-pci -m128 graph_autotest graph_autotest

First sanitizer message  (similar ones follow):

    lib/graph/graph_stats.c:209:13: runtime error: member access within
    misaligned address 0x606000008ea0 for type 'struct
    rte_graph_cluster_stats', which requires 64 byte alignment

To fix the issue replace realloc calls with rte_malloc and rte_realloc
specifying correct alignment, use rte_free to free the result.

Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
---
 lib/graph/graph_stats.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/lib/graph/graph_stats.c b/lib/graph/graph_stats.c
index 57cd72e7cc..7ae8ee4987 100644
--- a/lib/graph/graph_stats.c
+++ b/lib/graph/graph_stats.c
@@ -203,7 +203,7 @@ stats_mem_init(struct cluster *cluster,
 	cluster_node_size += cluster->nb_graphs * sizeof(struct rte_node *);
 	cluster_node_size = RTE_ALIGN(cluster_node_size, RTE_CACHE_LINE_SIZE);
 
-	stats = realloc(NULL, sz);
+	stats = rte_malloc(NULL, sz, RTE_CACHE_LINE_SIZE);
 	if (stats) {
 		memset(stats, 0, sz);
 		stats->fn = fn;
@@ -248,7 +248,8 @@ stats_mem_populate(struct rte_graph_cluster_stats **stats_in,
 	}
 
 	/* Hey, it is a new node, allocate space for it in the reel */
-	stats = realloc(stats, stats->sz + stats->cluster_node_size);
+	stats = rte_realloc(stats, stats->sz + stats->cluster_node_size,
+		RTE_CACHE_LINE_SIZE);
 	if (stats == NULL)
 		SET_ERR_JMP(ENOMEM, err, "Realloc failed");
 	*stats_in = NULL;
@@ -301,7 +302,7 @@ stats_mem_populate(struct rte_graph_cluster_stats **stats_in,
 
 	return 0;
 free:
-	free(stats);
+	rte_free(stats);
 err:
 	return -rte_errno;
 }
@@ -309,7 +310,7 @@ stats_mem_populate(struct rte_graph_cluster_stats **stats_in,
 static void
 stats_mem_fini(struct rte_graph_cluster_stats *stats)
 {
-	free(stats);
+	rte_free(stats);
 }
 
 static void
-- 
2.43.0

Re: [PATCH 2/2] lib/graph: rte_malloc for cache-aligned structs

[Jerin Jacob]

On Tue, Jun 17, 2025 at 4:32 PM Marat Khalili <marat.khalili@huawei.com> wrote:
>
> This was flagged by undefined behaviour sanitizer: struct
> rte_graph_cluster_stats is declared as `__rte_cache_aligned` but was
> allocated using stdlib realloc which caused misaligned allocation. More
> than one test needs to be executed in series in order to reproduce the
> problem using graph_autotest, e.g:
>
>     app/dpdk-test --no-huge --no-pci -m128 graph_autotest graph_autotest
>
> First sanitizer message  (similar ones follow):
>
>     lib/graph/graph_stats.c:209:13: runtime error: member access within
>     misaligned address 0x606000008ea0 for type 'struct
>     rte_graph_cluster_stats', which requires 64 byte alignment
>
> To fix the issue replace realloc calls with rte_malloc and rte_realloc
> specifying correct alignment, use rte_free to free the result.
>
> Signed-off-by: Marat Khalili <marat.khalili@huawei.com>


Since this memory is used in slowpath, heap memory is fine.
I think, better fix will be to remove cache alignment from
rte_graph_cluster_stats.
Not sure it will call for ABI change though.Run
devtools/test-meson-builds.sh to validate any ABI breakage.






> ---
>  lib/graph/graph_stats.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/lib/graph/graph_stats.c b/lib/graph/graph_stats.c
> index 57cd72e7cc..7ae8ee4987 100644
> --- a/lib/graph/graph_stats.c
> +++ b/lib/graph/graph_stats.c
> @@ -203,7 +203,7 @@ stats_mem_init(struct cluster *cluster,
>         cluster_node_size += cluster->nb_graphs * sizeof(struct rte_node *);
>         cluster_node_size = RTE_ALIGN(cluster_node_size, RTE_CACHE_LINE_SIZE);
>
> -       stats = realloc(NULL, sz);
> +       stats = rte_malloc(NULL, sz, RTE_CACHE_LINE_SIZE);
>         if (stats) {
>                 memset(stats, 0, sz);
>                 stats->fn = fn;
> @@ -248,7 +248,8 @@ stats_mem_populate(struct rte_graph_cluster_stats **stats_in,
>         }
>
>         /* Hey, it is a new node, allocate space for it in the reel */
> -       stats = realloc(stats, stats->sz + stats->cluster_node_size);
> +       stats = rte_realloc(stats, stats->sz + stats->cluster_node_size,
> +               RTE_CACHE_LINE_SIZE);
>         if (stats == NULL)
>                 SET_ERR_JMP(ENOMEM, err, "Realloc failed");
>         *stats_in = NULL;
> @@ -301,7 +302,7 @@ stats_mem_populate(struct rte_graph_cluster_stats **stats_in,
>
>         return 0;
>  free:
> -       free(stats);
> +       rte_free(stats);
>  err:
>         return -rte_errno;
>  }
> @@ -309,7 +310,7 @@ stats_mem_populate(struct rte_graph_cluster_stats **stats_in,
>  static void
>  stats_mem_fini(struct rte_graph_cluster_stats *stats)
>  {
> -       free(stats);
> +       rte_free(stats);
>  }
>
>  static void
> --
> 2.43.0
>

Re: [PATCH 2/2] lib/graph: rte_malloc for cache-aligned structs

[Stephen Hemminger]

On Tue, 17 Jun 2025 11:52:08 +0100
Marat Khalili <marat.khalili@huawei.com> wrote:

> This was flagged by undefined behaviour sanitizer: struct
> rte_graph_cluster_stats is declared as `__rte_cache_aligned` but was
> allocated using stdlib realloc which caused misaligned allocation. More
> than one test needs to be executed in series in order to reproduce the
> problem using graph_autotest, e.g:
> 
>     app/dpdk-test --no-huge --no-pci -m128 graph_autotest graph_autotest
> 
> First sanitizer message  (similar ones follow):
> 
>     lib/graph/graph_stats.c:209:13: runtime error: member access within
>     misaligned address 0x606000008ea0 for type 'struct
>     rte_graph_cluster_stats', which requires 64 byte alignment
> 
> To fix the issue replace realloc calls with rte_malloc and rte_realloc
> specifying correct alignment, use rte_free to free the result.
> 
> Signed-off-by: Marat Khalili <marat.khalili@huawei.com>

There is a way to get aligned memory,

       #include <stdlib.h>

       int posix_memalign(void **memptr, size_t alignment, size_t size);
       void *aligned_alloc(size_t alignment, size_t size);
  

Re: [PATCH 2/2] lib/graph: rte_malloc for cache-aligned structs

[Jerin Jacob]

On Tue, Jun 17, 2025 at 7:07 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Tue, 17 Jun 2025 11:52:08 +0100
> Marat Khalili <marat.khalili@huawei.com> wrote:
>
> > This was flagged by undefined behaviour sanitizer: struct
> > rte_graph_cluster_stats is declared as `__rte_cache_aligned` but was
> > allocated using stdlib realloc which caused misaligned allocation. More
> > than one test needs to be executed in series in order to reproduce the
> > problem using graph_autotest, e.g:
> >
> >     app/dpdk-test --no-huge --no-pci -m128 graph_autotest graph_autotest
> >
> > First sanitizer message  (similar ones follow):
> >
> >     lib/graph/graph_stats.c:209:13: runtime error: member access within
> >     misaligned address 0x606000008ea0 for type 'struct
> >     rte_graph_cluster_stats', which requires 64 byte alignment
> >
> > To fix the issue replace realloc calls with rte_malloc and rte_realloc
> > specifying correct alignment, use rte_free to free the result.
> >
> > Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
>
> There is a way to get aligned memory,
>
>        #include <stdlib.h>
>
>        int posix_memalign(void **memptr, size_t alignment, size_t size);
>        void *aligned_alloc(size_t alignment, size_t size);

There is a need for aligned realloc. I think it is not there in
standard libraries.



>

Re: [PATCH 1/2] lib/graph: avoid memset(NULL, 0, 0)

[Jerin Jacob]

On Tue, Jun 17, 2025 at 4:22 PM Marat Khalili <marat.khalili@huawei.com> wrote:
>
> This was flagged by undefined behaviour sanitizer: memset should not be
> called with NULL first argument. (memset requires first argument to be
> pointer to a memory object, so passing NULL may result in an undefined
> behaviour including among other things optimizer potentially removing
> code paths depending on stat->xstat_count being NULL.)
>
> Sanitizer message:
>
>     lib/graph/graph_stats.c:473:2: runtime error: null pointer passed as
>     argument 1, which is declared to never be null
>
> To fix the issue add a check that stat->xstat_count is not NULL before
> the call.
>
> Signed-off-by: Marat Khalili <marat.khalili@huawei.com>


Change subject as  lib/graph: fix memset with NULL. Also add Fixes:
With that

Acked-by: Jerin Jacob <jerinj@marvell.com>


> ---
>  lib/graph/graph_stats.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/lib/graph/graph_stats.c b/lib/graph/graph_stats.c
> index eac73cbf71..57cd72e7cc 100644
> --- a/lib/graph/graph_stats.c
> +++ b/lib/graph/graph_stats.c
> @@ -470,7 +470,9 @@ cluster_node_arregate_stats(struct cluster_node *cluster, bool dispatch)
>         uint64_t *xstat;
>         uint8_t i;
>
> -       memset(stat->xstat_count, 0, sizeof(uint64_t) * stat->xstat_cntrs);
> +       if (stat->xstat_count != NULL)
> +               memset(stat->xstat_count, 0,
> +                       sizeof(uint64_t) * stat->xstat_cntrs);
>         for (count = 0; count < cluster->nb_nodes; count++) {
>                 node = cluster->nodes[count];
>
> --
> 2.43.0
>