Re: [PATCH] [v2] net/mlx5e: fix a double-free in arfs_create_groups

From: alexious
Date: Fri Jan 12 2024 - 02:46:15 EST



> On Wed, Jan 10, 2024 at 09:23:24PM +0800, alexious@xxxxxxxxxx wrote:
> > > On Mon, Jan 08, 2024 at 11:26:04PM +0800, Zhipeng Lu wrote:
> > > > When `in` allocated by kvzalloc fails, arfs_create_groups will free
> > > > ft->g and return an error. However, arfs_create_table, the only caller of
> > > > arfs_create_groups, will hold this error and call to
> > > > mlx5e_destroy_flow_table, in which the ft->g will be freed again.
> > > >
> > > > Fixes: 1cabe6b0965e ("net/mlx5e: Create aRFS flow tables")
> > > > Signed-off-by: Zhipeng Lu <alexious@xxxxxxxxxx>
> > > > Reviewed-by: Simon Horman <horms@xxxxxxxxxx>
> > >
> > > When working on netdev (and probably elsewhere)
> > > Please don't include Reviewed-by or other tags
> > > that were explicitly supplied by someone: I don't recall
> > > supplying the tag above so please drop it.
> >
> > I apologize, but it appears that you included a "reviewed-by"
> > tag along with certain suggestions for version 1 of this patch
> > in the first review email(about 6 days before).
>
> Yes, sorry. My statement above is not correct:
> I now see that I did supply the tag.

Never mind, you did give a lot of constructive suggestion proberly to every patch.
Forgetting about a tag, well, it can't be helped.

> > In response, after a short discussion, I followed some of
> > those suggestions and send this v2 patch.
> > I referred to the "Dealing with tags" section in this KernelNewbies
> > tips: https://kernelnewbies.org/PatchTipsAndTricks and thought
> > that I should include that tag in v1 email to this v2 patch.
> > So now I'm a little bit confused here: if the tag rule has changed
> > or I got some misunderstanding on existing rules? Your clarification
> > on this matter would be greatly appreciated.
>

...

>
> Right, I think it would be best to focus on fixing arfs_create_groups().
> And making sure that neither leaks nor double frees occur. And I think
> that at this point that includes ensuring ft->g is NULL if it has been freed.

A v3 version of this patch was sent just now, which is focusing on arfs_create_groups itself.
Have a nice day!