diff mbox series

[v3] lib/graph: lib/graph: fix memset with NULL

Message ID 20250625092722.70938-1-marat.khalili@huawei.com (mailing list archive)
State New
Delegated to: Thomas Monjalon
Headers
Series [v3] lib/graph: lib/graph: fix memset with NULL |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/github-robot: build success github build: passed
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-marvell-Functional success Functional Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-compile-amd64-testing warning Testing issues
ci/intel-Testing success Testing PASS
ci/intel-Functional success Functional PASS
ci/aws-unit-testing success Unit Testing PASS
ci/iol-sample-apps-testing success Testing PASS

Commit Message

Marat Khalili June 25, 2025, 9:27 a.m. UTC 
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

Add a check that stat->xstat_cntrs is not zero before the call, since
stat->xstat_count can only be NULL when stat->xstat_cntrs is zero.

Fixes: 070db97e017 ("graph: support node xstats")

Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
---

Thanks to Jerin Jacob and David Marchand for the reviews.

v3:
* Addressing comments from David Marchand change to check the length
instead of the pointer, fix formatting.
* Drop the other half of the two-patch set since the problem it was
addressing was already getting fixed elsewhere.

v2: Following the suggestions from Jerin Jacob changed the Subject and
added Fixes line.

 lib/graph/graph_stats.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

David Marchand July 8, 2025, 12:40 p.m. UTC | #1 
On Wed, Jun 25, 2025 at 11:29 AM 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
>
> Add a check that stat->xstat_cntrs is not zero before the call, since
> stat->xstat_count can only be NULL when stat->xstat_cntrs is zero.
>
> Fixes: 070db97e017 ("graph: support node xstats")
>
> Signed-off-by: Marat Khalili <marat.khalili@huawei.com>

Just a fyi, I added your patch as part of my series, since running
with UBSan in GHA required this fix.

We may still take your fix as is, so I'll leave it in patchwork.
diff mbox series

Patch

diff --git a/lib/graph/graph_stats.c b/lib/graph/graph_stats.c
index bb202577e9..9e330c8e61 100644
--- a/lib/graph/graph_stats.c
+++ b/lib/graph/graph_stats.c
@@ -470,7 +470,8 @@  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_cntrs != 0)
+		memset(stat->xstat_count, 0, sizeof(uint64_t) * stat->xstat_cntrs);
 	for (count = 0; count < cluster->nb_nodes; count++) {
 		node = cluster->nodes[count];