Re: [PATCH net 1/1] net/mlx5e: fix NULL pointer dereference in arfs_create_groups()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] net/mlx5e: single-word handle used in Signed-off-by trailer
--

commit e432b554ac248a62d98278a82ac39e3c5d863c99
Author: Jinvas <jinvas@126.com>

> net/mlx5e: fix NULL pointer dereference in arfs_create_groups()
>
> arfs_create_groups() creates up to two flow groups in ft->g[], and
> increments ft->num_groups for each group that was created successfully.
> If creating the second group fails, the error path frees ft->g and sets
> it to NULL, but leaves ft->num_groups at 1.
>
> The only caller, arfs_create_table(), calls mlx5e_destroy_flow_table() on
> the error path, which calls mlx5e_destroy_groups(). The latter walks
> ft->g[] from ft->num_groups - 1 down to 0 and dereferences ft->g[0].
> Since ft->g was freed and set to NULL, this is a NULL pointer
> dereference, and the kernel oopses.
>
> The same error path also leaks the flow group that was created
> successfully before the failure, since ft->g is freed without the group
> being destroyed.
>
> Freeing ft->g here was added by commit 3c6d5189246f ("net/mlx5e: fix a
> double-free in arfs_create_groups"). Before that commit, the group
> failure path did not touch ft->g at all, so the caller destroyed the
> flow groups that had been created and then freed the array. That
> commit fixed a double free in the allocation failure path by freeing
> ft->g here as well, and in doing so changed the group failure path
> too.
>
> Rather than also resetting ft->num_groups, leave the free of ft->g to
> the caller: mlx5e_destroy_flow_table() frees ft->g exactly once, and
> also destroys the flow groups that were created. This fixes the NULL
> pointer dereference and the leak, while keeping the double free fixed.
>
> This also makes the error handling of this function consistent with the
> rest of the driver, e.g. mlx5e_create_l2_table().
>
> Fixes: 3c6d5189246f ("net/mlx5e: fix a double-free in arfs_create_groups")
> Signed-off-by: Jinvas <jinvas@126.com>

[Severity: High]
Documentation/process/submitting-patches.rst requires a full real name in
the Author header and Signed-off-by trailer rather than a single-word
handle or username, so the patch cannot be merged under the Developer's
Certificate of Origin rules as signed.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261114144325.816-1-jinvas@126.com?part=1