| Message ID | 20240204021739.1157830-7-viro@zeniv.linux.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/13] fs/super.c: don't drop ->s_user_ns until we free struct super_block itself | expand |
On Sun, Feb 04, 2024 at 02:17:33AM +0000, Al Viro wrote: > nfs_set_verifier() relies upon dentry being pinned; if that's > the case, grabbing ->d_lock stabilizes ->d_parent and guarantees > that ->d_parent points to a positive dentry. For something > we'd run into in RCU mode that is *not* true - dentry might've > been through dentry_kill() just as we grabbed ->d_lock, with > its parent going through the same just as we get to into > nfs_set_verifier_locked(). It might get to detaching inode > (and zeroing ->d_inode) before nfs_set_verifier_locked() gets > to fetching that; we get an oops as the result. > > That can happen in nfs{,4} ->d_revalidate(); the call chain in > question is nfs_set_verifier_locked() <- nfs_set_verifier() <- > nfs_lookup_revalidate_delegated() <- nfs{,4}_do_lookup_revalidate(). > We have checked that the parent had been positive, but that's > done before we get to nfs_set_verifier() and it's possible for > memory pressure to pick our dentry as eviction candidate by that > time. If that happens, back-to-back attempts to kill dentry and > its parent are quite normal. Sure, in case of eviction we'll > fail the ->d_seq check in the caller, but we need to survive > until we return there... > > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> > --- Acked-by: Christian Brauner <brauner@kernel.org>
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c index c8ecbe999059..ac505671efbd 100644 --- a/fs/nfs/dir.c +++ b/fs/nfs/dir.c @@ -1431,9 +1431,9 @@ static bool nfs_verifier_is_delegated(struct dentry *dentry) static void nfs_set_verifier_locked(struct dentry *dentry, unsigned long verf) { struct inode *inode = d_inode(dentry); - struct inode *dir = d_inode(dentry->d_parent); + struct inode *dir = d_inode_rcu(dentry->d_parent); - if (!nfs_verify_change_attribute(dir, verf)) + if (!dir || !nfs_verify_change_attribute(dir, verf)) return; if (inode && NFS_PROTO(inode)->have_delegation(inode, FMODE_READ)) nfs_set_verifier_delegated(&verf);
nfs_set_verifier() relies upon dentry being pinned; if that's the case, grabbing ->d_lock stabilizes ->d_parent and guarantees that ->d_parent points to a positive dentry. For something we'd run into in RCU mode that is *not* true - dentry might've been through dentry_kill() just as we grabbed ->d_lock, with its parent going through the same just as we get to into nfs_set_verifier_locked(). It might get to detaching inode (and zeroing ->d_inode) before nfs_set_verifier_locked() gets to fetching that; we get an oops as the result. That can happen in nfs{,4} ->d_revalidate(); the call chain in question is nfs_set_verifier_locked() <- nfs_set_verifier() <- nfs_lookup_revalidate_delegated() <- nfs{,4}_do_lookup_revalidate(). We have checked that the parent had been positive, but that's done before we get to nfs_set_verifier() and it's possible for memory pressure to pick our dentry as eviction candidate by that time. If that happens, back-to-back attempts to kill dentry and its parent are quite normal. Sure, in case of eviction we'll fail the ->d_seq check in the caller, but we need to survive until we return there... Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> --- fs/nfs/dir.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)