Merge branch 'fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs
Pull umount fix from Al Viro: "Brown paperbag time: dumb braino in the series that went into 5.7 broke the 'don't step into ->d_weak_revalidate() when umount(2) looks the victim up' behaviour. Spotted only now - saw if (!err && unlikely(nd->flags & LOOKUP_MOUNTPOINT)) { err = handle_lookup_down(nd); nd->flags &= ~LOOKUP_JUMPED; // no d_weak_revalidate(), please... } and went "why do we clear that flag here - nothing below that point is going to check it anyway" / "wait a minute, what is it doing *after* complete_walk() (which is where we check that flag and call ->d_weak_revalidate())" / "how could that possibly _not_ break?", followed by reproducing the breakage and verifying that the obvious fix of that braino does, indeed, fix it. The reproducer is (assuming that $DIR exists and is exported r/w to localhost) mkdir $DIR/a mkdir /tmp/foo mount --bind /tmp/foo /tmp/foo mkdir /tmp/foo/a mkdir /tmp/foo/b mount -t nfs4 localhost:$DIR/a /tmp/foo/a mount -t nfs4 localhost:$DIR /tmp/foo/b rmdir /tmp/foo/b/a umount /tmp/foo/b umount /tmp/foo/a umount -l /tmp/foo # will get everything under /tmp/foo, no matter what Correct behaviour is successful umount; broken kernels (5.7-rc1 and later) get umount.nfs4: /tmp/foo/a: Stale file handle Note that bind mount is there to be able to recover - on broken kernels we'd get stuck with impossible-to-umount filesystem if not for that. FWIW, that braino had been posted for review back then, at least twice. Unfortunately, the call of complete_walk() was outside of diff context, so the bogosity hadn't been immediately obvious from the patch alone ;-/" * 'fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs: LOOKUP_MOUNTPOINT: we are cleaning "jumped" flag too late
This commit is contained in:
commit
035d80695f
@ -2421,16 +2421,16 @@ static int path_lookupat(struct nameidata *nd, unsigned flags, struct path *path
|
||||
while (!(err = link_path_walk(s, nd)) &&
|
||||
(s = lookup_last(nd)) != NULL)
|
||||
;
|
||||
if (!err && unlikely(nd->flags & LOOKUP_MOUNTPOINT)) {
|
||||
err = handle_lookup_down(nd);
|
||||
nd->flags &= ~LOOKUP_JUMPED; // no d_weak_revalidate(), please...
|
||||
}
|
||||
if (!err)
|
||||
err = complete_walk(nd);
|
||||
|
||||
if (!err && nd->flags & LOOKUP_DIRECTORY)
|
||||
if (!d_can_lookup(nd->path.dentry))
|
||||
err = -ENOTDIR;
|
||||
if (!err && unlikely(nd->flags & LOOKUP_MOUNTPOINT)) {
|
||||
err = handle_lookup_down(nd);
|
||||
nd->flags &= ~LOOKUP_JUMPED; // no d_weak_revalidate(), please...
|
||||
}
|
||||
if (!err) {
|
||||
*path = nd->path;
|
||||
nd->path.mnt = NULL;
|
||||
|
Loading…
Reference in New Issue
Block a user