Date: Tue, 19 Oct 2021 03:36:30 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: 16f3a89d08b2 - stable/12 - nfscl: Fix a deadlock related to the NFSv4 clientID lock Message-ID: <202110190336.19J3aUcW048273@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch stable/12 has been updated by rmacklem: URL: https://cgit.FreeBSD.org/src/commit/?id=16f3a89d08b2deffccace6f605a9e9883cfbd4e2 commit 16f3a89d08b2deffccace6f605a9e9883cfbd4e2 Author: Rick Macklem <rmacklem@FreeBSD.org> AuthorDate: 2021-10-12 04:58:24 +0000 Commit: Rick Macklem <rmacklem@FreeBSD.org> CommitDate: 2021-10-19 03:31:47 +0000 nfscl: Fix a deadlock related to the NFSv4 clientID lock Without this patch, it is possible for a process doing an NFSv4 Open/create of a file to block to allow another process to acquire the exclusive lock on the clientID when holding a shared lock on the clientID. As such, both processes deadlock, with one wanting the exclusive lock, while the other holds the shared lock. This deadlock is unlikely to occur unless delegations are in use on the NFSv4 mount. This patch fixes the problem by not deferring to the process waiting for the exclusive lock when a shared lock (reference cnt) is already held by the process. This problem was detected during a recent NFSv4 interoperability testing event held by the IETF working group. (cherry picked from commit 120b20bdf49630cf2a7dbc5f93b9e985e1f4f198) --- sys/fs/nfs/nfs_var.h | 4 ++-- sys/fs/nfsclient/nfs_clrpcops.c | 12 ++++++------ sys/fs/nfsclient/nfs_clstate.c | 29 ++++++++++++++++------------- sys/fs/nfsclient/nfs_clvfsops.c | 2 +- 4 files changed, 25 insertions(+), 22 deletions(-) diff --git a/sys/fs/nfs/nfs_var.h b/sys/fs/nfs/nfs_var.h index 0f656601e531..5177a1c2c065 100644 --- a/sys/fs/nfs/nfs_var.h +++ b/sys/fs/nfs/nfs_var.h @@ -540,13 +540,13 @@ void nfsrpc_bindconnsess(CLIENT *, void *, struct ucred *); /* nfs_clstate.c */ int nfscl_open(vnode_t, u_int8_t *, int, u_int32_t, int, struct ucred *, NFSPROC_T *, struct nfsclowner **, struct nfsclopen **, - int *, int *, int); + int *, int *, int, bool); int nfscl_getstateid(vnode_t, u_int8_t *, int, u_int32_t, int, struct ucred *, NFSPROC_T *, nfsv4stateid_t *, void **); void nfscl_ownerrelease(struct nfsmount *, struct nfsclowner *, int, int, int); void nfscl_openrelease(struct nfsmount *, struct nfsclopen *, int, int); int nfscl_getcl(struct mount *, struct ucred *, NFSPROC_T *, int, - struct nfsclclient **); + bool, struct nfsclclient **); struct nfsclclient *nfscl_findcl(struct nfsmount *); void nfscl_clientrelease(struct nfsclclient *); void nfscl_freelock(struct nfscllock *, int); diff --git a/sys/fs/nfsclient/nfs_clrpcops.c b/sys/fs/nfsclient/nfs_clrpcops.c index 3ff23a8b147b..3d26a1dfda88 100644 --- a/sys/fs/nfsclient/nfs_clrpcops.c +++ b/sys/fs/nfsclient/nfs_clrpcops.c @@ -365,7 +365,7 @@ else printf(" fhl=0\n"); do { dp = NULL; error = nfscl_open(vp, nfhp->nfh_fh, nfhp->nfh_len, mode, 1, - cred, p, NULL, &op, &newone, &ret, 1); + cred, p, NULL, &op, &newone, &ret, 1, true); if (error) { return (error); } @@ -1997,7 +1997,7 @@ nfsrpc_create(vnode_t dvp, char *name, int namelen, struct vattr *vap, dp = NULL; error = nfscl_open(dvp, NULL, 0, (NFSV4OPEN_ACCESSWRITE | NFSV4OPEN_ACCESSREAD), 0, cred, p, &owp, NULL, &newone, - NULL, 1); + NULL, 1, true); if (error) return (error); if (nmp->nm_clp != NULL) @@ -2283,7 +2283,7 @@ nfsrpc_createv4(vnode_t dvp, char *name, int namelen, struct vattr *vap, */ error = nfscl_open(dvp, nfhp->nfh_fh, nfhp->nfh_len, (NFSV4OPEN_ACCESSWRITE | NFSV4OPEN_ACCESSREAD), 0, - cred, p, NULL, &op, &newone, NULL, 0); + cred, p, NULL, &op, &newone, NULL, 0, false); if (error) goto nfsmout; op->nfso_stateid = stateid; @@ -3886,7 +3886,7 @@ nfsrpc_advlock(vnode_t vp, off_t size, int op, struct flock *fl, do { nd->nd_repstat = 0; if (op == F_GETLK) { - error = nfscl_getcl(vnode_mount(vp), cred, p, 1, &clp); + error = nfscl_getcl(vnode_mount(vp), cred, p, 1, true, &clp); if (error) return (error); error = nfscl_lockt(vp, clp, off, len, fl, p, id, flags); @@ -3903,7 +3903,7 @@ nfsrpc_advlock(vnode_t vp, off_t size, int op, struct flock *fl, * We must loop around for all lockowner cases. */ callcnt = 0; - error = nfscl_getcl(vnode_mount(vp), cred, p, 1, &clp); + error = nfscl_getcl(vnode_mount(vp), cred, p, 1, true, &clp); if (error) return (error); do { @@ -7537,7 +7537,7 @@ nfsrpc_createlayout(vnode_t dvp, char *name, int namelen, struct vattr *vap, */ error = nfscl_open(dvp, nfhp->nfh_fh, nfhp->nfh_len, (NFSV4OPEN_ACCESSWRITE | NFSV4OPEN_ACCESSREAD), 0, - cred, p, NULL, &op, &newone, NULL, 0); + cred, p, NULL, &op, &newone, NULL, 0, false); if (error != 0) goto nfsmout; op->nfso_stateid = stateid; diff --git a/sys/fs/nfsclient/nfs_clstate.c b/sys/fs/nfsclient/nfs_clstate.c index 9965b7ca7e4f..92688fcfbe28 100644 --- a/sys/fs/nfsclient/nfs_clstate.c +++ b/sys/fs/nfsclient/nfs_clstate.c @@ -213,7 +213,7 @@ static short *nfscl_cberrmap[] = { int nfscl_open(vnode_t vp, u_int8_t *nfhp, int fhlen, u_int32_t amode, int usedeleg, struct ucred *cred, NFSPROC_T *p, struct nfsclowner **owpp, - struct nfsclopen **opp, int *newonep, int *retp, int lockit) + struct nfsclopen **opp, int *newonep, int *retp, int lockit, bool firstref) { struct nfsclclient *clp; struct nfsclowner *owp, *nowp; @@ -239,7 +239,7 @@ nfscl_open(vnode_t vp, u_int8_t *nfhp, int fhlen, u_int32_t amode, int usedeleg, if (nfhp != NULL) nop = malloc(sizeof (struct nfsclopen) + fhlen - 1, M_NFSCLOPEN, M_WAITOK); - ret = nfscl_getcl(vnode_mount(vp), cred, p, 1, &clp); + ret = nfscl_getcl(vnode_mount(vp), cred, p, 1, firstref, &clp); if (ret != 0) { free(nowp, M_NFSCLOWNER); if (nop != NULL) @@ -778,7 +778,7 @@ nfscl_openrelease(struct nfsmount *nmp, struct nfsclopen *op, int error, */ int nfscl_getcl(struct mount *mp, struct ucred *cred, NFSPROC_T *p, - int start_renewthread, struct nfsclclient **clpp) + int start_renewthread, bool firstref, struct nfsclclient **clpp) { struct nfsclclient *clp; struct nfsclclient *newclp = NULL; @@ -850,14 +850,16 @@ nfscl_getcl(struct mount *mp, struct ucred *cred, NFSPROC_T *p, NFSCLSTATEMUTEXPTR, mp); if (igotlock == 0) { /* - * Call nfsv4_lock() with "iwantlock == 0" so that it will - * wait for a pending exclusive lock request. This gives the - * exclusive lock request priority over this shared lock - * request. + * Call nfsv4_lock() with "iwantlock == 0" on the firstref so + * that it will wait for a pending exclusive lock request. + * This gives the exclusive lock request priority over this + * shared lock request. * An exclusive lock on nfsc_lock is used mainly for server - * crash recoveries. + * crash recoveries and delegation recalls. */ - nfsv4_lock(&clp->nfsc_lock, 0, NULL, NFSCLSTATEMUTEXPTR, mp); + if (firstref) + nfsv4_lock(&clp->nfsc_lock, 0, NULL, NFSCLSTATEMUTEXPTR, + mp); nfsv4_getref(&clp->nfsc_lock, NULL, NFSCLSTATEMUTEXPTR, mp); } if (igotlock == 0 && NFSCL_FORCEDISM(mp)) { @@ -1032,7 +1034,8 @@ nfscl_getbytelock(vnode_t vp, u_int64_t off, u_int64_t len, if (recovery) clp = rclp; else - error = nfscl_getcl(vnode_mount(vp), cred, p, 1, &clp); + error = nfscl_getcl(vnode_mount(vp), cred, p, 1, true, + &clp); } if (error) { free(nlp, M_NFSCLLOCKOWNER); @@ -1370,7 +1373,7 @@ nfscl_checkwritelocked(vnode_t vp, struct flock *fl, end = NFS64BITSSET; } - error = nfscl_getcl(vnode_mount(vp), cred, p, 1, &clp); + error = nfscl_getcl(vnode_mount(vp), cred, p, 1, true, &clp); if (error) return (1); nfscl_filllockowner(id, own, flags); @@ -3140,7 +3143,7 @@ nfscl_getclose(vnode_t vp, struct nfsclclient **clpp) struct nfsfh *nfhp; int error, notdecr; - error = nfscl_getcl(vnode_mount(vp), NULL, NULL, 1, &clp); + error = nfscl_getcl(vnode_mount(vp), NULL, NULL, 1, true, &clp); if (error) return (error); *clpp = clp; @@ -3215,7 +3218,7 @@ nfscl_doclose(vnode_t vp, struct nfsclclient **clpp, NFSPROC_T *p) struct nfsclrecalllayout *recallp; int error; - error = nfscl_getcl(vnode_mount(vp), NULL, NULL, 1, &clp); + error = nfscl_getcl(vnode_mount(vp), NULL, NULL, 1, true, &clp); if (error) return (error); *clpp = clp; diff --git a/sys/fs/nfsclient/nfs_clvfsops.c b/sys/fs/nfsclient/nfs_clvfsops.c index 1a1e1b122994..49a08e3787b4 100644 --- a/sys/fs/nfsclient/nfs_clvfsops.c +++ b/sys/fs/nfsclient/nfs_clvfsops.c @@ -1524,7 +1524,7 @@ mountnfs(struct nfs_args *argp, struct mount *mp, struct sockaddr *nam, /* For NFSv4.1, get the clientid now. */ if (nmp->nm_minorvers > 0) { NFSCL_DEBUG(3, "at getcl\n"); - error = nfscl_getcl(mp, cred, td, 0, &clp); + error = nfscl_getcl(mp, cred, td, 0, true, &clp); NFSCL_DEBUG(3, "aft getcl=%d\n", error); if (error != 0) goto bad;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202110190336.19J3aUcW048273>