Date: Wed, 3 Nov 2021 19:19:29 GMT From: Rick Macklem <rmacklem@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 441222585968 - main - nfscl: Fix use after free for forced dismount Message-ID: <202111031919.1A3JJT98038476@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by rmacklem: URL: https://cgit.FreeBSD.org/src/commit/?id=441222585968517c595ef7f39e5c71a42d238acd commit 441222585968517c595ef7f39e5c71a42d238acd Author: Rick Macklem <rmacklem@FreeBSD.org> AuthorDate: 2021-11-03 19:15:40 +0000 Commit: Rick Macklem <rmacklem@FreeBSD.org> CommitDate: 2021-11-03 19:15:40 +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. MFC after: 2 weeks --- 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 7f0ca990540d..7e4136a6ff67 100644 --- a/sys/fs/nfs/nfs_var.h +++ b/sys/fs/nfs/nfs_var.h @@ -596,7 +596,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 22c0da1bcef8..42233ea7cf9d 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); @@ -1985,7 +1986,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; @@ -2047,7 +2048,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); @@ -3438,7 +3439,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; @@ -3447,7 +3449,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 d5d9bb395c79..330279b9ef4d 100644 --- a/sys/fs/nfsclient/nfs_clvfsops.c +++ b/sys/fs/nfsclient/nfs_clvfsops.c @@ -1767,8 +1767,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; @@ -1792,7 +1795,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 { @@ -1807,7 +1810,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; @@ -1849,6 +1852,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?202111031919.1A3JJT98038476>