git: 501bdf300119 - main - nfscl: newnfs_copycred() cannot be called when a mutex is held

From: Rick Macklem <rmacklem_at_FreeBSD.org>
Date: Mon, 06 Nov 2023 22:26:30 UTC
The branch main has been updated by rmacklem:

URL: https://cgit.FreeBSD.org/src/commit/?id=501bdf3001190686bf55d9d333cb533858c2cf2f

commit 501bdf3001190686bf55d9d333cb533858c2cf2f
Author:     Rick Macklem <rmacklem@FreeBSD.org>
AuthorDate: 2023-11-06 22:25:30 +0000
Commit:     Rick Macklem <rmacklem@FreeBSD.org>
CommitDate: 2023-11-06 22:25:30 +0000

    nfscl: newnfs_copycred() cannot be called when a mutex is held
    
    Since newnfs_copycred() calls crsetgroups() which in turn calls
    crextend() which might do a malloc(M_WAITOK), newnfs_copycred()
    cannot be called with a mutex held.  Fortunately, the malloc()
    call is rarely done, since XU_GROUPS is 16 and the NFS client
    uses a maximum of 17 (only 17 groups will cause the malloc() to
    be called).  Further, it is only a problem if the malloc() tries
    to sleep().  As such, this bug does not seem to have caused
    problems in practice.
    
    This patch fixes the one place in the NFS client where
    newnfs_copycred() is called while a mutex is held by moving the
    call to after where the mutex is released.
    
    Found by inspection while working on an experimental patch.
    
    MFC after:      2 weeks
---
 sys/fs/nfsclient/nfs_clstate.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/sys/fs/nfsclient/nfs_clstate.c b/sys/fs/nfsclient/nfs_clstate.c
index 579210941802..ebc11efea637 100644
--- a/sys/fs/nfsclient/nfs_clstate.c
+++ b/sys/fs/nfsclient/nfs_clstate.c
@@ -526,6 +526,7 @@ nfscl_getstateid(vnode_t vp, u_int8_t *nfhp, int fhlen, u_int32_t mode,
 	struct nfscldeleg *dp;
 	struct nfsnode *np;
 	struct nfsmount *nmp;
+	struct nfscred ncr;
 	u_int8_t own[NFSV4CL_LOCKNAMELEN], lockown[NFSV4CL_LOCKNAMELEN];
 	int error;
 	bool done;
@@ -683,7 +684,7 @@ nfscl_getstateid(vnode_t vp, u_int8_t *nfhp, int fhlen, u_int32_t mode,
 		 * A read ahead or write behind is indicated by p == NULL.
 		 */
 		if (p == NULL)
-			newnfs_copycred(&op->nfso_cred, cred);
+			memcpy(&ncr, &op->nfso_cred, sizeof(ncr));
 	}
 
 	/*
@@ -697,6 +698,8 @@ nfscl_getstateid(vnode_t vp, u_int8_t *nfhp, int fhlen, u_int32_t mode,
 	stateidp->other[1] = op->nfso_stateid.other[1];
 	stateidp->other[2] = op->nfso_stateid.other[2];
 	NFSUNLOCKCLSTATE();
+	if (p == NULL)
+		newnfs_copycred(&ncr, cred);
 	return (0);
 }