git: 4a03ae8d17dd - stable/13 - nfscl: Fix use after free for forced dismount

From: Rick Macklem <rmacklem_at_FreeBSD.org>
Date: Wed, 17 Nov 2021 01:23:46 UTC
The branch stable/13 has been updated by rmacklem:

URL: https://cgit.FreeBSD.org/src/commit/?id=4a03ae8d17ddf3d3b57ca281000fd98e200b92cc

commit 4a03ae8d17ddf3d3b57ca281000fd98e200b92cc
Author:     Rick Macklem <rmacklem@FreeBSD.org>
AuthorDate: 2021-11-03 19:15:40 +0000
Commit:     Rick Macklem <rmacklem@FreeBSD.org>
CommitDate: 2021-11-17 01:20:01 +0000

    nfscl: Fix use after free for forced dismount
    
    When a forced dismount is done and delegations are being
    issued by the server (disabled by default for FreeBSD
    servers), the delegation structure is free'd before the
    loop calling vflush().  This could result in a use after
    free of the delegation structure.
    
    This patch changes the code so that the delegation
    structures are not free'd until after the vflush()
    loop for forced dismounts.
    
    Found during a recent IETF NFSv4 working group testing event.
    
    (cherry picked from commit 441222585968517c595ef7f39e5c71a42d238acd)
---
 sys/fs/nfs/nfs_var.h            |  2 +-
 sys/fs/nfsclient/nfs_clstate.c  | 16 +++++++++++-----
 sys/fs/nfsclient/nfs_clvfsops.c | 13 +++++++++++--
 3 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/sys/fs/nfs/nfs_var.h b/sys/fs/nfs/nfs_var.h
index 44aedec09a96..b45143067c46 100644
--- a/sys/fs/nfs/nfs_var.h
+++ b/sys/fs/nfs/nfs_var.h
@@ -592,7 +592,7 @@ void nfscl_lockrelease(struct nfscllockowner *, int, int);
 void nfscl_fillclid(u_int64_t, char *, u_int8_t *, u_int16_t);
 void nfscl_filllockowner(void *, u_int8_t *, int);
 void nfscl_freeopen(struct nfsclopen *, int, bool);
-void nfscl_umount(struct nfsmount *, NFSPROC_T *);
+void nfscl_umount(struct nfsmount *, NFSPROC_T *, struct nfscldeleghead *);
 void nfscl_renewthread(struct nfsclclient *, NFSPROC_T *);
 void nfscl_initiate_recovery(struct nfsclclient *);
 int nfscl_hasexpired(struct nfsclclient *, u_int32_t, NFSPROC_T *);
diff --git a/sys/fs/nfsclient/nfs_clstate.c b/sys/fs/nfsclient/nfs_clstate.c
index 51f901bf14fc..4392ed6cbcb2 100644
--- a/sys/fs/nfsclient/nfs_clstate.c
+++ b/sys/fs/nfsclient/nfs_clstate.c
@@ -119,7 +119,8 @@ static void nfscl_insertlock(struct nfscllockowner *, struct nfscllock *,
     struct nfscllock *, int);
 static int nfscl_updatelock(struct nfscllockowner *, struct nfscllock **,
     struct nfscllock **, int);
-static void nfscl_delegreturnall(struct nfsclclient *, NFSPROC_T *);
+static void nfscl_delegreturnall(struct nfsclclient *, NFSPROC_T *,
+    struct nfscldeleghead *);
 static u_int32_t nfscl_nextcbident(void);
 static mount_t nfscl_getmnt(int, uint8_t *, u_int32_t, struct nfsclclient **);
 static struct nfsclclient *nfscl_getclnt(u_int32_t);
@@ -1987,7 +1988,7 @@ static int	fake_global;	/* Used to force visibility of MNTK_UNMOUNTF */
  * Called from nfs umount to free up the clientid.
  */
 void
-nfscl_umount(struct nfsmount *nmp, NFSPROC_T *p)
+nfscl_umount(struct nfsmount *nmp, NFSPROC_T *p, struct nfscldeleghead *dhp)
 {
 	struct nfsclclient *clp;
 	struct ucred *cred;
@@ -2049,7 +2050,7 @@ nfscl_umount(struct nfsmount *nmp, NFSPROC_T *p)
 		 * the server throws it away?
 		 */
 		LIST_REMOVE(clp, nfsc_list);
-		nfscl_delegreturnall(clp, p);
+		nfscl_delegreturnall(clp, p, dhp);
 		cred = newnfs_getcred();
 		if (NFSHASNFSV4N(nmp)) {
 			(void)nfsrpc_destroysession(nmp, clp, cred, p);
@@ -3440,7 +3441,8 @@ lookformore:
  * (Must be called with client sleep lock.)
  */
 static void
-nfscl_delegreturnall(struct nfsclclient *clp, NFSPROC_T *p)
+nfscl_delegreturnall(struct nfsclclient *clp, NFSPROC_T *p,
+    struct nfscldeleghead *dhp)
 {
 	struct nfscldeleg *dp, *ndp;
 	struct ucred *cred;
@@ -3449,7 +3451,11 @@ nfscl_delegreturnall(struct nfsclclient *clp, NFSPROC_T *p)
 	TAILQ_FOREACH_SAFE(dp, &clp->nfsc_deleg, nfsdl_list, ndp) {
 		nfscl_cleandeleg(dp);
 		(void) nfscl_trydelegreturn(dp, cred, clp->nfsc_nmp, p);
-		nfscl_freedeleg(&clp->nfsc_deleg, dp, true);
+		if (dhp != NULL) {
+			nfscl_freedeleg(&clp->nfsc_deleg, dp, false);
+			TAILQ_INSERT_HEAD(dhp, dp, nfsdl_list);
+		} else
+			nfscl_freedeleg(&clp->nfsc_deleg, dp, true);
 	}
 	NFSFREECRED(cred);
 }
diff --git a/sys/fs/nfsclient/nfs_clvfsops.c b/sys/fs/nfsclient/nfs_clvfsops.c
index 57ac7f59f38d..55941170fc69 100644
--- a/sys/fs/nfsclient/nfs_clvfsops.c
+++ b/sys/fs/nfsclient/nfs_clvfsops.c
@@ -1765,8 +1765,11 @@ nfs_unmount(struct mount *mp, int mntflags)
 	struct nfsmount *nmp;
 	int error, flags = 0, i, trycnt = 0;
 	struct nfsclds *dsp, *tdsp;
+	struct nfscldeleg *dp, *ndp;
+	struct nfscldeleghead dh;
 
 	td = curthread;
+	TAILQ_INIT(&dh);
 
 	if (mntflags & MNT_FORCE)
 		flags |= FORCECLOSE;
@@ -1790,7 +1793,7 @@ nfs_unmount(struct mount *mp, int mntflags)
 		if (error)
 			goto out;
 		/* For a forced close, get rid of the renew thread now */
-		nfscl_umount(nmp, td);
+		nfscl_umount(nmp, td, &dh);
 	}
 	/* We hold 1 extra ref on the root vnode; see comment in mountnfs(). */
 	do {
@@ -1805,7 +1808,7 @@ nfs_unmount(struct mount *mp, int mntflags)
 	 * We are now committed to the unmount.
 	 */
 	if ((mntflags & MNT_FORCE) == 0)
-		nfscl_umount(nmp, td);
+		nfscl_umount(nmp, td, NULL);
 	else {
 		mtx_lock(&nmp->nm_mtx);
 		nmp->nm_privflag |= NFSMNTP_FORCEDISM;
@@ -1847,6 +1850,12 @@ nfs_unmount(struct mount *mp, int mntflags)
 	}
 	free(nmp->nm_tlscertname, M_NEWNFSMNT);
 	free(nmp, M_NEWNFSMNT);
+
+	/* Free up the delegation structures for forced dismounts. */
+	TAILQ_FOREACH_SAFE(dp, &dh, nfsdl_list, ndp) {
+		TAILQ_REMOVE(&dh, dp, nfsdl_list);
+		free(dp, M_NFSCLDELEG);
+	}
 out:
 	return (error);
 }