Message-ID: <958649bb1d76442d8aa76067e0a3e0b6@AcuMS.aculab.com>
Date:   Tue, 6 Sep 2022 08:25:59 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Alexey Dobriyan' <adobriyan@...il.com>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>,
        Eric Dumazet <edumazet@...gle.com>,
        "viro@...iv.linux.org.uk" <viro@...iv.linux.org.uk>,
        "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        "regressions@...ts.linux.dev" <regressions@...ts.linux.dev>
Subject: RE: setns() affecting other threads in 5.10.132 and 6.0

From: Alexey Dobriyan
> Sent: 05 September 2022 18:33
> 
> On Mon, Sep 05, 2022 at 09:54:34AM +0000, David Laight wrote:
> > 7055197705709c59b8ab77e6a5c7d46d61edd96e
> > Author: Alexey Dobriyan <adobriyan@...il.com>
> >     Cc: Al Viro <viro@...iv.linux.org.uk>
> >     Signed-off-by: Andrew Morton <akpm@...ux-foundation.org>
> > c6c75deda813
> > 1fde6f21d90f
> >
> > > -----Original Message-----
> > > From: David Laight <David.Laight@...LAB.COM>
> > > Sent: 04 September 2022 15:05
> > >
> > > Sometime after 5.10.105 (5.10.132 and 6.0) there is a change that
> > > makes setns(open("/proc/1/ns/net")) in the main process change
> > > the behaviour of other process threads.
> 
> Not again...

The bisection gave a whack-a-mole patch :-)

> > > I don't know how much is broken, but the following fails.
> > >
> > > Create a network namespace (eg "test").
> > > Create a 'bond' interface (eg "test0") in the namespace.
> > >
> > > Then /proc/net/bonding/test0 only exists inside the namespace.
> > >
> > > However if you run a program in the "test" namespace that does:
> > > - create a thread.
> > > - change the main thread to in "init" namespace.
> > > - try to open /proc/net/bonding/test0 in the thread.
> > > then the open fails.
> > >
> > > I don't know how much else is affected and haven't tried
> > > to bisect (I can't create bonds on my normal test kernel).
> >
> > I've now bisected it.
> > Prior to change 7055197705709c59b8ab77e6a5c7d46d61edd96e
> >     proc: fix dentry/inode overinstantiating under /proc/${pid}/net
> > the setns() had no effect of either thread.
> > Afterwards both threads see the entries in the init namespace.
> >
> > However I think that in 5.10.105 the setns() did affect
> > the thread it was run in.
> > That might be the behaviour before c6c75deda813.
> >     proc: fix lookup in /proc/net subdirectories after setns(2)
> >
> > There is also the earlier 1fde6f21d90f
> >     proc: fix /proc/net/* after setns(2)
> >
> > From the commit messages it does look as though setns() should
> > change what is seen, but just for the current thread.
> > So it is currently broken - and has been since 5.18.0-rc4
> > and whichever stable branches the change was backported to.
> >
> > 	David
> >
> > >
> > > The test program below shows the problem.
> > > Compile and run as:
> > > # ip netns exec test strace -f test_prog /proc/net/bonding/test0
> > >
> > > The second open by the child should succeed, but fails.
> > >
> > > I can't see any changes to the bonding code, so I suspect
> > > it is something much more fundamental.
> > > It might only affect /proc/net, but it might also affect
> > > which namespace sockets get created in.
> 
> How? setns(2) acts on "current", and sockets are created from
> current->nsproxy->net_ns?

I was worried that things might be really badly broken.
The bisection implied /proc/net rather than namespace setup itself.

> > > IIRC ls -l /proc/n/task/*/ns gives the correct namespaces.
> 
> > >
> > > 	David
> > >
> > >
> > > #define _GNU_SOURCE
> > >
> > > #include <fcntl.h>
> > > #include <unistd.h>
> > > #include <poll.h>
> > > #include <pthread.h>
> > > #include <sched.h>
> > >
> > > #define delay(secs) poll(0,0, (secs) * 1000)
> > >
> > > static void *thread_fn(void *file)
> > > {
> > >         delay(2);
> > >         open(file, O_RDONLY);
> > >
> > >         delay(5);
> > >         open(file, O_RDONLY);
> > >
> > >         return NULL;
> > > }
> > >
> > > int main(int argc, char **argv)
> > > {
> > >         pthread_t id;
> > >
> > >         pthread_create(&id, NULL, thread_fn, argv[1]);
> > >
> > >         delay(1);
> > >         open(argv[1], O_RDONLY);
> > >
> > >         delay(2);
> > >         setns(open("/proc/1/ns/net", O_RDONLY), 0);
> > >
> > >         delay(1);
> > >         open(argv[1], O_RDONLY);
> > >
> > >         delay(4);
> > >
> > >         return 0;
> > > }
> 
> Can you test before this one? This is where it all started.
> 
> 	commit 1da4d377f943fe4194ffb9fb9c26cc58fad4dd24

That one really doesn't want to build with the toolchain I'm using.
gcc generates a lot of warnings (mostly about function pointer
types) and then objtool fails the build.

The changes seem to be about flushing the dnlc.
In my case everything is pretty static.
The namespace is created once just after boot.
The real failing program just does:
	ip netns exec namespace program 3</sys/class/net
	process: clone()
	process: setns()
	thread: open("/proc/net/bonding/not_in_namespace")
and expects the open() to fail.
So it actually looks like the wrong namespace is being used.

It is interesting that /proc/net is affected by setns()
whereas you have to go through hoops to access /sys/class/net.
We pass an open fd to /sys/class/net in the init namespace
through to the program so it can use openat(3, ...) to
see entries in both namespaces.

The namespace exists to separate network traffic, not isolate
applications.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Powered by blists - more mailing lists