[src/trunk]: src/sys Bracket do_sys_renameat() and nfsrv_rename() with fstrans.

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

details:   https://anonhg.NetBSD.org/src/rev/68f945c02ff1
branches:  trunk
changeset: 449058:68f945c02ff1
user:      hannken <hannken%NetBSD.org@localhost>
date:      Wed Feb 20 10:05:20 2019 +0000

description:
Bracket do_sys_renameat() and nfsrv_rename() with fstrans.

The v_mount field for vnodes on the same file system as "from"
is now stable for referenced vnodes.

VFS_RENAMELOCK no longer may use lock from an unreferenced and
freed "struct mount".

diffstat:

 sys/kern/vfs_syscalls.c |  53 +++++++++++++++++++-----------------------------
 sys/nfs/nfs_serv.c      |  10 ++++++--
 2 files changed, 28 insertions(+), 35 deletions(-)

diffs (157 lines):

diff -r 2854ca2bc214 -r 68f945c02ff1 sys/kern/vfs_syscalls.c
--- a/sys/kern/vfs_syscalls.c   Wed Feb 20 10:04:28 2019 +0000
+++ b/sys/kern/vfs_syscalls.c   Wed Feb 20 10:05:20 2019 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: vfs_syscalls.c,v 1.525 2019/02/19 06:55:28 mlelstv Exp $       */
+/*     $NetBSD: vfs_syscalls.c,v 1.526 2019/02/20 10:05:20 hannken Exp $       */
 
 /*-
  * Copyright (c) 2008, 2009 The NetBSD Foundation, Inc.
@@ -70,7 +70,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: vfs_syscalls.c,v 1.525 2019/02/19 06:55:28 mlelstv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: vfs_syscalls.c,v 1.526 2019/02/20 10:05:20 hannken Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_fileassoc.h"
@@ -4224,9 +4224,18 @@
         */
        fdvp = fnd.ni_dvp;
        fvp = fnd.ni_vp;
+       mp = fdvp->v_mount;
        KASSERT(fdvp != NULL);
        KASSERT(fvp != NULL);
        KASSERT((fdvp == fvp) || (VOP_ISLOCKED(fdvp) == LK_EXCLUSIVE));
+       /*
+        * Bracket the operation with fstrans_start()/fstrans_done().
+        *
+        * Inside the bracket this file system cannot be unmounted so
+        * a vnode on this file system cannot change its v_mount.
+        * A vnode on another file system may still change to dead mount.
+        */
+       fstrans_start(mp);
 
        /*
         * Make sure neither fdvp nor fvp is locked.
@@ -4311,38 +4320,16 @@
        }
 
        /*
-        * Get the mount point.  If the file system has been unmounted,
-        * which it may be because we're not holding any vnode locks,
-        * then v_mount will be NULL.  We're not really supposed to
-        * read v_mount without holding the vnode lock, but since we
-        * have fdvp referenced, if fdvp->v_mount changes then at worst
-        * it will be set to NULL, not changed to another mount point.
-        * And, of course, since it is up to the file system to
-        * determine the real lock order, we can't lock both fdvp and
-        * tdvp at the same time.
+        * Make sure the mount points match.  Although we don't hold
+        * any vnode locks, the v_mount on fdvp file system are stable.
+        *
+        * Unmounting another file system at an inopportune moment may
+        * cause tdvp to disappear and change its v_mount to dead.
+        *
+        * So in either case different v_mount means cross-device rename.
         */
-       mp = fdvp->v_mount;
-       if (mp == NULL) {
-               error = ENOENT;
-               goto abort1;
-       }
-
-       /*
-        * Make sure the mount points match.  Again, although we don't
-        * hold any vnode locks, the v_mount fields may change -- but
-        * at worst they will change to NULL, so this will never become
-        * a cross-device rename, because we hold vnode references.
-        *
-        * XXX Because nothing is locked and the compiler may reorder
-        * things here, unmounting the file system at an inopportune
-        * moment may cause rename to fail with EXDEV when it really
-        * should fail with ENOENT.
-        */
+       KASSERT(mp != NULL);
        tmp = tdvp->v_mount;
-       if (tmp == NULL) {
-               error = ENOENT;
-               goto abort1;
-       }
 
        if (mp != tmp) {
                error = EXDEV;
@@ -4497,6 +4484,7 @@
         * destroy the pathbufs.
         */
        VFS_RENAMELOCK_EXIT(mp);
+       fstrans_done(mp);
        goto out2;
 
 abort3:        if ((tvp != NULL) && (tvp != tdvp))
@@ -4510,6 +4498,7 @@
 abort0:        VOP_ABORTOP(fdvp, &fnd.ni_cnd);
        vrele(fdvp);
        vrele(fvp);
+       fstrans_done(mp);
 out2:  pathbuf_destroy(tpb);
 out1:  pathbuf_destroy(fpb);
 out0:  return error;
diff -r 2854ca2bc214 -r 68f945c02ff1 sys/nfs/nfs_serv.c
--- a/sys/nfs/nfs_serv.c        Wed Feb 20 10:04:28 2019 +0000
+++ b/sys/nfs/nfs_serv.c        Wed Feb 20 10:05:20 2019 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: nfs_serv.c,v 1.176 2019/02/03 03:19:28 mrg Exp $       */
+/*     $NetBSD: nfs_serv.c,v 1.177 2019/02/20 10:05:20 hannken Exp $   */
 
 /*
  * Copyright (c) 1989, 1993
@@ -55,7 +55,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: nfs_serv.c,v 1.176 2019/02/03 03:19:28 mrg Exp $");
+__KERNEL_RCSID(0, "$NetBSD: nfs_serv.c,v 1.177 2019/02/20 10:05:20 hannken Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -64,6 +64,7 @@
 #include <sys/namei.h>
 #include <sys/vnode.h>
 #include <sys/mount.h>
+#include <sys/fstrans.h>
 #include <sys/socket.h>
 #include <sys/socketvar.h>
 #include <sys/mbuf.h>
@@ -1957,12 +1958,13 @@
                }
                return (0);
        }
+       localfs = fromnd.ni_dvp->v_mount;
+       fstrans_start(localfs);
        if (fromnd.ni_dvp != fromnd.ni_vp) {
                VOP_UNLOCK(fromnd.ni_dvp);
        }
        fvp = fromnd.ni_vp;
 
-       localfs = fvp->v_mount;
        error = VFS_RENAMELOCK_ENTER(localfs);
        if (error) {
                VOP_ABORTOP(fromnd.ni_dvp, &fromnd.ni_cnd);
@@ -2130,6 +2132,7 @@
        pathbuf_destroy(fromnd.ni_pathbuf);
        fromnd.ni_pathbuf = NULL;
        fromnd.ni_cnd.cn_nameiop = 0;
+       fstrans_done(localfs);
        localfs = NULL;
        nfsm_reply(2 * NFSX_WCCDATA(v3));
        if (v3) {
@@ -2153,6 +2156,7 @@
        }
        if (localfs) {
                VFS_RENAMELOCK_EXIT(localfs);
+               fstrans_done(localfs);
        }
        if (fromnd.ni_cnd.cn_nameiop) {
                VOP_ABORTOP(fromnd.ni_dvp, &fromnd.ni_cnd);