Re: [PATCH v2 1/2] lib/graph: lib/graph: fix memset with NULL

[David Marchand <david.marchand@redhat.com>]

On Tue, Jun 17, 2025 at 5:14 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.
>
> Fixes: 070db97e017 ("graph: support node xstats")
>
> Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
> ---
>
> v2: Following the suggestions from Jerin Jacob changed the Subject and
> added Fixes line.
>
>  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)

I would check against stat->xstat_cntrs since the array is allocated
based on this count.
This will be more consistent with the loop on this same counter later
in this function.


> +               memset(stat->xstat_count, 0,
> +                       sizeof(uint64_t) * stat->xstat_cntrs);

No need for going to a new line.


>         for (count = 0; count < cluster->nb_nodes; count++) {
>                 node = cluster->nodes[count];
>

Thanks for the fix.


-- 
David Marchand