On Thu, Sep 29, 2022 at 12:05:32PM -0700, Linus Torvalds wrote:
> On Thu, Sep 29, 2022 at 12:00 PM Al Viro <[email protected]> wrote:
> >
> > Which is insane, especially since the entire problem is due to wanting
> > that directory to be different for different threads...
>
> Absolutely. This is all due to Apparmor (a) basing things on pathnames
> and (b) then getting those pathnames wrong.
>
> Which is why I'm just suggesting we short-circuit the path-name part,
> and not make this be a real symlink that actually walks a real path.
>
> The proc <pid> handling uses "readlink" to make it *look* like a
> symlink, but then "get_link" to actually look it up (and never walk it
> as a path).
>
> Something similar?
Apparmor takes mount+dentry and turns that into pathname. Then acts
upon the resulting string. *AFTER* the original had been resolved.
IOW, it doesn't see the symlink contents - only the location where the
entire thing ends up.
AFAICS, the only way to make it STFU is either
* fix the idiotic policy
or
* make the per-thread directory show up as /proc/<something>/net
As in "../.. from there lands you in /proc". Because that's what
apparmor does to generate the string it treats as the pathname...
On Thu, Sep 29, 2022 at 12:34 PM Al Viro <[email protected]> wrote:
>
> Apparmor takes mount+dentry and turns that into pathname. Then acts
> upon the resulting string. *AFTER* the original had been resolved.
Ok. So it would have to act like a bind mount.
Which is probably not too bad.
In fact, maybe it would be ok for this to act like a hardlink and just
fill in the inode - not safe for a filesystem in general due to the
whole rename loop issue, but for /proc it might be fine?
Linus
On Thu, Sep 29, 2022 at 02:13:57PM -0700, Linus Torvalds wrote:
> On Thu, Sep 29, 2022 at 12:34 PM Al Viro <[email protected]> wrote:
> >
> > Apparmor takes mount+dentry and turns that into pathname. Then acts
> > upon the resulting string. *AFTER* the original had been resolved.
>
> Ok. So it would have to act like a bind mount.
>
> Which is probably not too bad.
>
> In fact, maybe it would be ok for this to act like a hardlink and just
> fill in the inode - not safe for a filesystem in general due to the
> whole rename loop issue, but for /proc it might be fine?
_Which_ hardlink?
Linus, where in dentry tree would you want it to be seen? Because
apparmor profile wants /proc/net/dev to land at /proc/<pid>/net/dev
and will fail with anything else.
Do you really want multiple dentries with the same name in the same
parent, refering to different directory inodes with different contents?
And that's different inodes with different contents - David's complaint
is precisely about seeing the same thing for all threads and apparmor
issue is with *NOT* seeing each of those things at the same location.
On Thu, Sep 29, 2022 at 08:34:52PM +0100, Al Viro wrote:
> On Thu, Sep 29, 2022 at 12:05:32PM -0700, Linus Torvalds wrote:
> > On Thu, Sep 29, 2022 at 12:00 PM Al Viro <[email protected]> wrote:
> > >
> > > Which is insane, especially since the entire problem is due to wanting
> > > that directory to be different for different threads...
> >
> > Absolutely. This is all due to Apparmor (a) basing things on pathnames
> > and (b) then getting those pathnames wrong.
> >
> > Which is why I'm just suggesting we short-circuit the path-name part,
> > and not make this be a real symlink that actually walks a real path.
> >
> > The proc <pid> handling uses "readlink" to make it *look* like a
> > symlink, but then "get_link" to actually look it up (and never walk it
> > as a path).
> >
> > Something similar?
>
> Apparmor takes mount+dentry and turns that into pathname. Then acts
> upon the resulting string. *AFTER* the original had been resolved.
> IOW, it doesn't see the symlink contents - only the location where the
> entire thing ends up.
>
> AFAICS, the only way to make it STFU is either
> * fix the idiotic policy
> or
> * make the per-thread directory show up as /proc/<something>/net
>
> As in "../.. from there lands you in /proc". Because that's what
> apparmor does to generate the string it treats as the pathname...
FWIW, what e.g. debian profile for dhclient has is
@{PROC}/@{pid}/net/dev r,
Note that it's not
@{PROC}/net/dev r,
precisely because the rules are applied after the pathname got resolved.
*IF* we want that rule to allow opening /proc/net/dev, we'd better have
it yield a dentry in procfs that would have "dev" as ->d_name, with
its parent having "net" as ->d_name and its grandparent being the child
of procfs root with ->d_name containing decimal representation of PID.
Worse, original poster in _this_ thread wants the same /proc/net/dev to
to yield different files for different threads belonging to the same
process and we'd need _all_ of them to have identical chain of ->d_name
occuring on the way to root.
On Thu, Sep 29, 2022 at 10:21:04PM +0100, Al Viro wrote:
> On Thu, Sep 29, 2022 at 02:13:57PM -0700, Linus Torvalds wrote:
> > On Thu, Sep 29, 2022 at 12:34 PM Al Viro <[email protected]> wrote:
> > >
> > > Apparmor takes mount+dentry and turns that into pathname. Then acts
> > > upon the resulting string. *AFTER* the original had been resolved.
> >
> > Ok. So it would have to act like a bind mount.
> >
> > Which is probably not too bad.
> >
> > In fact, maybe it would be ok for this to act like a hardlink and just
> > fill in the inode - not safe for a filesystem in general due to the
> > whole rename loop issue, but for /proc it might be fine?
>
> _Which_ hardlink?
>
> Linus, where in dentry tree would you want it to be seen? Because
> apparmor profile wants /proc/net/dev to land at /proc/<pid>/net/dev
> and will fail with anything else.
>
> Do you really want multiple dentries with the same name in the same
> parent, refering to different directory inodes with different contents?
>
> And that's different inodes with different contents - David's complaint
> is precisely about seeing the same thing for all threads and apparmor
> issue is with *NOT* seeing each of those things at the same location.
Put it another way:
David:
when I'm opening /proc/net/whatever, I want its contents to match
this thread's netns, not that of some other thread.
dhclient+apparmor:
whatever you get from /proc/net/dev, it would better be at
/proc/<pid>/net/dev, no matter which thread you happen to be.
It's not that we want to see the same thing in several places; it's that
we want to see *different* things in the same place. Opposite to what
hardlinks or bindings would be about.
On Thu, Sep 29, 2022 at 2:27 PM Al Viro <[email protected]> wrote:
>
> Put it another way:
>
> David:
> when I'm opening /proc/net/whatever, I want its contents to match
> this thread's netns, not that of some other thread.
> dhclient+apparmor:
> whatever you get from /proc/net/dev, it would better be at
> /proc/<pid>/net/dev, no matter which thread you happen to be.
... which actually creates an opening for a truly disgusting solution:
- when an outsider else opens /proc/<pid>/net, they get the thread leader netns
- when a thread opens its *own* thread group /proc/<pid>/net, it gets
its own thread netns, not the thread leader one.
Disgusting.
Linus