[PATCH] nfsd: update existing connection flags under client lock

[Jiwoong Wi <dwdnlzj@gmail.com>]

nfsd4_match_existing_connection() searches the session connection list
while holding clp->cl_lock, but returns the matching nfsd4_conn after
releasing the lock. nfsd4_bind_conn_to_session() then updates
conn->cn_flags. Concurrent transport loss can remove and free the
connection in nfsd4_conn_lost() between the unlock and the update,
leading to a use-after-free read-modify-write of cn_flags.

Update cn_flags before releasing clp->cl_lock and stop returning the raw
connection pointer to the caller.

A natural reproducer pipelined 16 BIND_CONN_TO_SESSION requests and reset
the connection after receiving the first reply. On a Generic KASAN kernel
at v7.3-rc4-606-gfd179f8a05be, it triggered three reports in 1,000
attempts. All attempts completed, covering 55,494 BIND operations and
6,494 processed pipelined BIND requests.

With this change applied to nfsd-testing at 32eb1a60b456, the same
reproducer completed 1,000 attempts with no harness error or KASAN
report. A 750 ms post-unlock timing-aid control also completed without a
KASAN report. Basic NFSv4.2 and NFSv3 read/write/unmount smoke tests
passed, and fs/nfsd built with GCC 12.2 without a new warning.

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

Fixes: 02579b2ff8b0 ("nfsd: back channel stuck in SEQ4_STATUS_CB_PATH_DOWN")
Assisted-by: LLM
Signed-off-by: Jiwoong Wi <dwdnlzj@gmail.com>
---
 fs/nfsd/nfs4state.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index bbc16dd2294d..6b445c5d4eb3 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4810,7 +4810,7 @@ static struct nfsd4_conn *__nfsd4_find_conn(struct svc_xprt *xpt, struct nfsd4_s
 }
 
 static __be32 nfsd4_match_existing_connection(struct svc_rqst *rqst,
-		struct nfsd4_session *session, u32 req, struct nfsd4_conn **conn)
+		struct nfsd4_session *session, u32 req)
 {
 	struct nfs4_client *clp = session->se_client;
 	struct svc_xprt *xpt = rqst->rq_xprt;
@@ -4832,9 +4832,10 @@ static __be32 nfsd4_match_existing_connection(struct svc_rqst *rqst,
 		status = nfs_ok;
 	else
 		status = nfserr_inval;
+	if (status == nfs_ok &&
+	    (req == NFS4_CDFC4_FORE_OR_BOTH || req == NFS4_CDFC4_BACK))
+		c->cn_flags |= NFS4_CDFC4_BACK;
 	spin_unlock(&clp->cl_lock);
-	if (status == nfs_ok && conn)
-		*conn = c;
 	return status;
 }
 
@@ -4859,12 +4860,8 @@ __be32 nfsd4_bind_conn_to_session(struct svc_rqst *rqstp,
 	status = nfserr_wrong_cred;
 	if (!nfsd4_mach_creds_match(session->se_client, rqstp))
 		goto out;
-	status = nfsd4_match_existing_connection(rqstp, session,
-			bcts->dir, &conn);
+	status = nfsd4_match_existing_connection(rqstp, session, bcts->dir);
 	if (status == nfs_ok) {
-		if (bcts->dir == NFS4_CDFC4_FORE_OR_BOTH ||
-				bcts->dir == NFS4_CDFC4_BACK)
-			conn->cn_flags |= NFS4_CDFC4_BACK;
 		nfsd4_probe_callback(session->se_client);
 		goto out;
 	}

base-commit: 32eb1a60b456980761cf7a9cee8f907fdc08afb8
-- 
2.50.1 (Apple Git-155)