netdev - Re: [PATCH v2 net-next 1/7] socket: Un-export __sock

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250526110632.7ab3323e@pumpkin>
Date: Mon, 26 May 2025 11:06:32 +0100
From: David Laight <david.laight.linux@...il.com>
To: Christoph Hellwig <hch@....de>
Cc: Kuniyuki Iwashima <kuniyu@...zon.com>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski
 <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Willem de Bruijn
 <willemb@...gle.com>, Simon Horman <horms@...nel.org>, Kuniyuki Iwashima
 <kuni1840@...il.com>, Chuck Lever <chuck.lever@...cle.com>, Jeff Layton
 <jlayton@...nel.org>, Matthieu Baerts <matttbe@...nel.org>, Keith Busch
 <kbusch@...nel.org>, Jens Axboe <axboe@...nel.dk>, Wenjia Zhang
 <wenjia@...ux.ibm.com>, Jan Karcher <jaka@...ux.ibm.com>, Steve French
 <sfrench@...ba.org>, netdev@...r.kernel.org, mptcp@...ts.linux.dev,
 linux-nfs@...r.kernel.org, linux-rdma@...r.kernel.org,
 linux-nvme@...ts.infradead.org
Subject: Re: [PATCH v2 net-next 1/7] socket: Un-export __sock_create().

On Mon, 26 May 2025 07:29:07 +0200
Christoph Hellwig <hch@....de> wrote:

> On Fri, May 23, 2025 at 11:21:07AM -0700, Kuniyuki Iwashima wrote:
> > Since commit eeb1bd5c40ed ("net: Add a struct net parameter to
> > sock_create_kern"), we no longer need to export __sock_create()
> > and can replace all non-core users with sock_create_kern().
> > 
> > Let's convert them and un-export __sock_create().  
> 
> The changes looks good, but the commit log including subject line
> is rather confusing.  What you do is to replace all uses of
> __sock_create with sock_create_kern, which works because
> sock_create_kern just calls __sock_create with the last argument set
> to 1 as those callers do it.  This then allows marking __sock_create
> static because all outside users are gone.
> 
> Please state that, i.e.
> 
> Subect: use sock_create_kern insteadf of opencoding it
> 
> Replace all callers of __sock_create that set the kernel argument to 1
> with sock_create_kern, which is the improve interface for that.
> Mark __sock_create static now that all users outside of socket.c
> are gone.

I'd also like to see an explicit statement on all these patches
about whether the created sockets hold a reference to the namespace.

I know it is documented in the function definitions, but the issue
has always been that the callers get it wrong.

From what I remember, as this point in the patch series sock_create_kern()
doesn't holds a reference, but by the end of the series it does.
That just has to be a recipe for disaster and pretty much requires the
changes all go through the same tree in one merge window.
But the code touches multiple areas and the changes would normally go through
multiple trees.
So it's going to be hard to get all the acks and the patch accepted.
(Unless you persuade Linus to 'just apply the changes'.

I think you need to look at three merge windows.
1) Add new function(s) for creating user/kernel sockets with/without holding
   a namespace reference.
2) Update all the callers to use the new functions.
3) Delete the old functions.

There is no point modifying the callers twice, and the commits need to
explicitly state whether they want the namespace held or not.

	David