From owner-dev-commits-src-branches@freebsd.org Sun May 9 03:04:04 2021 Return-Path: Delivered-To: dev-commits-src-branches@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 99316647D9D; Sun, 9 May 2021 03:04:04 +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 4Fd8Cm3zrRz4XdF; Sun, 9 May 2021 03:04:04 +0000 (UTC) (envelope-from git@FreeBSD.org) 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 7B93F1C78; Sun, 9 May 2021 03:04:04 +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 149344Xd069143; Sun, 9 May 2021 03:04:04 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 149344dX069142; Sun, 9 May 2021 03:04:04 GMT (envelope-from git) Date: Sun, 9 May 2021 03:04:04 GMT Message-Id: <202105090304.149344dX069142@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Rick Macklem Subject: git: 9223cea0795f - stable/12 - nfscl: fix delegation recall when the file is not open 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/stable/12 X-Git-Reftype: branch X-Git-Commit: 9223cea0795f1ef2a5bb856908acc78416bbc966 Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-branches@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commits to the stable branches of the FreeBSD src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 09 May 2021 03:04:04 -0000 The branch stable/12 has been updated by rmacklem: URL: https://cgit.FreeBSD.org/src/commit/?id=9223cea0795f1ef2a5bb856908acc78416bbc966 commit 9223cea0795f1ef2a5bb856908acc78416bbc966 Author: Rick Macklem AuthorDate: 2021-04-25 19:52:48 +0000 Commit: Rick Macklem CommitDate: 2021-05-09 03:00:38 +0000 nfscl: fix delegation recall when the file is not open Without this patch, if a NFSv4 server recalled a delegation when the file is not open, the renew thread would block in the NFS VOP_INACTIVE() trying to acquire the client state lock that it already holds. This patch fixes the problem by delaying the vrele() call until after the client state lock is released. This bug has been in the NFSv4 client for a long time, but since it only affects delegation when recalled due to another client opening the file, it got missed during previous testing. Until you have this patch in your client, you should avoid the use of delegations. (cherry picked from commit 02695ea8909d818ceaa726f90f889889dfd39fac) --- sys/fs/nfsclient/nfs_clstate.c | 53 ++++++++++++++++++++++++------------------ 1 file changed, 30 insertions(+), 23 deletions(-) diff --git a/sys/fs/nfsclient/nfs_clstate.c b/sys/fs/nfsclient/nfs_clstate.c index 0cade3e4178a..03c046c36ccc 100644 --- a/sys/fs/nfsclient/nfs_clstate.c +++ b/sys/fs/nfsclient/nfs_clstate.c @@ -152,7 +152,8 @@ static void nfscl_freedeleg(struct nfscldeleghead *, struct nfscldeleg *); static int nfscl_errmap(struct nfsrv_descript *, u_int32_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); + struct nfscldeleg *, vnode_t, struct ucred *, NFSPROC_T *, int, + vnode_t *); static void nfscl_freeopenowner(struct nfsclowner *, int); static void nfscl_cleandeleg(struct nfscldeleg *); static int nfscl_trydelegreturn(struct nfscldeleg *, struct ucred *, @@ -2532,6 +2533,7 @@ nfscl_renewthread(struct nfsclclient *clp, NFSPROC_T *p) struct nfsclrecalllayout *recallp; struct nfsclds *dsp; struct mount *mp; + vnode_t vp; cred = newnfs_getcred(); NFSLOCKCLSTATE(); @@ -2651,7 +2653,7 @@ tryagain: NFSUNLOCKCLSTATE(); newnfs_copycred(&dp->nfsdl_cred, cred); ret = nfscl_recalldeleg(clp, clp->nfsc_nmp, dp, - NULL, cred, p, 1); + NULL, cred, p, 1, &vp); if (!ret) { nfscl_cleandeleg(dp); TAILQ_REMOVE(&clp->nfsc_deleg, dp, @@ -2662,6 +2664,22 @@ tryagain: nfsstatsv1.cldelegates--; } NFSLOCKCLSTATE(); + /* + * The nfsc_lock must be released before doing + * vrele(), since it might call nfs_inactive(). + * For the unlikely case where the vnode failed + * to be acquired by nfscl_recalldeleg(), a + * VOP_RECLAIM() should be in progress and it + * will return the delegation. + */ + nfsv4_unlock(&clp->nfsc_lock, 0); + igotlock = 0; + if (vp != NULL) { + NFSUNLOCKCLSTATE(); + vrele(vp); + NFSLOCKCLSTATE(); + } + goto tryagain; } dp = ndp; } @@ -3906,16 +3924,18 @@ nfscl_lockt(vnode_t vp, struct nfsclclient *clp, u_int64_t off, static int nfscl_recalldeleg(struct nfsclclient *clp, struct nfsmount *nmp, struct nfscldeleg *dp, vnode_t vp, struct ucred *cred, NFSPROC_T *p, - int called_from_renewthread) + int called_from_renewthread, vnode_t *vpp) { struct nfsclowner *owp, *lowp, *nowp; struct nfsclopen *op, *lop; struct nfscllockowner *lp; struct nfscllock *lckp; struct nfsnode *np; - int error = 0, ret, gotvp = 0; + int error = 0, ret; if (vp == NULL) { + KASSERT(vpp != NULL, ("nfscl_recalldeleg: vpp NULL")); + *vpp = NULL; /* * First, get a vnode for the file. This is needed to do RPCs. */ @@ -3929,7 +3949,7 @@ nfscl_recalldeleg(struct nfsclclient *clp, struct nfsmount *nmp, return (0); } vp = NFSTOV(np); - gotvp = 1; + *vpp = vp; } else { np = VTONFS(vp); } @@ -3956,8 +3976,6 @@ nfscl_recalldeleg(struct nfsclclient *clp, struct nfsmount *nmp, * return now, so that the dirty buffer will be flushed * later. */ - if (gotvp != 0) - vrele(vp); return (ret); } @@ -3981,11 +3999,8 @@ nfscl_recalldeleg(struct nfsclclient *clp, struct nfsmount *nmp, owp, dp, cred, p); if (ret == NFSERR_STALECLIENTID || ret == NFSERR_STALEDONTRECOVER || - ret == NFSERR_BADSESSION) { - if (gotvp) - vrele(vp); + ret == NFSERR_BADSESSION) return (ret); - } if (ret) { nfscl_freeopen(lop, 1); if (!error) @@ -4013,11 +4028,8 @@ nfscl_recalldeleg(struct nfsclclient *clp, struct nfsmount *nmp, nfscl_freeopenowner(owp, 0); if (ret == NFSERR_STALECLIENTID || ret == NFSERR_STALEDONTRECOVER || - ret == NFSERR_BADSESSION) { - if (gotvp) - vrele(vp); + ret == NFSERR_BADSESSION) return (ret); - } if (ret) { nfscl_freeopen(lop, 1); if (!error) @@ -4038,17 +4050,12 @@ nfscl_recalldeleg(struct nfsclclient *clp, struct nfsmount *nmp, if (ret == NFSERR_STALESTATEID || ret == NFSERR_STALEDONTRECOVER || ret == NFSERR_STALECLIENTID || - ret == NFSERR_BADSESSION) { - if (gotvp) - vrele(vp); + ret == NFSERR_BADSESSION) return (ret); - } if (ret && !error) error = ret; } } - if (gotvp) - vrele(vp); return (error); } @@ -4460,7 +4467,7 @@ nfscl_removedeleg(vnode_t vp, NFSPROC_T *p, nfsv4stateid_t *stp) NFSUNLOCKCLSTATE(); cred = newnfs_getcred(); newnfs_copycred(&dp->nfsdl_cred, cred); - (void) nfscl_recalldeleg(clp, nmp, dp, vp, cred, p, 0); + nfscl_recalldeleg(clp, nmp, dp, vp, cred, p, 0, NULL); NFSFREECRED(cred); triedrecall = 1; NFSLOCKCLSTATE(); @@ -4559,7 +4566,7 @@ nfscl_renamedeleg(vnode_t fvp, nfsv4stateid_t *fstp, int *gotfdp, vnode_t tvp, NFSUNLOCKCLSTATE(); cred = newnfs_getcred(); newnfs_copycred(&dp->nfsdl_cred, cred); - (void) nfscl_recalldeleg(clp, nmp, dp, fvp, cred, p, 0); + nfscl_recalldeleg(clp, nmp, dp, fvp, cred, p, 0, NULL); NFSFREECRED(cred); triedrecall = 1; NFSLOCKCLSTATE();