Re: [PATCH] genetlink: fix potencial use-after-free and null-ptr-deref in genl_dumpit()

[Simon Horman <horms@kernel.org>]

On Mon, Feb 19, 2024 at 03:15:52PM +0300, kovalev@altlinux.org wrote:
> + Pablo Neira Ayuso <pablo@netfilter.org>
> 
> 19.02.2024 14:32, Simon Horman wrote:
> > + Jiri Pirko <jiri@resnulli.us>
> >    David Lebrun <david.lebrun@uclouvain.be>
> > 
> > On Thu, Feb 15, 2024 at 11:23:09PM +0300, kovalev@altlinux.org wrote:
> > > From: Vasiliy Kovalev <kovalev@altlinux.org>
> > > 
> > > The pernet operations structure for the subsystem must be registered
> > > before registering the generic netlink family.
> > > 
> > > Fixes: 134e63756d5f ("genetlink: make netns aware")
> > Hi Vasiliy,
> > 
> > A Fixes tag implies that this is a bug fix.
> > So I think some explanation is warranted of what, user-visible,
> > problem this resolves.
> > 
> > In that case the patch should be targeted at net.
> > Which means it should be based on that tree and have a net annotation
> > in the subject
> > 
> > 	Subject: [PATCH net] ...
> > 
> > Alternatively, the Fixes tag should be dropped and some explanation
> > should be provided of why the structure needs to be registered before
> > the family.
> > 
> > In this case, if you wish to refer to the patch where the problem (but not
> > bug) was introduced you can use something like the following.
> > It is just the Fixes tag that has a special meaning.
> > 
> > 	Introduced in 134e63756d5f ("genetlink: make netns aware")
> > 
> > I think the above comments also apply to:
> > 
> > - [PATCH] ipv6: sr: fix possible use-after-free and null-ptr-deref
> >    https://lore.kernel.org/all/20240215202717.29815-1-kovalev@altlinux.org/
> > 
> > - [PATCH] devlink: fix possible use-after-free and memory leaks in devlink_init()
> >    https://lore.kernel.org/all/20240215203400.29976-1-kovalev@altlinux.org/
> > 
> > And as these patches seem to try to fix the same problem in different
> > places, all under Networking, I would suggest that if you do repost,
> > they are combined into a patch series (3 patches in the same series).
> > 
> > But I do wonder, how such an apparently fundamental problem has been
> > present for so long in what I assume to be well exercised code.
> 
> Hi Simon,
> 
> The history of these changes began with the crash fix in the gtp module [1]
> 
> A solution to the problem was found [2] and Pablo Neruda Ayuso suggested
> fixing similar
> 
> sections of code if they might have the same problem.
> 
> I have sent patches, but do not have reproducers, relying on drawing
> attention to the problem.

Thanks Vasiliy,

I think it would be worth adding some text along those lines to
the commit messages for the patches you have posted.

> 
> 
> [1]
> https://lore.kernel.org/lkml/20240124101404.161655-1-kovalev@altlinux.org/T/
> 
> [2] https://lore.kernel.org/netdev/20240214162733.34214-1-kovalev@altlinux.org/T/#u
> 
> -- 
> Thanks,
> Vasiliy Kovalev
>