Re: [PATCH] nfsd: fix potential race in nfs4_find_file
From: Chuck Lever III <hidden>
Date: 2023-01-05 14:47:38
quoted hunk ↗ jump to hunk
On Jan 5, 2023, at 7:18 AM, Jeff Layton [off-list ref] wrote: Even though there is a WARN_ON_ONCE check, it seems possible for nfs4_find_file to race with the destruction of an fi_deleg_file while trying to take a reference to it. put_deleg_file is done while holding the fi_lock. Take and hold it when dealing with the fi_deleg_file in nfs4_find_file. Signed-off-by: Jeff Layton <jlayton@kernel.org> --- fs/nfsd/nfs4state.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index b68238024e49..3df3ae84bd07 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c@@ -6417,23 +6417,27 @@ nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,static struct nfsd_file * nfs4_find_file(struct nfs4_stid *s, int flags) { + struct nfsd_file *ret = NULL; + if (!s) return NULL; switch (s->sc_type) { case NFS4_DELEG_STID: - if (WARN_ON_ONCE(!s->sc_file->fi_deleg_file)) - return NULL; - return nfsd_file_get(s->sc_file->fi_deleg_file); + spin_lock(&s->sc_file->fi_lock); + if (!WARN_ON_ONCE(!s->sc_file->fi_deleg_file))
You'd think this would be a really really hard race to hit. What I'm wondering, though, is whether the WARN_ON_ONCE should be dropped by this patch. I've never seen it fire.
+ ret = nfsd_file_get(s->sc_file->fi_deleg_file); + spin_unlock(&s->sc_file->fi_lock); + break; case NFS4_OPEN_STID: case NFS4_LOCK_STID: if (flags & RD_STATE) - return find_readable_file(s->sc_file); + ret = find_readable_file(s->sc_file); else - return find_writeable_file(s->sc_file); + ret = find_writeable_file(s->sc_file); } - return NULL; + return ret; } static __be32 -- 2.39.0
-- Chuck Lever