nfsd: more bugfixes

This version is addresses review comments from Sashiko. There wasn't much in the way of regressions in v1, but it did notice a number of nearby problems that should also be fixed.

This adds 3 patches to fix those. I also dropped the localio patch from the series since that should probably go through the NFS client tree. I'll send it separately.

AFAICT, these are garden-variety bugs. Chuck, please consider these for v7.3.

---
Changes in v2:
- Add 3 new patches for nearby bugs sashiko noticed while reviewing v1
- Clean up unwinding when server startup fails
- Link to v1: https://lore.kernel.org/r/[email protected]

---
Jeff Layton (21):
nfsd: clear opcnt on compound arg release to prevent OOB read
nfsd: add missing read barrier to rpc_status_get dumpit seqcount retry
nfsd: fix netlink dumpit error handling for rpc_status_get
sunrpc: defer rq_argp and rq_resp free until after RCU grace period
nfsd: check nfsd4_acl_to_attr() return value in nfsd4_create()
nfsd: add filehandle match check to nfsd4_delegreturn()
nfsd: validate nseconds in TIME_DELEG decode paths
nfsd: remove premature NFS4_OO_CONFIRMED in CLAIM_PREVIOUS path
nfsd: fix version mismatch loops in nfsd_acl_init_request()
nfsd: fix FL_SLEEP being set unconditionally for all LOCK types
nfsd: add fh_want_write() for early-verified SETATTR in nfsd_proc_setattr()
nfsd: fix clock domain mismatch in clients_still_reclaiming()
nfsd: use test_and_clear_bit for somebody_reclaimed to prevent lost update
nfsd: reject reclaim LOCK after RECLAIM_COMPLETE
nfsd: validate sockaddr length per family in listener_set lockd, nfsd: RCU-protect nlmsvc_ops dispatch
nfsd: move nfsd_debugfs_init() after nfsd4_init_slabs() in init_nfsd()
nfsd: initialize DRC hash table before registering shrinker
nfsd: restore rq_status_counter to even on all nfsd_dispatch() exit paths
nfsd: reset thread skip index when advancing pools in rpc_status dumpit
nfsd: drop the stateid, not the stateowner, on seqid_op replay retry

 Documentation/netlink/specs/nfsd.yaml |  4 ++
 fs/lockd/svc.c                        |  4 +-
 fs/lockd/svc4proc.c                   |  4 +-
 fs/lockd/svcproc.c                    |  4 +-
 fs/lockd/svcsubs.c                    | 52 +++++++++++++++++++----
 fs/nfsd/lockd.c                       |  6 ++-
 fs/nfsd/netlink.c                     |  2 +-
 fs/nfsd/netns.h                       |  1 +
 fs/nfsd/nfs4callback.c                |  4 ++
 fs/nfsd/nfs4proc.c                    |  3 +-
 fs/nfsd/nfs4state.c                   | 24 +++++++----
 fs/nfsd/nfs4xdr.c                     |  5 +++
 fs/nfsd/nfscache.c                    |  4 +-
 fs/nfsd/nfsctl.c                      | 77 +++++++++++++++++++++++++++++------
 fs/nfsd/nfsproc.c                     |  7 ++++
 fs/nfsd/nfssvc.c                      | 27 ++++++++----
 include/linux/lockd/bind.h            | 12 ++++--
 net/sunrpc/svc.c                      | 13 ++++--
 18 files changed, 200 insertions(+), 53 deletions(-)

---
base-commit: 8defc3ed26a2b4c8677ce2106c2c92cd26ef1316
change-id: 20260608-nfsd-testing-688a82433c50

Best regards,

Jeff Layton <[email protected]>

nfsd4_release_compoundargs() resets args->ops to the inline iops[8] array when the dynamically-allocated ops buffer is freed, but leaves args->opcnt at its original value (which can be up to 200 for NFSv4.1+ compounds).

If rq_status_counter is stuck at an odd value (which can happen when nfsd_dispatch() hits an error path after setting it odd), the RPC status dumpit handler reads min(opcnt, 16) entries from args->ops[]. Since iops only has 8 elements and is the last field in struct nfsd4_compoundargs, reading indices 8-15 accesses adjacent slab memory and leaks it to userspace via netlink.

Zero opcnt unconditionally in nfsd4_release_compoundargs() so stale compound metadata is never exposed through the status interface.

---
 fs/nfsd/nfs4xdr.c | 1 +
 1 file changed, 1 insertion(+)

index b9037d99b564..1e4a51926910 100644
@@ -6440,6 +6440,7 @@ void nfsd4_release_compoundargs(struct svc_rqst *rqstp)
 		args->ops = args->iops;
 		kvfree_rcu_mightsleep(old_ops);
 	}
+	args->opcnt = 0;
 	while (args->to_free) {
 		struct svcxdr_tmpbuf *tb = args->to_free;
 		args->to_free = tb->next;

2.54.0

The hand-rolled seqcount-like protocol in nfsd_nl_rpc_status_get_dumpit() is missing a read memory barrier (smp_rmb) before its second counter check. The standard kernel read_seqcount_retry() includes smp_rmb() to ensure that all data reads complete before the counter is re-checked.

Without this barrier, on weakly-ordered architectures (ARM, POWER), the CPU may reorder field reads past the second counter check, making the retry logic ineffective: it could observe a consistent counter pair while reading fields that have been concurrently modified by the writer.

Add smp_rmb() before the second smp_load_acquire() to match the barrier semantics of the standard seqcount read-side.

---
 fs/nfsd/nfsctl.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

index c06d25c06f06..a4b5b1467fe2 100644
@@ -1576,9 +1576,11 @@ int nfsd_nl_rpc_status_get_dumpit(struct sk_buff *skb,
 #endif /* CONFIG_NFSD_V4 */
 
 			/*
-			 * Acquire rq_status_counter before reporting the rqst
-			 * fields to the user.
+			 * Ensure all field reads complete before re-checking
+			 * the status counter. Pairs with the smp_store_release
+			 * in nfsd_dispatch to form a seq-lock like protocol.
 			 */
+			smp_rmb();
 			if (smp_load_acquire(&rqstp->rq_status_counter) !=
 			    status_counter)
 				continue;

2.54.0

nfsd_genl_rpc_status_compose_msg() returns -ENOBUFS on nla_put failure without calling genlmsg_cancel(), leaving a partial message in the skb. The caller then propagates -ENOBUFS directly, which the netlink dump infrastructure treats as a fatal error, aborting the entire dump.

The correct netlink dump convention is:
- Cancel any partial message with genlmsg_cancel()
- If prior messages were added to the skb (skb->len > 0), save the current iterator position and return skb->len to paginate
- Only return a negative errno when no messages fit at all

Fix compose_msg to cancel the partial message on all nla_put failure paths, and fix the caller to paginate when possible rather than returning a fatal error.

---
 fs/nfsd/nfsctl.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

index a4b5b1467fe2..ab10692ee937 100644
@@ -1452,7 +1452,7 @@ static int nfsd_genl_rpc_status_compose_msg(struct sk_buff *skb,
 	    nla_put_s64(skb, NFSD_A_RPC_STATUS_SERVICE_TIME,
 			ktime_to_us(genl_rqstp->rq_stime),
 			NFSD_A_RPC_STATUS_PAD))
-		return -ENOBUFS;
+		goto out_cancel;
 
 	switch (genl_rqstp->rq_saddr.ss_family) {
 	case AF_INET: {

@@ -1468,7 +1468,7 @@ static int nfsd_genl_rpc_status_compose_msg(struct sk_buff *skb,
 				 s_in->sin_port) ||
 		    nla_put_be16(skb, NFSD_A_RPC_STATUS_DPORT,
 				 d_in->sin_port))
-			return -ENOBUFS;
+			goto out_cancel;
 		break;
 	}
 	case AF_INET6: {

@@ -1484,7 +1484,7 @@ static int nfsd_genl_rpc_status_compose_msg(struct sk_buff *skb,
 				 s_in->sin6_port) ||
 		    nla_put_be16(skb, NFSD_A_RPC_STATUS_DPORT,
 				 d_in->sin6_port))
-			return -ENOBUFS;
+			goto out_cancel;
 		break;
 	}
 	}

@@ -1492,10 +1492,14 @@ static int nfsd_genl_rpc_status_compose_msg(struct sk_buff *skb,
 	for (i = 0; i < genl_rqstp->rq_opcnt; i++)
 		if (nla_put_u32(skb, NFSD_A_RPC_STATUS_COMPOUND_OPS,
 				genl_rqstp->rq_opnum[i]))
-			return -ENOBUFS;
+			goto out_cancel;
 
 	genlmsg_end(skb, hdr);
 	return 0;
+
+out_cancel:
+	genlmsg_cancel(skb, hdr);
+	return -ENOBUFS;
 }
 
 /**

@@ -1587,8 +1591,14 @@ int nfsd_nl_rpc_status_get_dumpit(struct sk_buff *skb,
 
 			ret = nfsd_genl_rpc_status_compose_msg(skb, cb,
 							       &genl_rqstp);
-			if (ret)
+			if (ret) {
+				if (skb->len) {
+					cb->args[0] = i;
+					cb->args[1] = rqstp_index - 1;
+					ret = skb->len;
+				}
 				goto out;
+			}
 		}
 	}
 

2.54.0

svc_rqst_free() frees rqstp->rq_argp and rqstp->rq_resp synchronously via kfree(), but defers the rqstp struct free via kfree_rcu(). After svc_exit_thread() calls list_del_rcu() and svc_rqst_free(), there is a window where RCU readers that started before list_del_rcu() can still traverse the thread list and find the rqstp. These readers (e.g. nfsd_nl_rpc_status_get_dumpit()) dereference rqstp->rq_argp, which has already been freed — a use-after-free.

Fix this by moving the kfree of rq_argp and rq_resp into an explicit call_rcu() callback alongside the struct free. Resources not accessed by RCU readers (bvec, buffer pages, scratch folio, auth_data) remain synchronously freed.

---
 net/sunrpc/svc.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

index 86d39610cf0a..dd80a2eaaa74 100644
@@ -716,6 +716,15 @@ svc_release_buffer(struct svc_rqst *rqstp)
 	}
 }
 
+static void svc_rqst_free_rcu(struct rcu_head *head)
+{
+	struct svc_rqst *rqstp = container_of(head, struct svc_rqst, rq_rcu_head);
+
+	kfree(rqstp->rq_resp);
+	kfree(rqstp->rq_argp);
+	kfree(rqstp);
+}
+
 static void
 svc_rqst_free(struct svc_rqst *rqstp)
 {

@@ -724,10 +733,8 @@ svc_rqst_free(struct svc_rqst *rqstp)
 	svc_release_buffer(rqstp);
 	if (rqstp->rq_scratch_folio)
 		folio_put(rqstp->rq_scratch_folio);
-	kfree(rqstp->rq_resp);
-	kfree(rqstp->rq_argp);
 	kfree(rqstp->rq_auth_data);
-	kfree_rcu(rqstp, rq_rcu_head);
+	call_rcu(&rqstp->rq_rcu_head, svc_rqst_free_rcu);
 }
 
 static struct svc_rqst *

2.54.0

nfsd4_create() stores the return value of nfsd4_acl_to_attr() in status, but the switch(create->cr_type) block unconditionally overwrites it in every branch. ACL translation errors are silently discarded, and the CREATE proceeds without the requested ACL.

Add an early exit check after nfsd4_acl_to_attr(), matching the pattern already used in nfsd4_setattr().

---
 fs/nfsd/nfs4proc.c | 2 ++
 1 file changed, 2 insertions(+)

index 0c37d7c6d28c..69fee481581d 100644
@@ -855,6 +855,8 @@ nfsd4_create(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		}
 		status = nfsd4_acl_to_attr(create->cr_type, create->cr_acl,
 								&attrs);
+		if (status != nfs_ok)
+			goto out_aftermask;
 	}
 	current->fs->umask = create->cr_umask;
 	switch (create->cr_type) {

2.54.0

nfsd4_delegreturn() is the only stateful NFSv4 operation that does not call nfs4_check_fh() to verify the delegation's file matches cstate->current_fh. A client can DELEGRETURN with a mismatched filehandle, destroying the correct delegation but waking the wrong inode's waiters.

Add the missing nfs4_check_fh() call after the generation check.

---
 fs/nfsd/nfs4state.c | 4 ++++
 1 file changed, 4 insertions(+)

index e59aec57e9e8..eb832e996364 100644
@@ -8126,6 +8126,10 @@ nfsd4_delegreturn(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	if (status)
 		goto put_stateid;
 
+	status = nfs4_check_fh(&cstate->current_fh, &dp->dl_stid);
+	if (status)
+		goto put_stateid;
+
 	trace_nfsd_deleg_return(stateid);
 	destroy_delegation(dp);
 	smp_mb__after_atomic();

2.54.0

The xdrgen-based TIME_DELEG_ACCESS and TIME_DELEG_MODIFY decode arms store a raw uint32_t nseconds directly into tv_nsec without enforcing nseconds < NSEC_PER_SEC. The legacy nfsd4_decode_nfstime4 has this check but the TIME_DELEG paths do not. A malformed timespec can propagate through notify_change() to disk.

Add range checks in both nfs4xdr.c (SETATTR path) and nfs4callback.c (CB_GETATTR path).

---
 fs/nfsd/nfs4callback.c | 4 ++++
 fs/nfsd/nfs4xdr.c      | 4 ++++
 2 files changed, 8 insertions(+)

index 1628bb9ef9dd..7c868afc329e 100644
@@ -108,6 +108,8 @@ static int decode_cb_fattr4(struct xdr_stream *xdr, uint32_t *bitmap,
 
 		if (!xdrgen_decode_fattr4_time_deleg_access(xdr, &access))
 			return -EIO;
+		if (access.nseconds >= NSEC_PER_SEC)
+			return -EIO;
 		fattr->ncf_cb_atime.tv_sec = access.seconds;
 		fattr->ncf_cb_atime.tv_nsec = access.nseconds;
 

@@ -117,6 +119,8 @@ static int decode_cb_fattr4(struct xdr_stream *xdr, uint32_t *bitmap,
 
 		if (!xdrgen_decode_fattr4_time_deleg_modify(xdr, &modify))
 			return -EIO;
+		if (modify.nseconds >= NSEC_PER_SEC)
+			return -EIO;
 		fattr->ncf_cb_mtime.tv_sec = modify.seconds;
 		fattr->ncf_cb_mtime.tv_nsec = modify.nseconds;
 

index 1e4a51926910..056a8df3fd50 100644
@@ -637,6 +637,8 @@ nfsd4_decode_fattr4(struct nfsd4_compoundargs *argp, u32 *bmval, u32 bmlen,
 
 		if (!xdrgen_decode_fattr4_time_deleg_access(argp->xdr, &access))
 			return nfserr_bad_xdr;
+		if (access.nseconds >= NSEC_PER_SEC)
+			return nfserr_inval;
 		iattr->ia_atime.tv_sec = access.seconds;
 		iattr->ia_atime.tv_nsec = access.nseconds;
 		iattr->ia_valid |= ATTR_ATIME | ATTR_ATIME_SET | ATTR_DELEG;

@@ -646,6 +648,8 @@ nfsd4_decode_fattr4(struct nfsd4_compoundargs *argp, u32 *bmval, u32 bmlen,
 
 		if (!xdrgen_decode_fattr4_time_deleg_modify(argp->xdr, &modify))
 			return nfserr_bad_xdr;
+		if (modify.nseconds >= NSEC_PER_SEC)
+			return nfserr_inval;
 		iattr->ia_mtime.tv_sec = modify.seconds;
 		iattr->ia_mtime.tv_nsec = modify.nseconds;
 		iattr->ia_ctime.tv_sec = modify.seconds;

2.54.0

nfsd4_open() sets NFS4_OO_CONFIRMED on the openowner before calling do_open_fhandle(), which can fail. If it fails, the openowner stays permanently confirmed despite the OPEN failing. The correct success-path setter already exists in init_open_stateid().

Remove the premature setter. NFSv4.1+ is unaffected as sessions always confirm at creation time.

---
 fs/nfsd/nfs4proc.c | 1 -
 1 file changed, 1 deletion(-)

index 69fee481581d..4fe46996c8ed 100644
@@ -643,7 +643,6 @@ nfsd4_open(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		status = nfs4_check_open_reclaim(cstate->clp);
 		if (status)
 			goto out;
-		open->op_openowner->oo_flags |= NFS4_OO_CONFIRMED;
 		reclaim = true;
 		fallthrough;
 	case NFS4_OPEN_CLAIM_FH:

2.54.0

The loops that compute the supported version range for PROG_MISMATCH test nfsd_support_acl_version(rqstp->rq_vers) instead of nfsd_support_acl_version(i), so every iteration fails and the function returns rpc_prog_unavail instead of rpc_prog_mismatch.

Replace rqstp->rq_vers with the loop variable i, matching the pattern used by the sibling nfsd_init_request() function.

---
 fs/nfsd/nfssvc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

index 0d3838dd59c1..b8e8d80e984c 100644
@@ -821,7 +821,7 @@ nfsd_acl_init_request(struct svc_rqst *rqstp,
 
 	ret->mismatch.lovers = NFSD_ACL_NRVERS;
 	for (i = NFSD_ACL_MINVERS; i < NFSD_ACL_NRVERS; i++) {
-		if (nfsd_support_acl_version(rqstp->rq_vers) &&
+		if (nfsd_support_acl_version(i) &&
 		    nfsd_vers(nn, i, NFSD_TEST)) {
 			ret->mismatch.lovers = i;
 			break;

@@ -831,7 +831,7 @@ nfsd_acl_init_request(struct svc_rqst *rqstp,
 		return rpc_prog_unavail;
 	ret->mismatch.hivers = NFSD_ACL_MINVERS;
 	for (i = NFSD_ACL_NRVERS - 1; i >= NFSD_ACL_MINVERS; i--) {
-		if (nfsd_support_acl_version(rqstp->rq_vers) &&
+		if (nfsd_support_acl_version(i) &&
 		    nfsd_vers(nn, i, NFSD_TEST)) {
 			ret->mismatch.hivers = i;
 			break;

2.54.0

The FL_SLEEP guard uses lk_type & (NFS4_READW_LT | NFS4_WRITEW_LT) which computes lk_type & 7, non-zero for all valid lock types including non-blocking ones. This was introduced by commit 7e64c5bc497c ("NLM/NFSD: Fix lock notifications for async-capable filesystems") when refactoring from per-case switch arms.

Replace the bitmask test with explicit equality checks.

---
 fs/nfsd/nfs4state.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

index eb832e996364..3dc0c0f6eb5d 100644
@@ -8636,10 +8636,11 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		goto out;
 	}
 
-	if (lock->lk_type & (NFS4_READW_LT | NFS4_WRITEW_LT) &&
-		nfsd4_has_session(cstate) &&
-		locks_can_async_lock(nf->nf_file->f_op))
-			flags |= FL_SLEEP;
+	if ((lock->lk_type == NFS4_READW_LT ||
+	     lock->lk_type == NFS4_WRITEW_LT) &&
+	    nfsd4_has_session(cstate) &&
+	    locks_can_async_lock(nf->nf_file->f_op))
+		flags |= FL_SLEEP;
 
 	nbl = find_or_allocate_block(lock_sop, &fp->fi_fhandle, nn);
 	if (!nbl) {

2.54.0

The BOTH_TIME_SET branch calls fh_verify() early so setattr_prepare() can inspect the dentry. This causes nfsd_setattr() to skip fh_want_write(), so notify_change() runs without a mount write reference.

Add the missing fh_want_write() call after the early fh_verify().

---
 fs/nfsd/nfsproc.c | 7 +++++++
 1 file changed, 7 insertions(+)

index 8873033d1e82..a73d5c259cd9 100644
@@ -82,6 +82,7 @@ nfsd_proc_setattr(struct svc_rqst *rqstp)
 		.na_iattr	= iap,
 	};
 	struct svc_fh *fhp;
+	int hosterr;
 
 	dprintk("nfsd: SETATTR  %s, valid=%x, size=%ld\n",
 		SVCFH_fmt(&argp->fh),

@@ -117,6 +118,12 @@ nfsd_proc_setattr(struct svc_rqst *rqstp)
 		if (resp->status != nfs_ok)
 			goto out;
 
+		hosterr = fh_want_write(fhp);
+		if (hosterr) {
+			resp->status = nfserrno(hosterr);
+			goto out;
+		}
+
 		if (delta < 0)
 			delta = -delta;
 		if (delta < MAX_TOUCH_TIME_ERROR &&

2.54.0

clients_still_reclaiming() computes a deadline from nn->boot_time (CLOCK_REALTIME, ~1.7 billion) but compares it against ktime_get_boottime_seconds() (CLOCK_BOOTTIME, seconds since boot). The comparison is always false — it would take ~54 years of uptime for BOOTTIME to exceed the REALTIME-derived deadline.

This means any client can hold the server in grace indefinitely by sending CLAIM_PREVIOUS OPEN requests, blocking all non-reclaim operations for all other clients.

Add boot_time_bt (CLOCK_BOOTTIME) alongside the existing boot_time and use it for the deadline computation. boot_time (CLOCK_REALTIME) is preserved for its cl_boot clientid-nonce role.

---
 fs/nfsd/netns.h     | 1 +
 fs/nfsd/nfs4state.c | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

index 5c33c96da28e..03724bef10a7 100644
@@ -78,6 +78,7 @@ struct nfsd_net {
 	struct lock_manager nfsd4_manager;
 	unsigned long flags;
 	time64_t boot_time;
+	time64_t boot_time_bt;	/* same instant in CLOCK_BOOTTIME */
 
 	struct dentry *nfsd_client_dir;
 

index 3dc0c0f6eb5d..17cb3b0ad956 100644
@@ -6870,7 +6870,7 @@ bool nfsd4_force_end_grace(struct nfsd_net *nn)
  */
 static bool clients_still_reclaiming(struct nfsd_net *nn)
 {
-	time64_t double_grace_period_end = nn->boot_time +
+	time64_t double_grace_period_end = nn->boot_time_bt +
 					   2 * nn->nfsd4_lease;
 
 	if (test_bit(NFSD_NET_GRACE_END_FORCED, &nn->flags))

@@ -9245,6 +9245,7 @@ static int nfs4_state_create_net(struct net *net)
 	nn->conf_name_tree = RB_ROOT;
 	nn->unconf_name_tree = RB_ROOT;
 	nn->boot_time = ktime_get_real_seconds();
+	nn->boot_time_bt = ktime_get_boottime_seconds();
 	clear_bit(NFSD_NET_GRACE_ENDED, &nn->flags);
 	clear_bit(NFSD_NET_GRACE_END_FORCED, &nn->flags);
 	nn->nfsd4_manager.block_opens = true;

2.54.0

clients_still_reclaiming() uses separate test_bit() and clear_bit() calls on NFSD_NET_SOMEBODY_RECLAIMED. A concurrent set_bit() from the OPEN or LOCK reclaim path arriving between the test and clear is silently lost, causing the next laundromat tick to end grace prematurely.

Replace with test_and_clear_bit() to make the read-and-clear atomic.

---
 fs/nfsd/nfs4state.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

index 17cb3b0ad956..0735a3bafa58 100644
@@ -6884,9 +6884,8 @@ static bool clients_still_reclaiming(struct nfsd_net *nn)
 		if (atomic_read(&nn->nr_reclaim_complete) == size)
 			return false;
 	}
-	if (!test_bit(NFSD_NET_SOMEBODY_RECLAIMED, &nn->flags))
+	if (!test_and_clear_bit(NFSD_NET_SOMEBODY_RECLAIMED, &nn->flags))
 		return false;
-	clear_bit(NFSD_NET_SOMEBODY_RECLAIMED, &nn->flags);
 	/*
 	 * If we've given them *two* lease times to reclaim, and they're
 	 * still not done, give up:

2.54.0

nfsd4_lock() only checks the namespace-wide grace flag when deciding whether to accept a reclaim LOCK. It does not check the per-client NFSD4_CLIENT_RECLAIM_COMPLETE bit. A NFSv4.1+ client that has already sent RECLAIM_COMPLETE can submit lk_reclaim=1 while grace is still active (e.g. lockd holds the grace list open), and the server accepts it instead of returning NFS4ERR_NO_GRACE as required by RFC 8881 section 8.4.2.1.

The OPEN path already has the correct two-tier guard via nfs4_check_open_reclaim(). Add the equivalent check to the LOCK path.

---
 fs/nfsd/nfs4state.c | 3 +++
 1 file changed, 3 insertions(+)

index 0735a3bafa58..a0c97bff3cff 100644
@@ -8599,6 +8599,9 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	status = nfserr_no_grace;
 	if (!locks_in_grace(net) && lock->lk_reclaim)
 		goto out;
+	if (lock->lk_reclaim &&
+	    test_bit(NFSD4_CLIENT_RECLAIM_COMPLETE, &cstate->clp->cl_flags))
+		goto out;
 
 	if (lock->lk_reclaim)
 		flags |= FL_RECLAIM;

2.54.0

nfsd_sock_nl_policy declares NFSD_A_SOCK_ADDR as bare NLA_BINARY with no minimum length. A CAP_NET_ADMIN caller can send a 16-byte NFSD_A_SOCK_ADDR with sa_family=AF_INET6, causing a 12-byte OOB read across three consumers (rpc_cmp_addr_port, svc_find_listener, kernel_bind).

Tighten the policy to NLA_POLICY_MIN_LEN(16) and add per-family length validation in both nlmsg_for_each_attr_type loops.

---
 Documentation/netlink/specs/nfsd.yaml |  4 ++++
 fs/nfsd/netlink.c                     |  2 +-
 fs/nfsd/nfsctl.c                      | 30 ++++++++++++++++++++++++++++++
 3 files changed, 35 insertions(+), 1 deletion(-)

index 8f36fadd68f7..9677ba19ffcd 100644
@@ -156,6 +156,10 @@ attribute-sets:
       -
         name: addr
         type: binary
+        # 16 == sizeof(struct sockaddr_in); AF_INET6 callers
+        # validate the full sockaddr_in6 length in nfsctl.c.
+        checks:
+          min-len: 16
       -
         name: transport-name
         type: string

index fbee3676d253..6570960034f1 100644
@@ -37,7 +37,7 @@ const struct nla_policy nfsd_fslocations_nl_policy[NFSD_A_FSLOCATIONS_LOCATION +
 };
 
 const struct nla_policy nfsd_sock_nl_policy[NFSD_A_SOCK_TRANSPORT_NAME + 1] = {
-	[NFSD_A_SOCK_ADDR] = { .type = NLA_BINARY, },
+	[NFSD_A_SOCK_ADDR] = NLA_POLICY_MIN_LEN(16),
 	[NFSD_A_SOCK_TRANSPORT_NAME] = { .type = NLA_NUL_STRING, },
 };
 

index ab10692ee937..f3b3154b16c5 100644
@@ -2016,6 +2016,21 @@ int nfsd_nl_listener_set_doit(struct sk_buff *skb, struct genl_info *info)
 		xcl_name = nla_data(tb[NFSD_A_SOCK_TRANSPORT_NAME]);
 		sa = nla_data(tb[NFSD_A_SOCK_ADDR]);
 
+		switch (sa->sa_family) {
+		case AF_INET:
+			if (nla_len(tb[NFSD_A_SOCK_ADDR]) <
+			    sizeof(struct sockaddr_in))
+				continue;
+			break;
+		case AF_INET6:
+			if (nla_len(tb[NFSD_A_SOCK_ADDR]) <
+			    sizeof(struct sockaddr_in6))
+				continue;
+			break;
+		default:
+			continue;
+		}
+
 		/* Put back any matching sockets */
 		list_for_each_entry_safe(xprt, tmp, &permsocks, xpt_list) {
 			/* This shouldn't be possible */

@@ -2077,6 +2092,21 @@ int nfsd_nl_listener_set_doit(struct sk_buff *skb, struct genl_info *info)
 		xcl_name = nla_data(tb[NFSD_A_SOCK_TRANSPORT_NAME]);
 		sa = nla_data(tb[NFSD_A_SOCK_ADDR]);
 
+		switch (sa->sa_family) {
+		case AF_INET:
+			if (nla_len(tb[NFSD_A_SOCK_ADDR]) <
+			    sizeof(struct sockaddr_in))
+				continue;
+			break;
+		case AF_INET6:
+			if (nla_len(tb[NFSD_A_SOCK_ADDR]) <
+			    sizeof(struct sockaddr_in6))
+				continue;
+			break;
+		default:
+			continue;
+		}
+
 		xprt = svc_find_listener(serv, xcl_name, net, sa);
 		if (xprt) {
 			if (delete)

2.54.0

nlmsvc_ops is published by nfsd_lockd_init() and cleared by nfsd_lockd_shutdown() with plain stores, while lockd dereferences it unguarded from dispatch sites in fs/lockd/svcsubs.c. The pointer targets nfsd's .rodata and the fopen/fclose callbacks live in nfsd's .text, so a stale load after rmmod nfsd results in either a NULL deref or a module-text use-after-free.

Declare nlmsvc_ops as __rcu, publish via rcu_assign_pointer(), clear via RCU_INIT_POINTER() + synchronize_rcu(). Add a struct module *owner field to nlmsvc_binding and pin the module across indirect calls with try_module_get/module_put. When the binding is torn down, fall back to fput() to avoid leaking struct file references.

---
 fs/lockd/svc.c             |  4 ++--
 fs/lockd/svc4proc.c        |  4 ++--
 fs/lockd/svcproc.c         |  4 ++--
 fs/lockd/svcsubs.c         | 52 +++++++++++++++++++++++++++++++++++++++-------
 fs/nfsd/lockd.c            |  6 ++++--
 include/linux/lockd/bind.h | 12 ++++++++---
 6 files changed, 64 insertions(+), 18 deletions(-)

index 490551369ef2..ee90e743064a 100644
@@ -47,7 +47,7 @@
 
 static struct svc_program	nlmsvc_program;
 
-const struct nlmsvc_binding	*nlmsvc_ops;
+const struct nlmsvc_binding __rcu *nlmsvc_ops;
 EXPORT_SYMBOL_GPL(nlmsvc_ops);
 
 static DEFINE_MUTEX(nlmsvc_mutex);

@@ -142,7 +142,7 @@ lockd(void *vrqstp)
 		nlmsvc_retry_blocked(rqstp);
 		svc_recv(rqstp, 0);
 	}
-	if (nlmsvc_ops)
+	if (rcu_access_pointer(nlmsvc_ops))
 		nlmsvc_invalidate_all();
 	nlm_shutdown_hosts();
 	cancel_delayed_work_sync(&ln->grace_period_end);

index 78e675470c4b..080dffce9d8e 100644
@@ -128,7 +128,7 @@ nlm4svc_lookup_host(struct svc_rqst *rqstp, string caller, bool monitored)
 {
 	struct nlm_host *host;
 
-	if (!nlmsvc_ops)
+	if (!rcu_access_pointer(nlmsvc_ops))
 		return NULL;
 	host = nlmsvc_lookup_host(rqstp, caller.data, caller.len);
 	if (!host)

@@ -894,7 +894,7 @@ static __be32 nlm4svc_proc_granted_res(struct svc_rqst *rqstp)
 {
 	struct nlm4_res_wrapper *argp = rqstp->rq_argp;
 
-	if (!nlmsvc_ops)
+	if (!rcu_access_pointer(nlmsvc_ops))
 		return rpc_success;
 
 	if (nlm4_netobj_to_cookie(&argp->cookie, &argp->xdrgen.cookie))

index 4836887f11ef..dce6f6e3fd40 100644
@@ -133,7 +133,7 @@ nlm3svc_lookup_host(struct svc_rqst *rqstp, string caller, bool monitored)
 {
 	struct nlm_host *host;
 
-	if (!nlmsvc_ops)
+	if (!rcu_access_pointer(nlmsvc_ops))
 		return NULL;
 	host = nlmsvc_lookup_host(rqstp, caller.data, caller.len);
 	if (!host)

@@ -923,7 +923,7 @@ static __be32 nlmsvc_proc_granted_res(struct svc_rqst *rqstp)
 {
 	struct nlm_res_wrapper *argp = rqstp->rq_argp;
 
-	if (!nlmsvc_ops)
+	if (!rcu_access_pointer(nlmsvc_ops))
 		return rpc_success;
 
 	if (nlm_netobj_to_cookie(&argp->cookie, &argp->xdrgen.cookie))

index 6fe1325815e0..72a5b499839d 100644
@@ -92,6 +92,7 @@ nlm_fclose(struct file *filp)
 }
 
 static const struct nlmsvc_binding nfsd_nlm_ops = {
+	.owner		= THIS_MODULE,
 	.fopen		= nlm_fopen,		/* open file for locking */
 	.fclose		= nlm_fclose,		/* close file */
 };

@@ -100,11 +101,12 @@ void
 nfsd_lockd_init(void)
 {
 	dprintk("nfsd: initializing lockd\n");
-	nlmsvc_ops = &nfsd_nlm_ops;
+	rcu_assign_pointer(nlmsvc_ops, &nfsd_nlm_ops);
 }
 
 void
 nfsd_lockd_shutdown(void)
 {
-	nlmsvc_ops = NULL;
+	RCU_INIT_POINTER(nlmsvc_ops, NULL);
+	synchronize_rcu();
 }

index b614e0deea72..db8207d4059f 100644
@@ -16,17 +16,23 @@ struct svc_rqst;
 struct rpc_task;
 struct rpc_clnt;
 struct super_block;
+struct module;
 
-/*
- * This is the set of functions for lockd->nfsd communication
+/**
+ * struct nlmsvc_binding - lockd -> nfsd callback table
+ * @owner:  module that provides this binding.
+ * @fopen:  open a file by NFS file handle on behalf of an NLM request.
+ * @fclose: close a file that was previously opened via @fopen.
+ *          Implementations MUST be semantically equivalent to fput().
  */
 struct nlmsvc_binding {
+	struct module	*owner;
 	int		(*fopen)(struct svc_rqst *rqstp, struct nfs_fh *f,
 				 struct file **filp, int flags);
 	void		(*fclose)(struct file *filp);
 };
 
-extern const struct nlmsvc_binding *nlmsvc_ops;
+extern const struct nlmsvc_binding __rcu *nlmsvc_ops;
 
 /*
  * Similar to nfs_client_initdata, but without the NFS-specific

2.54.0

nfsd_debugfs_init() runs before nfsd4_init_slabs() in init_nfsd(). If the slab allocation fails, the bare "return retval" bypasses nfsd_debugfs_exit(), leaving orphan debugfs files with stale fops pointers into the freed module text.

Move nfsd_debugfs_init() to after the slab init succeeds, so the early return has no debugfs state to clean up.

Since debugfs is now the more recently initialized of the two, also update the unwind paths to match reverse-initialization (LIFO) order:
run nfsd_debugfs_exit() before nfsd4_free_slabs() in both the init_nfsd() error path and exit_nfsd(). The nfsd debugfs files only reference module-global state and have no dependency on the slab caches, so that reordering is a cleanup with no functional change.

---
 fs/nfsd/nfsctl.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

index f3b3154b16c5..720b481bb7ad 100644
@@ -2557,11 +2557,12 @@ static int __init init_nfsd(void)
 {
 	int retval;
 
-	nfsd_debugfs_init();
-
 	retval = nfsd4_init_slabs();
 	if (retval)
 		return retval;
+
+	nfsd_debugfs_init();
+
 	retval = nfsd4_init_pnfs();
 	if (retval)
 		goto out_free_slabs;

@@ -2606,8 +2607,8 @@ static int __init init_nfsd(void)
 out_free_pnfs:
 	nfsd4_exit_pnfs();
 out_free_slabs:
-	nfsd4_free_slabs();
 	nfsd_debugfs_exit();
+	nfsd4_free_slabs();
 	return retval;
 }
 

@@ -2622,9 +2623,9 @@ static void __exit exit_nfsd(void)
 	unregister_pernet_subsys(&nfsd_net_ops);
 	nfsd_drc_slab_free();
 	nfsd_lockd_shutdown();
-	nfsd4_free_slabs();
 	nfsd4_exit_pnfs();
 	nfsd_debugfs_exit();
+	nfsd4_free_slabs();
 }
 
 MODULE_AUTHOR("Olaf Kirch <[email protected]>");

2.54.0

shrinker_register() precedes the INIT_LIST_HEAD loop and the drc_hashsize store. On weakly-ordered architectures (arm64, ppc), a shrinker scan can observe drc_hashsize before the bucket list heads are initialized, causing a NULL deref in the DRC shrinker callback.

Move bucket initialization and the drc_hashsize store before shrinker_register() so the hash table is fully initialized before it becomes visible to the shrinker.

---
 fs/nfsd/nfscache.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

index 154468ceccdc..18f8556d33dd 100644
@@ -200,14 +200,14 @@ int nfsd_reply_cache_init(struct nfsd_net *nn)
 	nn->nfsd_reply_cache_shrinker->seeks = 1;
 	nn->nfsd_reply_cache_shrinker->private_data = nn;
 
-	shrinker_register(nn->nfsd_reply_cache_shrinker);
-
 	for (i = 0; i < hashsize; i++) {
 		INIT_LIST_HEAD(&nn->drc_hashtbl[i].lru_head);
 		spin_lock_init(&nn->drc_hashtbl[i].cache_lock);
 	}
 	nn->drc_hashsize = hashsize;
 
+	shrinker_register(nn->nfsd_reply_cache_shrinker);
+
 	return 0;
 out_shrinker:
 	kvfree(nn->drc_hashtbl);

2.54.0

nfsd_dispatch() sets rq_status_counter to an odd value once a request has been decoded, and back to an even value once it has been fully processed, forming a seq-lock like protocol with the lockless reader in nfsd_nl_rpc_status_get_dumpit().

Only the fully successful path restored the counter to even. The cache-hit (RC_REPLY), drop (RC_DROPIT / RQ_DROPME) and encode-error paths all return after the odd-valued store without ever bringing the counter back to even. Once one of those paths is taken, rq_status_counter is left odd: the next request's decode ORs in 1 (still odd) and only a subsequent successful encode restores even. While stuck odd, the dumpit reader treats the rqstp fields as stable and its retry check compares against the same unchanging odd value, so it never detects concurrent mutation. This exposes actively mutating fields (e.g. args->ops / args->opcnt during compound decode and release) to the lockless reader, which can read past the end of the 8-element inline ops array.

Add a helper that advances the counter to the next even value and call it on every return path that follows the odd-valued store. The decode-error path is left untouched as it is reached before the counter is set odd.

---
 fs/nfsd/nfssvc.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

index b8e8d80e984c..a8ea4dbfa56b 100644
@@ -966,6 +966,20 @@ nfsd(void *vrqstp)
 	return 0;
 }
 
+/*
+ * Set rq_status_counter back to an even value, indicating that the rqstp
+ * fields are no longer meaningful to a lockless reader. This pairs with the
+ * odd-valued store made once the request has been decoded, and must run on
+ * every return path that follows it so that the seq-lock like protocol used
+ * by nfsd_nl_rpc_status_get_dumpit() is not left permanently odd. The store
+ * also advances the counter so a concurrent reader detects the transition.
+ */
+static void nfsd_status_counter_set_idle(struct svc_rqst *rqstp)
+{
+	smp_store_release(&rqstp->rq_status_counter,
+			  (rqstp->rq_status_counter | 1) + 1);
+}
+
 /**
  * nfsd_dispatch - Process an NFS or NFSACL or LOCALIO Request
  * @rqstp: incoming request

@@ -1028,14 +1042,9 @@ int nfsd_dispatch(struct svc_rqst *rqstp)
 	if (!proc->pc_encode(rqstp, &rqstp->rq_res_stream))
 		goto out_encode_err;
 
-	/*
-	 * Release rq_status_counter setting it to an even value after the rpc
-	 * request has been properly processed.
-	 */
-	smp_store_release(&rqstp->rq_status_counter, rqstp->rq_status_counter + 1);
-
 	nfsd_cache_update(rqstp, rp, ntli->ntli_cachetype, nfs_reply);
 out_cached_reply:
+	nfsd_status_counter_set_idle(rqstp);
 	return 1;
 
 out_decode_err:

@@ -1046,12 +1055,14 @@ int nfsd_dispatch(struct svc_rqst *rqstp)
 out_update_drop:
 	nfsd_cache_update(rqstp, rp, RC_NOCACHE, NULL);
 out_dropit:
+	nfsd_status_counter_set_idle(rqstp);
 	return 0;
 
 out_encode_err:
 	trace_nfsd_cant_encode_err(rqstp);
 	nfsd_cache_update(rqstp, rp, RC_NOCACHE, NULL);
 	*statp = rpc_system_err;
+	nfsd_status_counter_set_idle(rqstp);
 	return 1;
 }
 

2.54.0

When a netlink dump of rpc_status fills the reply buffer, the iterator saves the pool index in cb->args[0] and the thread index within that pool in cb->args[1], so the next invocation can resume where it left off.

On resume the thread skip count was applied to every pool, not just the pool the dump was suspended in. Once the resumed pool was exhausted and the loop advanced to the next pool, the first cb->args[1] threads of that pool (and of every pool after it) were still skipped, silently omitting their rpc_status entries from the dump.

Apply the saved thread index only to the pool matching cb->args[0], and start every subsequent pool from thread 0.

---
 fs/nfsd/nfsctl.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

index 720b481bb7ad..a5e328b14e45 100644
@@ -1527,10 +1527,20 @@ int nfsd_nl_rpc_status_get_dumpit(struct sk_buff *skb,
 
 	for (i = 0; i < nn->nfsd_serv->sv_nrpools; i++) {
 		struct svc_rqst *rqstp;
+		long thread_skip = 0;
 
 		if (i < cb->args[0]) /* already consumed */
 			continue;
 
+		/*
+		 * The saved thread index only applies to the pool the dump
+		 * was resumed in. Subsequent pools must start from thread 0,
+		 * otherwise their first cb->args[1] threads are silently
+		 * skipped.
+		 */
+		if (i == cb->args[0])
+			thread_skip = cb->args[1];
+
 		rqstp_index = 0;
 		list_for_each_entry_rcu(rqstp,
 				&nn->nfsd_serv->sv_pools[i].sp_all_threads,

@@ -1538,7 +1548,7 @@ int nfsd_nl_rpc_status_get_dumpit(struct sk_buff *skb,
 			struct nfsd_genl_rqstp genl_rqstp = {};
 			unsigned int status_counter;
 
-			if (rqstp_index++ < cb->args[1]) /* already consumed */
+			if (rqstp_index++ < thread_skip) /* already consumed */
 				continue;
 			/*
 			 * Acquire rq_status_counter before parsing the rqst

2.54.0

In nfs4_preprocess_seqid_op() the stateid is obtained from nfsd4_lookup_stateid(), which holds a reference on the nfs4_stid (sc_count) but takes no reference on the stateowner. openlockstateid() merely casts that stid and likewise takes no reference.

When nfsd4_cstate_assign_replay() returns -EAGAIN (the replay owner is being torn down, RP_UNHASHED) it has not taken a stateowner reference on that path. The error handling nevertheless called nfs4_put_stateowner(stp->st_stateowner), dropping an so_count reference the function never acquired -- risking a stateowner refcount underflow and use-after-free -- while leaking the sc_count reference held on the stid. The leaked stid reference can also stall a concurrent nfsd4_close_open_stateid() waiting for sc_count to drop.

Drop the reference actually held -- the stid -- before retrying. The stateowner stays alive through the reference held by the stid. This mirrors the open path in nfsd4_process_open2(), where the put balances a reference that path explicitly holds on the stateowner.

---
 fs/nfsd/nfs4state.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index a0c97bff3cff..bef0ec9be459 100644
@@ -7876,7 +7876,7 @@ nfs4_preprocess_seqid_op(struct nfsd4_compound_state *cstate, u32 seqid,
 		return status;
 	stp = openlockstateid(s);
 	if (nfsd4_cstate_assign_replay(cstate, stp->st_stateowner) == -EAGAIN) {
-		nfs4_put_stateowner(stp->st_stateowner);
+		nfs4_put_stid(&stp->st_stid);
 		goto retry;
 	}
 

2.54.0

Dropped 08/21 - rejected in v1
Dropped 15/21 - will be replaced
Folded 20/21 into 03/21 to avoid a bisect hazard

Applied to nfsd-testing, thanks!

[01/21] nfsd: clear opcnt on compound arg release to prevent OOB read

        commit: ef6fe22ee7de0a016bd6b4ec6cf314b9f93cd816
[02/21] nfsd: add missing read barrier to rpc_status_get dumpit seqcount retry
        commit: 71a715232eff2ff9b8fbb7ad2f9f882187056ac8
[03/21] nfsd: fix netlink dumpit error handling for rpc_status_get
        commit: ef265c50d90fc7e92461668fab69f9a33e935092
[04/21] sunrpc: defer rq_argp and rq_resp free until after RCU grace period
        commit: 5d51eea254f4be0b146bd859384e10723d59bcf8
[05/21] nfsd: check nfsd4_acl_to_attr() return value in nfsd4_create()
        commit: 50c819a3406963f7935a62d81321984b947b5b97
[06/21] nfsd: add filehandle match check to nfsd4_delegreturn()
        commit: 35450eb04ec5718ef680062da9711f054cacfdb9
[07/21] nfsd: validate nseconds in TIME_DELEG decode paths
        commit: 387858689eb526681b3fca7d24d3fb18b353356a
[09/21] nfsd: fix version mismatch loops in nfsd_acl_init_request()
        commit: a6a22b9c4135f80b8b1b5c9f7629cddb677c5dad
[10/21] nfsd: fix FL_SLEEP being set unconditionally for all LOCK types
        commit: fd848bc2582871814762604f3675f1bd139e356e
[11/21] nfsd: add fh_want_write() for early-verified SETATTR in nfsd_proc_setattr()
        commit: 19a6dd09bf8d22d7af15ef068f57ec279f9051aa
[12/21] nfsd: fix clock domain mismatch in clients_still_reclaiming()
        commit: 0fecd3b7c474eedb60ec03716a13a61c93eadec6
[13/21] nfsd: use test_and_clear_bit for somebody_reclaimed to prevent lost update
        commit: 7f101efd236e14915bf35d5c2b62d10350a63b1c
[14/21] nfsd: reject reclaim LOCK after RECLAIM_COMPLETE
        commit: 7a02caf96fac6faa22a93b8bd06d29d6f90318cd
[16/21] lockd, nfsd: RCU-protect nlmsvc_ops dispatch
        commit: b601157bf2118094708689033921e996e3418e29
[17/21] nfsd: move nfsd_debugfs_init() after nfsd4_init_slabs() in init_nfsd()
        commit: bb483d4ed57fd52c21c0f64c126986489d51c9d5
[18/21] nfsd: initialize DRC hash table before registering shrinker
        commit: bec760bf8e868cb6e232b4760dac86c0172d17f5
[19/21] nfsd: restore rq_status_counter to even on all nfsd_dispatch() exit paths
        commit: f8b2bcada458865af99537ece2b5470417b0143b
[21/21] nfsd: drop the stateid, not the stateowner, on seqid_op replay retry
        commit: 402b9e49a8e5da556adbafd21871ea4e8a617372

--
Chuck Lever