BTW, this "just use nd->path.mnt" is *not* guaranteed to work - sadly,
it's possible to get a _symlink_ bound somewhere. I'm not sure whether
we should allow that or not, but as it is that's doable. And with
your ->follow_link() that means ending up with (vfsmount,dentry) pair
that has dentry completely unrelated to vfsmount. The thing is,
nd->path is where we would start the traversal for a normal symlink, i.e.
the directory where that thing had been found. Which almost always gets
covered by the vfsmount as the symlink itself, with one exception - when
that symlink is bound there.
_Normally_ namei can cope with that just fine - if that's a normal symlink,
everything ends up working and we only have to take a bit of care about
link->mnt in fs/namei.c:follow_link(). With procfs-style symlinks it
also happens to work. With one exception - /proc/<pid>/ns/*
Hell knows; my preference would be to ban that kind of crap, especially
since it comes from a rather convoluted loophole...
BTW, Linus, remember the thread back in March when I was grumbling about
procfs-symlinks-to-symlinks? We ended up killing that BUG_ON() in
nd_jump_link() and back then I hadn't found sufficiently convincing breakage
to require failing such suckers with -ELOOP. Looks like now I have a
reasonably good argument - pathname lookups with LOOKUP_FOLLOW should _not_
yield symlinks. That's the loophole refered to above...
If you have happily flushed that memory, the problem was that following
a procfs symlink gives us location in the tree, but that's not enough
when that location itself happens to be a symlink; to resolve a normal
symlink we need both the symlink itself *and* where had it been found.
When we'd arrived to that symlink by following a procfs-style symlink,
the second part is missing.
Now, normally that's not an issue - to hit that case you need O_PATH|O_NOFOLLOW
open of a symlink (giving you an O_PATH descriptor of symlink itself) and
try to follow /proc/<pid>/fd/<that descriptor>. No other way to get that
kind of symlink-to-symlink in procfs. I argued for ELOOP on following
those guys; you replied with
<quote>
Why? That's what I did last week, it seemed to be the RightThing(tm)
to do. It somebody has opened a symlink, opening that again through
/proc gives you the same symlink. That would be the natural semantics,
no? You do *not* want to follow it any further, afaik.
</quote>
The trouble is, there is code assuming that LOOKUP_FOLLOW pathname
resolution will stop at non-symlink or fail. One such example is
do_loopback(), aka. mount --bind old new, which uses LOOKUP_FOLLOW to
resolve old. However, using the trick above (open a symlink with
O_PATH|O_NOFOLLOW, then pass /proc/self/fd/<descriptor> to mount(2)
with MS_BIND in flags) you can actually have it yield a symlink.
It mostly works (Eric's proc_ns_follow_link() is actually the only
instance that has problems with that setup), but it's certainly
unexpected.
And I'm not at all sure that we have no other place using LOOKUP_FOLLOW
and breaking if what it gets is a symlink... I can buy your argument
about opening them (O_PATH open of /proc/self/fd/something yields
O_PATH descriptor of whatever's opened there, and if it's a symlink,
so be it - you've asked for it, you've got it), but I would like to have
path_lookupat() with LOOKUP_FOLLOW fail if it's ended up in a symlink.
With ELOOP, probably.
I agree that nd_jump_link() is not a good place for that; I would add
if (!err && nd->flags & LOOKUP_FOLLOW) {
if (unlikely(d_is_symlink(nd->path.dentry))) {
path_put(&nd->path);
err = -ELOOP;
}
}
next to similar check for LOOKUP_DIRECTORY in path_lookupat(). Objections?
_______________________________________________
Containers mailing list
Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/containers
