On Tue, 22 Apr 2025, trondmy@xxxxxxxxxx wrote:
> From: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
>
> Once the clp->cl_uuid.lock has been dropped, another CPU could come in
> and free the struct nfsd_file that was just added. To prevent that from
> happening, take the RCU read lock before dropping the spin lock.
I think there is a race here but I think the better fix would be to use
nfs_local_file_get() to get an extra reference earlier. That ensures we
won't lose the nfsd_file.
I'm working on a patch in this area which I hope to post soon. It will
address this.
Thanks,
NeilBrown
>
> Fixes: 86e00412254a ("nfs: cache all open LOCALIO nfsd_file(s) in client")
> Signed-off-by: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
> ---
> fs/nfs/localio.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/nfs/localio.c b/fs/nfs/localio.c
> index 5c21caeae075..4ec952f9f47d 100644
> --- a/fs/nfs/localio.c
> +++ b/fs/nfs/localio.c
> @@ -278,6 +278,7 @@ nfs_local_open_fh(struct nfs_client *clp, const struct cred *cred,
> new = __nfs_local_open_fh(clp, cred, fh, nfl, mode);
> if (IS_ERR(new))
> return NULL;
> + rcu_read_lock();
> /* try to swap in the pointer */
> spin_lock(&clp->cl_uuid.lock);
> nf = rcu_dereference_protected(*pnf, 1);
> @@ -287,7 +288,6 @@ nfs_local_open_fh(struct nfs_client *clp, const struct cred *cred,
> rcu_assign_pointer(*pnf, nf);
> }
> spin_unlock(&clp->cl_uuid.lock);
> - rcu_read_lock();
> }
> nf = nfs_local_file_get(nf);
> rcu_read_unlock();
> --
> 2.49.0
>
>
>
