netdev - Re: [PATCH] genetlink: fix potencial use-after-free and null-ptr-deref in genl

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20240220130206.GH40273@kernel.org>
Date: Tue, 20 Feb 2024 13:02:06 +0000
From: Simon Horman <horms@...nel.org>
To: kovalev@...linux.org
Cc: netdev@...r.kernel.org, davem@...emloft.net, edumazet@...gle.com,
	kuba@...nel.org, pabeni@...hat.com, jiri@...nulli.us,
	jacob.e.keller@...el.com, johannes@...solutions.net,
	idosch@...dia.com, David Lebrun <david.lebrun@...ouvain.be>,
	Pablo Neira Ayuso <pablo@...filter.org>
Subject: Re: [PATCH] genetlink: fix potencial use-after-free and
 null-ptr-deref in genl_dumpit()

On Mon, Feb 19, 2024 at 03:15:52PM +0300, kovalev@...linux.org wrote:
> + Pablo Neira Ayuso <pablo@...filter.org>
> 
> 19.02.2024 14:32, Simon Horman wrote:
> > + Jiri Pirko <jiri@...nulli.us>
> >    David Lebrun <david.lebrun@...ouvain.be>
> > 
> > On Thu, Feb 15, 2024 at 11:23:09PM +0300, kovalev@...linux.org wrote:
> > > From: Vasiliy Kovalev <kovalev@...linux.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
>