On Fri, 2023-01-06 at 17:56 +0000, Chuck Lever III wrote:

quoted

On Jan 5, 2023, at 2:55 PM, Jeff Layton [off-list ref] wrote:

Commit fb70bf124b05 ("NFSD: Instantiate a struct file when creating a
regular NFSv4 file") added the ability to cache an open fd over a
compound. There are a couple of problems with the way this currently
works:

It's racy, as a newly-created nfsd_file can end up with its PENDING bit
cleared while the nf is hashed, and the nf_file pointer is still zeroed
out. Other tasks can find it in this state and they expect to see a
valid nf_file, and can oops if nf_file is NULL.

Also, there is no guarantee that we'll end up creating a new nfsd_file
if one is already in the hash. If an extant entry is in the hash with a
valid nf_file, nfs4_get_vfs_file will clobber its nf_file pointer with
the value of op_file and the old nf_file will leak.

Fix both issues by making a new nfsd_file_acquirei_opened variant that
takes an optional file pointer. If one is present when this is called,
we'll take a new reference to it instead of trying to open the file. If
the nfsd_file already has a valid nf_file, we'll just ignore the
optional file and pass the nfsd_file back as-is.

Also rework the tracepoints a bit to allow for an "opened" variant and
don't try to avoid counting acquisitions in the case where we already
have a cached open file.

Fixes: fb70bf124b05 ("NFSD: Instantiate a struct file when creating a regular NFSv4 file")
Cc: Trond Myklebust <redacted>
Reported-by: Stanislav Saner <redacted>
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
fs/nfsd/filecache.c | 40 ++++++++++++++++++----------------
fs/nfsd/filecache.h |  5 +++--
fs/nfsd/nfs4state.c | 16 ++++----------
fs/nfsd/trace.h     | 52 ++++++++++++---------------------------------
4 files changed, 42 insertions(+), 71 deletions(-)

v3: add new nfsd_file_acquire_opened variant instead of changing
   nfsd_file_acquire prototype

Hi Jeff, v3 applied to nfsd's for-rc.

Can you also add:

    Reported-and-Tested-by: Ruben Vestergaard    rubenv@drcmr.dk
    Reported-and-Tested-by: Torkil Svensgaard    torkil@drcmr.dk

They've been helping test these patches (albeit backported versions on a
RHEL9 kernel), and reported the crash that helped me track this down.

Many thanks!
-- 
Jeff Layton [off-list ref]