From nobody Thu Feb 24 15:04:10 2022 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 2914619CD7FE; Thu, 24 Feb 2022 15:04:11 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4K4GQM0ddjz4nSS; Thu, 24 Feb 2022 15:04:11 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1645715051; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=K7o9SZRnBhasB0liLAO52N0KfaGJfpIFdOgYGxs2ieU=; b=C1kYQ1JpQ2/p75EvcIIskoiNxixj/eImOEIU0tUXLTS1LOUb4W5hJtcfy4IIubmGpQJ5oB G2Lq+Zow3Bfou8muVBqBpBbOC40qkh2D1kQmPclOvolspu8IFbgJkLELWo8zNmXqsccFw2 jKtjgrFcU0V+cj2TWA2FeEb5qEEq2df/5+0cWBA1vInOm/+c90ksfRm1Nk+sjOR6tFuizi Hc7fJB1FeVlQg9bPjKZ2Y7g+dD3IlkMYncKFDfKDOXieF71DSBl903i6SEqUWZF+46yvBF lVsLd7qxPxnh1/TvKBl7PwtbP8nypq171D5DP7bTk+NxiSDynORnByCPSj/cAA== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id E9800633C; Thu, 24 Feb 2022 15:04:10 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 21OF4AIA006552; Thu, 24 Feb 2022 15:04:10 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 21OF4AFW006551; Thu, 24 Feb 2022 15:04:10 GMT (envelope-from git) Date: Thu, 24 Feb 2022 15:04:10 GMT Message-Id: <202202241504.21OF4AFW006551@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Rick Macklem Subject: git: 06148d225170 - main - Revert "nfscl: Fix a use after free in nfscl_cleanupkext()" List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: rmacklem X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 06148d22517067dce12f90951c1cffa6ecad7a7b Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1645715051; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=K7o9SZRnBhasB0liLAO52N0KfaGJfpIFdOgYGxs2ieU=; b=Z5wZqNHy+iai5EB+RC89jO27OOEbtkCxlYtXWNcbV7BiGw0KvSg1Ypsibk2ZQ2GdTA61L6 m90jnUBYuvmPESyRY4xRkPd1LCz7em03aEUVu1DE086ogJYf7RdySqid+AlHgJvRq+6lLh NBB9HPYbUnrA9XOb8eNoXAuKPCFCQCp0P9sJodWHcyjYH+WpOJrq8urr1PMnUSiX9mSXhi tRFgr4NwfVedG5Urryl9WJo0luKTh1O8GPng5SF7lXUOeXBxhr95Y+FME/HvS6cYg4j5Yw vNjLY4ss6HC57BXVJJ+YuQO0yYXL5GroDL8hQ/QXid9lan6TE6jSMzlRi0NqEg== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1645715051; a=rsa-sha256; cv=none; b=Ds8EJ32maMlSjvPaSH8NtF3aAJDiOd3QFM2PDWd0nqid2DJ/Y0bq4QXfz7VWV490SQHv/c 7hw+kk+2K/bPy80aKSa0C3EUAHZizbjUDCnqrj0DHxLAIWr/o70vyfQ+0wGgFKoDtdH2ub ggGQWVugC9lFVvpspHwHR0tr/iu+gvTLDnROsU5NDGRTMiK2W62MgR9RJ0o2gy+gqEypLD FBZK7/2elF8wDBioJzSn9OfgrrNJu7/cv3DLUkFYkBGJymPTv1GLYnPmkQqTWj4hc970S7 z8nnkMIEd9C1+0lBz+hxNRlb1Tp+WRfmG28R57wfuPJ2laEYS+93rACADxc+kg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by rmacklem: URL: https://cgit.FreeBSD.org/src/commit/?id=06148d22517067dce12f90951c1cffa6ecad7a7b commit 06148d22517067dce12f90951c1cffa6ecad7a7b Author: Rick Macklem AuthorDate: 2022-02-24 15:01:03 +0000 Commit: Rick Macklem CommitDate: 2022-02-24 15:01:03 +0000 Revert "nfscl: Fix a use after free in nfscl_cleanupkext()" This reverts commit dd08b84e35b6fdee0df5fd0e0533cd361dec71cb. cy@ reported a problem caused by this patch. He will be testing an alternate patch, but I'm reverting this one. --- sys/fs/nfsclient/nfs_clstate.c | 70 +++++++----------------------------------- 1 file changed, 11 insertions(+), 59 deletions(-) diff --git a/sys/fs/nfsclient/nfs_clstate.c b/sys/fs/nfsclient/nfs_clstate.c index c37ff8a8ab11..5262986645cd 100644 --- a/sys/fs/nfsclient/nfs_clstate.c +++ b/sys/fs/nfsclient/nfs_clstate.c @@ -158,7 +158,7 @@ static int nfsrpc_reopen(struct nfsmount *, u_int8_t *, int, u_int32_t, static void nfscl_freedeleg(struct nfscldeleghead *, struct nfscldeleg *, bool); static int nfscl_errmap(struct nfsrv_descript *, u_int32_t); -static u_int nfscl_cleanup_common(struct nfsclclient *, u_int8_t *); +static void nfscl_cleanup_common(struct nfsclclient *, u_int8_t *); static int nfscl_recalldeleg(struct nfsclclient *, struct nfsmount *, struct nfscldeleg *, vnode_t, struct ucred *, NFSPROC_T *, int, vnode_t *); @@ -1642,21 +1642,8 @@ nfscl_expireopen(struct nfsclclient *clp, struct nfsclopen *op, static void nfscl_freeopenowner(struct nfsclowner *owp, int local) { - int owned; - /* - * Make sure the NFSCLSTATE mutex is held, to avoid races with - * calls in nfscl_renewthread() that do not hold a reference - * count on the nfsclclient and just the mutex. - * The mutex will not be held for calls done with the exclusive - * nfsclclient lock held. - */ - owned = mtx_owned(NFSCLSTATEMUTEXPTR); - if (owned == 0) - NFSLOCKCLSTATE(); LIST_REMOVE(owp, nfsow_list); - if (owned == 0) - NFSUNLOCKCLSTATE(); free(owp, M_NFSCLOWNER); if (local) nfsstatsv1.cllocalopenowners--; @@ -1671,21 +1658,8 @@ void nfscl_freelockowner(struct nfscllockowner *lp, int local) { struct nfscllock *lop, *nlop; - int owned; - /* - * Make sure the NFSCLSTATE mutex is held, to avoid races with - * calls in nfscl_renewthread() that do not hold a reference - * count on the nfsclclient and just the mutex. - * The mutex will not be held for calls done with the exclusive - * nfsclclient lock held. - */ - owned = mtx_owned(NFSCLSTATEMUTEXPTR); - if (owned == 0) - NFSLOCKCLSTATE(); LIST_REMOVE(lp, nfsl_list); - if (owned == 0) - NFSUNLOCKCLSTATE(); LIST_FOREACH_SAFE(lop, &lp->nfsl_lock, nfslo_list, nlop) { nfscl_freelock(lop, local); } @@ -1867,24 +1841,17 @@ nfscl_expireclient(struct nfsclclient *clp, struct nfsmount *nmp, } } -#define FREED_OPENOWNER 0x1 -#define FREED_LOCKOWNER 0x2 - /* * This function must be called after the process represented by "own" has * exited. Must be called with CLSTATE lock held. - * Return the FREED_xxx flag bits, since the caller needs to know if either - * the open owner or lock owner lists have changed. */ -static u_int +static void nfscl_cleanup_common(struct nfsclclient *clp, u_int8_t *own) { struct nfsclowner *owp, *nowp; struct nfscllockowner *lp, *nlp; struct nfscldeleg *dp; - u_int didfree; - didfree = 0; /* First, get rid of local locks on delegations. */ TAILQ_FOREACH(dp, &clp->nfsc_deleg, nfsdl_list) { LIST_FOREACH_SAFE(lp, &dp->nfsdl_lock, nfsl_list, nlp) { @@ -1892,7 +1859,6 @@ nfscl_cleanup_common(struct nfsclclient *clp, u_int8_t *own) if ((lp->nfsl_rwlock.nfslock_lock & NFSV4LOCK_WANTED)) panic("nfscllckw"); nfscl_freelockowner(lp, 1); - didfree |= FREED_LOCKOWNER; } } } @@ -1907,15 +1873,13 @@ nfscl_cleanup_common(struct nfsclclient *clp, u_int8_t *own) * here. For that case, let the renew thread clear * out the OpenOwner later. */ - if (LIST_EMPTY(&owp->nfsow_open)) { + if (LIST_EMPTY(&owp->nfsow_open)) nfscl_freeopenowner(owp, 0); - didfree |= FREED_OPENOWNER; - } else + else owp->nfsow_defunct = 1; } owp = nowp; } - return (didfree); } /* @@ -1924,11 +1888,10 @@ nfscl_cleanup_common(struct nfsclclient *clp, u_int8_t *own) static void nfscl_cleanupkext(struct nfsclclient *clp, struct nfscllockownerfhhead *lhp) { - struct nfsclowner *owp; + struct nfsclowner *owp, *nowp; struct nfsclopen *op; struct nfscllockowner *lp, *nlp; struct nfscldeleg *dp; - uint8_t own[NFSV4CL_LOCKNAMELEN]; /* * All the pidhash locks must be acquired, since they are sx locks @@ -1938,20 +1901,15 @@ nfscl_cleanupkext(struct nfsclclient *clp, struct nfscllockownerfhhead *lhp) */ pidhash_slockall(); NFSLOCKCLSTATE(); -tryagain: - LIST_FOREACH(owp, &clp->nfsc_owner, nfsow_list) { + LIST_FOREACH_SAFE(owp, &clp->nfsc_owner, nfsow_list, nowp) { LIST_FOREACH(op, &owp->nfsow_open, nfso_list) { LIST_FOREACH_SAFE(lp, &op->nfso_lock, nfsl_list, nlp) { if (LIST_EMPTY(&lp->nfsl_lock)) nfscl_emptylockowner(lp, lhp); } } - if (nfscl_procdoesntexist(owp->nfsow_owner)) { - memcpy(own, owp->nfsow_owner, NFSV4CL_LOCKNAMELEN); - if ((nfscl_cleanup_common(clp, own) & - FREED_OPENOWNER) != 0) - goto tryagain; - } + if (nfscl_procdoesntexist(owp->nfsow_owner)) + nfscl_cleanup_common(clp, owp->nfsow_owner); } /* @@ -1962,15 +1920,9 @@ tryagain: * nfscl_cleanup_common(). */ TAILQ_FOREACH(dp, &clp->nfsc_deleg, nfsdl_list) { -tryagain2: - LIST_FOREACH(lp, &dp->nfsdl_lock, nfsl_list) { - if (nfscl_procdoesntexist(lp->nfsl_owner)) { - memcpy(own, lp->nfsl_owner, - NFSV4CL_LOCKNAMELEN); - if ((nfscl_cleanup_common(clp, own) & - FREED_LOCKOWNER) != 0) - goto tryagain2; - } + LIST_FOREACH_SAFE(lp, &dp->nfsdl_lock, nfsl_list, nlp) { + if (nfscl_procdoesntexist(lp->nfsl_owner)) + nfscl_cleanup_common(clp, lp->nfsl_owner); } } NFSUNLOCKCLSTATE();