[PATCH v2] nfsd: update reclaim client pointer under hash lock

[PATCH v2] nfsd: update reclaim client pointer under hash lock

[Hyunsol Mun]

__nfsd4_create_reclaim_record_grace() assigns cr_clp after
nfs4_client_to_reclaim() has released reclaim_str_hashtbl_lock.
Concurrent reclaim-record removal can free the returned record before
that assignment.

Pass the client into nfs4_client_to_reclaim() and install cr_clp while
its write lock is held, both for an existing record and for a newly
allocated record.

The vulnerable path requires the default-off legacy client-tracking backend
and startup grace. For deterministic validation, a timing-only kretprobe
module delayed the helper return while the administrator ended grace. The
module did not allocate, free, or modify the reclaim record.
It only widened the post-unlock interval; it is not a remote capability.

Validation used the Torvalds mainline base recorded below. On the
unmodified KASAN kernel, the legacy backend and a real startup grace period
were active, the validator hit the post-unlock boundary, and
RECLAIM_COMPLETE returned NFS4_OK. Generic KASAN then reported an
eight-byte slab-use-after-free write in nfsd4_create_clid_dir(), with
allocation in nfs4_client_to_reclaim() and freeing in
nfs4_release_reclaim().

With this patch alone applied to the same source, five identical runs each
hit the validator boundary and returned NFS4_OK without a KASAN report.
Basic NFSv4.2 and NFSv3 read/write/unmount smoke tests also passed. The
unmodified and patched full kernels were built with GCC 12.2 and
CONFIG_WERROR without a compiler warning. A source reproducer and timing
module source, together with the complete KASAN logs, are available
privately on request.

The vulnerability research and validation were conducted by members of
the Tobabz team as part of the Best of the Best 15th program.

Fixes: 7e4f015d815d ("nfsd: release the legacy reclaimable clients list in grace_done")
Assisted-by: LLM
Signed-off-by: Hyunsol Mun <muumthf@gmail.com>
---
Changes in v2:
- Revalidate against current Torvalds mainline.
- Expand the lifetime-race and runtime-validation details.
- No code changes.

 fs/nfsd/nfs4recover.c | 8 +++-----
 fs/nfsd/nfs4state.c   | 6 ++++--
 fs/nfsd/state.h       | 3 ++-
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
index d513971fb119..357b53a5b7c7 100644
--- a/fs/nfsd/nfs4recover.c
+++ b/fs/nfsd/nfs4recover.c
@@ -113,10 +113,8 @@ __nfsd4_create_reclaim_record_grace(struct nfs4_client *clp,
 {
 	struct xdr_netobj name = { .len = strlen(dname), .data = dname };
 	struct xdr_netobj princhash = { .len = 0, .data = NULL };
-	struct nfs4_client_reclaim *crp;
 
-	crp = nfs4_client_to_reclaim(name, princhash, nn);
-	crp->cr_clp = clp;
+	nfs4_client_to_reclaim(name, princhash, clp, nn);
 }
 
 static void
@@ -404,7 +402,7 @@ load_recdir(struct dentry *parent, char *cname, struct nfsd_net *nn)
 		/* Keep trying; maybe the others are OK: */
 		return 0;
 	}
-	nfs4_client_to_reclaim(name, princhash, nn);
+	nfs4_client_to_reclaim(name, princhash, NULL, nn);
 	return 0;
 }
 
@@ -761,7 +759,7 @@ __cld_pipe_inprogress_downcall(const struct cld_msg_v2 __user *cmsg,
 			cn->cn_has_legacy = true;
 		}
 #endif
-		if (!nfs4_client_to_reclaim(name, princhash, nn))
+		if (!nfs4_client_to_reclaim(name, princhash, NULL, nn))
 			return -EFAULT;
 		return nn->client_tracking_ops->msglen;
 	}
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 9c4adf3110ae..fd947208cd78 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -9577,7 +9577,7 @@ nfs4_has_reclaimed_state(struct xdr_netobj name, struct nfsd_net *nn)
  */
 struct nfs4_client_reclaim *
 nfs4_client_to_reclaim(struct xdr_netobj name, struct xdr_netobj princhash,
-		struct nfsd_net *nn)
+		struct nfs4_client *clp, struct nfsd_net *nn)
 {
 	unsigned int strhashval;
 	struct nfs4_client_reclaim *crp;
@@ -9606,6 +9606,8 @@ nfs4_client_to_reclaim(struct xdr_netobj name, struct xdr_netobj princhash,
 				crp = NULL;
 			}
 		}
+		if (crp && clp)
+			crp->cr_clp = clp;
 		up_write(&nn->reclaim_str_hashtbl_lock);
 		return crp;
 	}
@@ -9637,7 +9639,7 @@ nfs4_client_to_reclaim(struct xdr_netobj name, struct xdr_netobj princhash,
 		crp->cr_name.len = name.len;
 		crp->cr_princhash.data = princhash.data;
 		crp->cr_princhash.len = princhash.len;
-		crp->cr_clp = NULL;
+		crp->cr_clp = clp;
 		nn->reclaim_str_hashtbl_size++;
 	} else {
 		kfree(name.data);
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 2d00a411c663..45324734e38c 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -921,7 +921,8 @@ void nfsd4_async_copy_reaper(struct nfsd_net *nn);
 bool nfsd4_has_active_async_copies(struct nfs4_client *clp);
 void nfsd_update_cmtime_attr(struct file *f, unsigned int flags);
 extern struct nfs4_client_reclaim *nfs4_client_to_reclaim(struct xdr_netobj name,
-				struct xdr_netobj princhash, struct nfsd_net *nn);
+				struct xdr_netobj princhash,
+				struct nfs4_client *clp, struct nfsd_net *nn);
 extern bool nfs4_has_reclaimed_state(struct xdr_netobj name, struct nfsd_net *nn);
 int nfsd_handle_dir_event(u32 mask, const struct inode *dir, const void *data,
 			  int data_type, const struct qstr *name);

base-commit: fd179f8a05be3ccae366b9b96e176b51fbe54aab
-- 
2.39.5

Re: [PATCH v2] nfsd: update reclaim client pointer under hash lock

[Chuck Lever]

Hello Hyunsol -

On Wed, Sep 30, 2026 at 02:40:32PM +0900, Hyunsol Mun wrote:
> __nfsd4_create_reclaim_record_grace() assigns cr_clp after
> nfs4_client_to_reclaim() has released reclaim_str_hashtbl_lock.
> Concurrent reclaim-record removal can free the returned record before
> that assignment.
>
> Pass the client into nfs4_client_to_reclaim() and install cr_clp while
> its write lock is held, both for an existing record and for a newly
> allocated record.

This patch does not apply to nfsd-testing. The first hunk in
fs/nfsd/nfs4recover.c fails because nfsd-testing carries these two
commits:

  nfsd: remove the nfsdcltrack usermodehelper tracking backend
  nfsd: remove CONFIG_NFSD_LEGACY_CLIENT_TRACKING

The second one deletes __nfsd4_create_reclaim_record_grace() and
load_recdir(), along with the rest of the legacy recovery directory
backend. The function with the post-unlock assignment no longer
exists.

So I can't take this patch for nfsd-next: there is nothing left for it
to fix there.

-- 
Chuck Lever (Come to NFS bake-a-thon! https://nfsv4bat.org)