[src/trunk]: src/sys/nfs As VOP_GETATTR() needs a shared lock at least move t...

[hannken <hannken%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/dcaa9bb09985
branches:  trunk
changeset: 365838:dcaa9bb09985
user:      hannken <hannken%NetBSD.org@localhost>
date:      Wed Apr 27 17:38:52 2022 +0000

description:
As VOP_GETATTR() needs a shared lock at least move the preopattr lookup
inside nfs_namei() where we may lock the start directory without violating
the lock order.

diffstat:

 sys/nfs/nfs_serv.c    |  68 ++++++++++++++++----------------------------------
 sys/nfs/nfs_srvsubs.c |  11 ++++++--
 sys/nfs/nfs_var.h     |   6 ++--
 3 files changed, 33 insertions(+), 52 deletions(-)

diffs (211 lines):

diff -r 78b01ddda733 -r dcaa9bb09985 sys/nfs/nfs_serv.c
--- a/sys/nfs/nfs_serv.c        Wed Apr 27 14:50:35 2022 +0000
+++ b/sys/nfs/nfs_serv.c        Wed Apr 27 17:38:52 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: nfs_serv.c,v 1.182 2021/09/16 20:17:47 andvar Exp $    */
+/*     $NetBSD: nfs_serv.c,v 1.183 2022/04/27 17:38:52 hannken Exp $   */
 
 /*
  * Copyright (c) 1989, 1993
@@ -55,7 +55,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: nfs_serv.c,v 1.182 2021/09/16 20:17:47 andvar Exp $");
+__KERNEL_RCSID(0, "$NetBSD: nfs_serv.c,v 1.183 2022/04/27 17:38:52 hannken Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -422,7 +422,8 @@
        nd.ni_cnd.cn_nameiop = LOOKUP;
        nd.ni_cnd.cn_flags = LOCKLEAF;
        error = nfs_namei(&nd, &nsfh, len, slp, nam, &md, &dpos,
-               &dirp, lwp, (nfsd->nd_flag & ND_KERBAUTH), pubflag);
+               &dirp, NULL, NULL,
+               lwp, (nfsd->nd_flag & ND_KERBAUTH), pubflag);
 
        if (!error && pubflag) {
                if (nd.ni_vp->v_type == VDIR && nfs_pub.np_index != NULL &&
@@ -1447,10 +1448,8 @@
        nd.ni_cnd.cn_nameiop = CREATE;
        nd.ni_cnd.cn_flags = LOCKPARENT | LOCKLEAF;
        error = nfs_namei(&nd, &nsfh, len, slp, nam, &md, &dpos,
-               &dirp, lwp, (nfsd->nd_flag & ND_KERBAUTH), false);
-       if (dirp && v3) {
-               dirfor_ret = VOP_GETATTR(dirp, &dirfor, cred);
-       }
+               &dirp, (v3 ? &dirfor_ret : NULL), &dirfor,
+               lwp, (nfsd->nd_flag & ND_KERBAUTH), false);
        if (error) {
                nfsm_reply(NFSX_WCCDATA(v3));
                nfsm_srvwcc_data(dirfor_ret, &dirfor, diraft_ret, &diraft);
@@ -1689,9 +1688,8 @@
        nd.ni_cnd.cn_nameiop = CREATE;
        nd.ni_cnd.cn_flags = LOCKPARENT | LOCKLEAF;
        error = nfs_namei(&nd, &nsfh, len, slp, nam, &md, &dpos,
-               &dirp, lwp, (nfsd->nd_flag & ND_KERBAUTH), false);
-       if (dirp)
-               dirfor_ret = VOP_GETATTR(dirp, &dirfor, cred);
+               &dirp, &dirfor_ret, &dirfor,
+               lwp, (nfsd->nd_flag & ND_KERBAUTH), false);
        if (error) {
                nfsm_reply(NFSX_WCCDATA(1));
                nfsm_srvwcc_data(dirfor_ret, &dirfor, diraft_ret, &diraft);
@@ -1845,14 +1843,8 @@
        nd.ni_cnd.cn_nameiop = DELETE;
        nd.ni_cnd.cn_flags = LOCKPARENT | LOCKLEAF;
        error = nfs_namei(&nd, &nsfh, len, slp, nam, &md, &dpos,
-               &dirp, lwp, (nfsd->nd_flag & ND_KERBAUTH), false);
-       if (error == 0 && dirp && v3) {
-               if (nd.ni_dvp == nd.ni_vp)
-                       vn_lock(dirp, LK_SHARED | LK_RETRY);
-               dirfor_ret = VOP_GETATTR(dirp, &dirfor, cred);
-               if (nd.ni_dvp == nd.ni_vp)
-                       VOP_UNLOCK(dirp);
-       }
+               &dirp, (v3 ? &dirfor_ret : NULL), &dirfor,
+               lwp, (nfsd->nd_flag & ND_KERBAUTH), false);
        if (!error) {
                vp = nd.ni_vp;
                if (vp->v_type == VDIR) {
@@ -1944,14 +1936,8 @@
        fromnd.ni_cnd.cn_nameiop = DELETE;
        fromnd.ni_cnd.cn_flags = LOCKPARENT;
        error = nfs_namei(&fromnd, &fnsfh, len, slp, nam, &md,
-               &dpos, &fdirp, lwp, (nfsd->nd_flag & ND_KERBAUTH), false);
-       if (error == 0 && fdirp && v3) {
-               if (fromnd.ni_dvp == fromnd.ni_vp)
-                       vn_lock(fdirp, LK_SHARED | LK_RETRY);
-               fdirfor_ret = VOP_GETATTR(fdirp, &fdirfor, cred);
-               if (fromnd.ni_dvp == fromnd.ni_vp)
-                       VOP_UNLOCK(fdirp);
-       }
+               &dpos, &fdirp, (v3 ? &fdirfor_ret : NULL), &fdirfor,
+               lwp, (nfsd->nd_flag & ND_KERBAUTH), false);
        if (error) {
                fromnd.ni_cnd.cn_nameiop = 0;
                nfsm_reply(2 * NFSX_WCCDATA(v3));
@@ -2021,10 +2007,8 @@
        tond.ni_cnd.cn_nameiop = RENAME;
        tond.ni_cnd.cn_flags = LOCKPARENT | LOCKLEAF | NOCACHE;
        error = nfs_namei(&tond, &tnsfh, len2, slp, nam, &md,
-               &dpos, &tdirp, lwp, (nfsd->nd_flag & ND_KERBAUTH), false);
-       if (tdirp && v3) {
-               tdirfor_ret = VOP_GETATTR(tdirp, &tdirfor, cred);
-       }
+               &dpos, &tdirp, (v3 ? &tdirfor_ret : NULL), &tdirfor,
+               lwp, (nfsd->nd_flag & ND_KERBAUTH), false);
        if (error) {
                VFS_RENAMELOCK_EXIT(localfs);
                VOP_ABORTOP(fromnd.ni_dvp, &fromnd.ni_cnd);
@@ -2218,10 +2202,8 @@
        nd.ni_cnd.cn_nameiop = CREATE;
        nd.ni_cnd.cn_flags = LOCKPARENT;
        error = nfs_namei(&nd, &dnsfh, len, slp, nam, &md, &dpos,
-               &dirp, lwp, (nfsd->nd_flag & ND_KERBAUTH), false);
-       if (dirp && v3) {
-               dirfor_ret = VOP_GETATTR(dirp, &dirfor, cred);
-       }
+               &dirp, (v3 ? &dirfor_ret : NULL), &dirfor,
+               lwp, (nfsd->nd_flag & ND_KERBAUTH), false);
        if (error)
                goto out1;
        xp = nd.ni_vp;
@@ -2310,10 +2292,8 @@
        nd.ni_cnd.cn_nameiop = CREATE;
        nd.ni_cnd.cn_flags = LOCKPARENT;
        error = nfs_namei(&nd, &nsfh, len, slp, nam, &md, &dpos,
-               &dirp, lwp, (nfsd->nd_flag & ND_KERBAUTH), false);
-       if (dirp && v3) {
-               dirfor_ret = VOP_GETATTR(dirp, &dirfor, cred);
-       }
+               &dirp, (v3 ? &dirfor_ret : NULL), &dirfor,
+               lwp, (nfsd->nd_flag & ND_KERBAUTH), false);
        if (error)
                goto out;
        abort = 1;
@@ -2455,10 +2435,8 @@
        nd.ni_cnd.cn_nameiop = CREATE;
        nd.ni_cnd.cn_flags = LOCKPARENT;
        error = nfs_namei(&nd, &nsfh, len, slp, nam, &md, &dpos,
-               &dirp, lwp, (nfsd->nd_flag & ND_KERBAUTH), false);
-       if (dirp && v3) {
-               dirfor_ret = VOP_GETATTR(dirp, &dirfor, cred);
-       }
+               &dirp, (v3 ? &dirfor_ret : NULL), &dirfor,
+               lwp, (nfsd->nd_flag & ND_KERBAUTH), false);
        if (error) {
                if (nd.ni_pathbuf != NULL) {
                        pathbuf_destroy(nd.ni_pathbuf);
@@ -2579,10 +2557,8 @@
        nd.ni_cnd.cn_nameiop = DELETE;
        nd.ni_cnd.cn_flags = LOCKPARENT | LOCKLEAF;
        error = nfs_namei(&nd, &nsfh, len, slp, nam, &md, &dpos,
-               &dirp, lwp, (nfsd->nd_flag & ND_KERBAUTH), false);
-       if (dirp && v3) {
-               dirfor_ret = VOP_GETATTR(dirp, &dirfor, cred);
-       }
+               &dirp, (v3 ? &dirfor_ret : NULL), &dirfor,
+               lwp, (nfsd->nd_flag & ND_KERBAUTH), false);
        if (error) {
                if (nd.ni_pathbuf != NULL) {
                        pathbuf_destroy(nd.ni_pathbuf);
diff -r 78b01ddda733 -r dcaa9bb09985 sys/nfs/nfs_srvsubs.c
--- a/sys/nfs/nfs_srvsubs.c     Wed Apr 27 14:50:35 2022 +0000
+++ b/sys/nfs/nfs_srvsubs.c     Wed Apr 27 17:38:52 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: nfs_srvsubs.c,v 1.15 2020/01/17 20:08:09 ad Exp $      */
+/*     $NetBSD: nfs_srvsubs.c,v 1.16 2022/04/27 17:38:52 hannken Exp $ */
 
 /*
  * Copyright (c) 1989, 1993
@@ -70,7 +70,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: nfs_srvsubs.c,v 1.15 2020/01/17 20:08:09 ad Exp $");
+__KERNEL_RCSID(0, "$NetBSD: nfs_srvsubs.c,v 1.16 2022/04/27 17:38:52 hannken Exp $");
 
 #include <sys/param.h>
 #include <sys/proc.h>
@@ -116,7 +116,7 @@
  * it is not.
  */
 int
-nfs_namei(struct nameidata *ndp, nfsrvfh_t *nsfh, uint32_t len, struct nfssvc_sock *slp, struct mbuf *nam, struct mbuf **mdp, char **dposp, struct vnode **retdirp, struct lwp *l, int kerbflag, int 
pubflag)
+nfs_namei(struct nameidata *ndp, nfsrvfh_t *nsfh, uint32_t len, struct nfssvc_sock *slp, struct mbuf *nam, struct mbuf **mdp, char **dposp, struct vnode **retdirp, int *dirattr_retp, struct vattr 
*dirattrp, struct lwp *l, int kerbflag, int pubflag)
 {
        int i, rem;
        struct mbuf *md;
@@ -188,6 +188,11 @@
                cnp->cn_flags |= RDONLY;
 
        *retdirp = dp;
+       if (dirattr_retp != NULL) {
+               vn_lock(dp, LK_SHARED | LK_RETRY);
+               *dirattr_retp = VOP_GETATTR(dp, dirattrp, ndp->ni_cnd.cn_cred);
+               VOP_UNLOCK(dp);
+       }
 
        if (pubflag) {
                /*
diff -r 78b01ddda733 -r dcaa9bb09985 sys/nfs/nfs_var.h
--- a/sys/nfs/nfs_var.h Wed Apr 27 14:50:35 2022 +0000
+++ b/sys/nfs/nfs_var.h Wed Apr 27 17:38:52 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: nfs_var.h,v 1.95 2021/06/04 10:44:58 hannken Exp $     */
+/*     $NetBSD: nfs_var.h,v 1.96 2022/04/27 17:38:52 hannken Exp $     */
 
 /*-
  * Copyright (c) 1996 The NetBSD Foundation, Inc.
@@ -271,8 +271,8 @@
 int nfs_check_wccdata(struct nfsnode *, const struct timespec *,
        struct timespec *, bool);
 int nfs_namei(struct nameidata *, nfsrvfh_t *, uint32_t, struct nfssvc_sock *,
-       struct mbuf *, struct mbuf **, char **, struct vnode **, struct lwp *,
-       int, int);
+       struct mbuf *, struct mbuf **, char **, struct vnode **,
+       int *, struct vattr *, struct lwp *, int, int);
 void nfs_zeropad(struct mbuf *, int, int);
 void nfsm_srvwcc(struct nfsrv_descript *, int, struct vattr *, int,
        struct vattr *, struct mbuf **, char **);