2022-09-29 15:44:30

by David Laight

[permalink] [raw]
Subject: re: [PATCH 3/4] proc: Point /proc/net at /proc/thread-self/net instead of /proc/self/net

I've just bumped into (the lack of) this change (from aug 2014):

> In oddball cases where the thread has a different network namespace
> than the primary thread group leader or more likely in cases where
> the thread remains and the thread group leader has exited this
> ensures that /proc/net continues to work.

> - proc_symlink("net", NULL, "self/net");
> + proc_symlink("net", NULL, "thread-self/net");

This was applied and then reverted by Linus (I can't find anything
in the LKML archive) - see git show 155134fef - because of
issues with apparmor and dhclient.

In my case we have an application that is started in one
network namespace (where most of what it needs to do exists)
but needs one thread to revert to the 'init' namespace in
order to accept TCP connections from applications.

The thread that reverts is the main thread.
Until a change made in the last 6 months it actually worked.
(I'm using 5.10 LTS kernels so I'm not sure when.)
Then a fix was made to correctly update the mounts when the
namespace changed - and it suddenly stopped working.

So the 'oddball' case of different threads being in different
namespaces and then accessing /proc/net (because that is what
the code always did before being changed to run in a namespace)
has happened to a real application.

Fortunately it happened in testing and the application could
be changed.

(I was looking at the kernel sources to propose the change
that got reverted!)

David

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


2022-09-29 19:20:05

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH 3/4] proc: Point /proc/net at /proc/thread-self/net instead of /proc/self/net

On Thu, Sep 29, 2022 at 8:22 AM David Laight <[email protected]> wrote:
>
> This was applied and then reverted by Linus (I can't find anything
> in the LKML archive) - see git show 155134fef - because of
> issues with apparmor and dhclient.

lkml archive link:

https://lore.kernel.org/all/CADDKRnDD_W5yJLo2otWXH8oEgmGdMP0N_p7wenBQbh17xKGZJg@mail.gmail.com/

in case anybody cares.

I wonder if the fix is to replace the symlink with a hardcoded lookup
(ie basically make it *act* like a hardlink - we don't really support
hardlinked directories, but we could basically fake the lookup in
proc). Since the problem was AppArmor reacting to the name in the
symlink.

Al added the participants so that he can say "hell no".

Actually, it might be cleaner to make it act like a dynamic
mount-point instead - kind of "automount" style. Again, Al would be
the person who can say "sure, that makes sense" or "over my dead
body".

Al?

Or maybe that crazy AppArmor rule just doesn't exist any more. It's
been 8 years, after all.

Linus

2022-09-29 19:38:53

by Al Viro

[permalink] [raw]
Subject: Re: [PATCH 3/4] proc: Point /proc/net at /proc/thread-self/net instead of /proc/self/net

On Thu, Sep 29, 2022 at 11:21:36AM -0700, Linus Torvalds wrote:
> On Thu, Sep 29, 2022 at 8:22 AM David Laight <[email protected]> wrote:
> >
> > This was applied and then reverted by Linus (I can't find anything
> > in the LKML archive) - see git show 155134fef - because of
> > issues with apparmor and dhclient.
>
> lkml archive link:
>
> https://lore.kernel.org/all/CADDKRnDD_W5yJLo2otWXH8oEgmGdMP0N_p7wenBQbh17xKGZJg@mail.gmail.com/
>
> in case anybody cares.
>
> I wonder if the fix is to replace the symlink with a hardcoded lookup
> (ie basically make it *act* like a hardlink - we don't really support
> hardlinked directories, but we could basically fake the lookup in
> proc). Since the problem was AppArmor reacting to the name in the
> symlink.
>
> Al added the participants so that he can say "hell no".

What do you mean? Lookup on "net" in /proc returning what, exactly?
What would that dentry have for ->d_parent?

2022-09-29 20:20:24

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH 3/4] proc: Point /proc/net at /proc/thread-self/net instead of /proc/self/net

On Thu, Sep 29, 2022 at 11:50 AM Al Viro <[email protected]> wrote:
>
> What do you mean? Lookup on "net" in /proc returning what, exactly?

Returning the same directory as "thread-self/net", just not with a
symlink so that Apparmor doesn't get to mess things up..

> What would that dentry have for ->d_parent?

In a perfect world, I think it should act like a dynamic bind mount
(where the "dynamic" part is that thread-self part, and the parent
would be /proc.

That said, I think this is all a hack to deal with an Apparmor bug, so
I don't think we need perfect. Right now it's a symlink, so the parent
is the thread-self directory. I think that kind of magic jump would be
perfectly acceptable.

We have "magic jump" behavior in other /proc places, where the thing
*looks* like a symlink (ie readlink and friends just work), but the
lookup doesn't *actually* follow the symlink, it just looks things up
directly. IOW, all the /proc/<pid>/fd/<X> stuff.

So I think this would be just another case of that.

Linus