Date: Wed, 17 Nov 2021 01:23:46 GMT From: Rick Macklem <rmacklem@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org Subject: git: 4a03ae8d17dd - stable/13 - nfscl: Fix use after free for forced dismount Message-ID: <202111170123.1AH1Nk3L039121@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
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); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202111170123.1AH1Nk3L039121>