Module Name: src
Committed By: hannken
Date: Wed Feb 20 10:05:20 UTC 2019
Modified Files:
src/sys/kern: vfs_syscalls.c
src/sys/nfs: nfs_serv.c
Log Message:
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".
To generate a diff of this commit:
cvs rdiff -u -r1.525 -r1.526 src/sys/kern/vfs_syscalls.c
cvs rdiff -u -r1.176 -r1.177 src/sys/nfs/nfs_serv.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/kern/vfs_syscalls.c
diff -u src/sys/kern/vfs_syscalls.c:1.525 src/sys/kern/vfs_syscalls.c:1.526
--- src/sys/kern/vfs_syscalls.c:1.525 Tue Feb 19 06:55:28 2019
+++ src/sys/kern/vfs_syscalls.c Wed Feb 20 10:05:20 2019
@@ -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 @@ do_sys_renameat(struct lwp *l, int fromf
*/
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 @@ do_sys_renameat(struct lwp *l, int fromf
}
/*
- * 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.
- */
- 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.
+ * Make sure the mount points match. Although we don't hold
+ * any vnode locks, the v_mount on fdvp file system are stable.
*
- * 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.
+ * 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.
*/
+ KASSERT(mp != NULL);
tmp = tdvp->v_mount;
- if (tmp == NULL) {
- error = ENOENT;
- goto abort1;
- }
if (mp != tmp) {
error = EXDEV;
@@ -4497,6 +4484,7 @@ do_sys_renameat(struct lwp *l, int fromf
* destroy the pathbufs.
*/
VFS_RENAMELOCK_EXIT(mp);
+ fstrans_done(mp);
goto out2;
abort3: if ((tvp != NULL) && (tvp != tdvp))
@@ -4510,6 +4498,7 @@ abort1: VOP_ABORTOP(tdvp, &tnd.ni_cnd);
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;
Index: src/sys/nfs/nfs_serv.c
diff -u src/sys/nfs/nfs_serv.c:1.176 src/sys/nfs/nfs_serv.c:1.177
--- src/sys/nfs/nfs_serv.c:1.176 Sun Feb 3 03:19:28 2019
+++ src/sys/nfs/nfs_serv.c Wed Feb 20 10:05:20 2019
@@ -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 @@ __KERNEL_RCSID(0, "$NetBSD: nfs_serv.c,v
#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 @@ nfsrv_rename(struct nfsrv_descript *nfsd
}
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 @@ out1:
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 @@ nfsmout:
}
if (localfs) {
VFS_RENAMELOCK_EXIT(localfs);
+ fstrans_done(localfs);
}
if (fromnd.ni_cnd.cn_nameiop) {
VOP_ABORTOP(fromnd.ni_dvp, &fromnd.ni_cnd);
