Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src The current implementation of vn_lock() is racy. Modificati...
details: https://anonhg.NetBSD.org/src/rev/f9494e617d6b
branches: trunk
changeset: 327107:f9494e617d6b
user: hannken <hannken%NetBSD.org@localhost>
date: Thu Feb 27 16:51:37 2014 +0000
description:
The current implementation of vn_lock() is racy. Modification of
the vnode operations vector for active vnodes is unsafe because it
is not known whether deadfs or the original file system will be
called.
- Pass down LK_RETRY to the lock operation (hint for deadfs only).
- Change deadfs lock operation to return ENOENT if LK_RETRY is unset.
- Change all other lock operations to check for dead vnode once
the vnode is locked and unlock and return ENOENT in this case.
With these changes in place vnode lock operations will never succeed
after vclean() has marked the vnode as VI_XLOCK and before vclean()
has changed the operations vector.
Adresses PR kern/37706 (Forced unmount of file systems is unsafe)
Discussed on tech-kern.
Welcome to 6.99.33
diffstat:
share/man/man9/vnodeops.9 | 9 ++-
share/man/man9/vnsubr.9 | 9 +--
sys/coda/coda_vnops.c | 12 +---
sys/fs/adosfs/adutil.c | 9 ++-
sys/fs/cd9660/cd9660_node.c | 8 ++-
sys/fs/efs/efs_ihash.c | 8 ++-
sys/fs/filecorefs/filecore_node.c | 8 ++-
sys/fs/hfs/hfs_nhash.c | 8 ++-
sys/fs/ptyfs/ptyfs_subr.c | 8 ++-
sys/fs/tmpfs/tmpfs_vnops.c | 6 +-
sys/fs/union/union_vnops.c | 45 ++++++++++++++++--
sys/kern/vfs_vnode.c | 62 +++++++++++++-------------
sys/kern/vfs_vnops.c | 34 ++++----------
sys/miscfs/deadfs/dead_vnops.c | 10 ++--
sys/miscfs/fdesc/fdesc_vnops.c | 7 +-
sys/miscfs/genfs/genfs.h | 6 ++-
sys/miscfs/genfs/genfs_vnops.c | 90 +++++++++++++++++++++++++++++++++++--
sys/miscfs/genfs/layer_extern.h | 3 +-
sys/miscfs/genfs/layer_vnops.c | 44 +++++++++++++++++-
sys/miscfs/kernfs/kernfs_subr.c | 8 ++-
sys/miscfs/nullfs/null_vnops.c | 5 +-
sys/miscfs/overlay/overlay_vnops.c | 7 +-
sys/miscfs/umapfs/umap_vnops.c | 5 +-
sys/nfs/nfs_node.c | 7 +-
sys/sys/param.h | 4 +-
sys/ufs/chfs/chfs_ihash.c | 6 +-
sys/ufs/lfs/ulfs_ihash.c | 8 ++-
sys/ufs/ufs/ufs_ihash.c | 8 ++-
28 files changed, 304 insertions(+), 140 deletions(-)
diffs (truncated from 1144 to 300 lines):
diff -r 8dfd34e13743 -r f9494e617d6b share/man/man9/vnodeops.9
--- a/share/man/man9/vnodeops.9 Thu Feb 27 16:47:48 2014 +0000
+++ b/share/man/man9/vnodeops.9 Thu Feb 27 16:51:37 2014 +0000
@@ -1,4 +1,4 @@
-.\" $NetBSD: vnodeops.9,v 1.92 2014/02/07 15:29:21 hannken Exp $
+.\" $NetBSD: vnodeops.9,v 1.93 2014/02/27 16:51:37 hannken Exp $
.\"
.\" Copyright (c) 2001, 2005, 2006 The NetBSD Foundation, Inc.
.\" All rights reserved.
@@ -27,7 +27,7 @@
.\" ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
.\" POSSIBILITY OF SUCH DAMAGE.
.\"
-.Dd February 7, 2014
+.Dd February 27, 2014
.Dt VNODEOPS 9
.Os
.Sh NAME
@@ -1071,6 +1071,11 @@
contains
.Dv LK_NOWAIT
and the lock is busy, the operation will return immediately with an error code.
+If
+.Fa flags
+contains
+.Dv LK_RETRY
+this is a hint the caller wants the lock on dead vnodes too.
If the operation is successful zero is returned, otherwise an
appropriate error code is returned.
.Fn VOP_LOCK
diff -r 8dfd34e13743 -r f9494e617d6b share/man/man9/vnsubr.9
--- a/share/man/man9/vnsubr.9 Thu Feb 27 16:47:48 2014 +0000
+++ b/share/man/man9/vnsubr.9 Thu Feb 27 16:51:37 2014 +0000
@@ -1,4 +1,4 @@
-.\" $NetBSD: vnsubr.9,v 1.41 2011/01/30 07:04:48 rmind Exp $
+.\" $NetBSD: vnsubr.9,v 1.42 2014/02/27 16:51:37 hannken Exp $
.\"
.\" Copyright (c) 2001, 2005, 2006 The NetBSD Foundation, Inc.
.\" All rights reserved.
@@ -27,7 +27,7 @@
.\" ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
.\" POSSIBILITY OF SUCH DAMAGE.
.\"
-.Dd January 30, 2010
+.Dd February 27, 2014
.Dt VNSUBR 9
.Os
.Sh NAME
@@ -138,14 +138,11 @@
.It LK_NOWAIT
do not sleep to await lock
.It LK_RETRY
-retry lock operation until locked
+allow lock operation on dead vnode
.El
.Pp
If the operation is successful zero is returned, otherwise an
appropriate error code is returned.
-The vnode interlock
-.Em v_interlock
-is released on return.
.Pp
.Fn vn_lock
must not be called when the vnode's reference count is zero.
diff -r 8dfd34e13743 -r f9494e617d6b sys/coda/coda_vnops.c
--- a/sys/coda/coda_vnops.c Thu Feb 27 16:47:48 2014 +0000
+++ b/sys/coda/coda_vnops.c Thu Feb 27 16:51:37 2014 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: coda_vnops.c,v 1.94 2014/02/07 15:29:21 hannken Exp $ */
+/* $NetBSD: coda_vnops.c,v 1.95 2014/02/27 16:51:37 hannken Exp $ */
/*
*
@@ -46,7 +46,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: coda_vnops.c,v 1.94 2014/02/07 15:29:21 hannken Exp $");
+__KERNEL_RCSID(0, "$NetBSD: coda_vnops.c,v 1.95 2014/02/27 16:51:37 hannken Exp $");
#include <sys/param.h>
#include <sys/systm.h>
@@ -229,8 +229,7 @@
MARK_ENTRY(CODA_OPEN_STATS);
- if (!VOP_ISLOCKED(vp))
- VOP_LOCK(vp, LK_EXCLUSIVE);
+ KASSERT(VOP_ISLOCKED(vp));
/* Check for open of control file. */
if (IS_CTL_VP(vp)) {
/* if (WRITABLE(flag)) */
@@ -1745,8 +1744,6 @@
/*
* Obtain vnode from mount point and inode.
- * XXX VFS_VGET does not clearly define locked/referenced state of
- * returned vnode.
*/
error = VFS_VGET(mp, ino, vpp);
if (error) {
@@ -1757,8 +1754,7 @@
/* share the underlying vnode lock with the coda vnode */
mutex_obj_hold((*vpp)->v_interlock);
uvm_obj_setlock(&uvp->v_uobj, (*vpp)->v_interlock);
- if (!VOP_ISLOCKED(*vpp))
- VOP_LOCK(*vpp, LK_EXCLUSIVE);
+ KASSERT(VOP_ISLOCKED(*vpp));
return(0);
}
diff -r 8dfd34e13743 -r f9494e617d6b sys/fs/adosfs/adutil.c
--- a/sys/fs/adosfs/adutil.c Thu Feb 27 16:47:48 2014 +0000
+++ b/sys/fs/adosfs/adutil.c Thu Feb 27 16:51:37 2014 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: adutil.c,v 1.15 2011/06/12 03:35:52 rmind Exp $ */
+/* $NetBSD: adutil.c,v 1.16 2014/02/27 16:51:37 hannken Exp $ */
/*
* Copyright (c) 1994 Christian E. Hopps
@@ -32,7 +32,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: adutil.c,v 1.15 2011/06/12 03:35:52 rmind Exp $");
+__KERNEL_RCSID(0, "$NetBSD: adutil.c,v 1.16 2014/02/27 16:51:37 hannken Exp $");
#include <sys/param.h>
#include <sys/vnode.h>
@@ -85,7 +85,10 @@
void
adosfs_ainshash(struct adosfsmount *amp, struct anode *ap)
{
- VOP_LOCK(ATOV(ap), LK_EXCLUSIVE);
+ int error __diagused;
+
+ error = VOP_LOCK(ATOV(ap), LK_EXCLUSIVE);
+ KASSERT(error == 0);
mutex_enter(&adosfs_hashlock);
LIST_INSERT_HEAD(&->anodetab[AHASH(ap->block)], ap, link);
diff -r 8dfd34e13743 -r f9494e617d6b sys/fs/cd9660/cd9660_node.c
--- a/sys/fs/cd9660/cd9660_node.c Thu Feb 27 16:47:48 2014 +0000
+++ b/sys/fs/cd9660/cd9660_node.c Thu Feb 27 16:51:37 2014 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: cd9660_node.c,v 1.29 2011/06/12 03:35:52 rmind Exp $ */
+/* $NetBSD: cd9660_node.c,v 1.30 2014/02/27 16:51:38 hannken Exp $ */
/*-
* Copyright (c) 1982, 1986, 1989, 1994
@@ -37,7 +37,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: cd9660_node.c,v 1.29 2011/06/12 03:35:52 rmind Exp $");
+__KERNEL_RCSID(0, "$NetBSD: cd9660_node.c,v 1.30 2014/02/27 16:51:38 hannken Exp $");
#include <sys/param.h>
#include <sys/systm.h>
@@ -170,6 +170,7 @@
cd9660_ihashins(struct iso_node *ip)
{
struct ihashhead *ipp;
+ int error __diagused;
KASSERT(mutex_owned(&cd9660_hashlock));
@@ -178,7 +179,8 @@
LIST_INSERT_HEAD(ipp, ip, i_hash);
mutex_exit(&cd9660_ihash_lock);
- VOP_LOCK(ITOV(ip), LK_EXCLUSIVE);
+ error = VOP_LOCK(ITOV(ip), LK_EXCLUSIVE);
+ KASSERT(error == 0);
}
/*
diff -r 8dfd34e13743 -r f9494e617d6b sys/fs/efs/efs_ihash.c
--- a/sys/fs/efs/efs_ihash.c Thu Feb 27 16:47:48 2014 +0000
+++ b/sys/fs/efs/efs_ihash.c Thu Feb 27 16:51:37 2014 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: efs_ihash.c,v 1.9 2012/04/29 20:27:31 dsl Exp $ */
+/* $NetBSD: efs_ihash.c,v 1.10 2014/02/27 16:51:38 hannken Exp $ */
/*
* Copyright (c) 1982, 1986, 1989, 1991, 1993
@@ -36,7 +36,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: efs_ihash.c,v 1.9 2012/04/29 20:27:31 dsl Exp $");
+__KERNEL_RCSID(0, "$NetBSD: efs_ihash.c,v 1.10 2014/02/27 16:51:38 hannken Exp $");
#include <sys/param.h>
#include <sys/systm.h>
@@ -162,11 +162,13 @@
efs_ihashins(struct efs_inode *eip)
{
struct ihashhead *ipp;
+ int error __diagused;
KASSERT(mutex_owned(&efs_hashlock));
/* lock the inode, then put it on the appropriate hash list */
- VOP_LOCK(EFS_ITOV(eip), LK_EXCLUSIVE);
+ error = VOP_LOCK(EFS_ITOV(eip), LK_EXCLUSIVE);
+ KASSERT(error == 0);
mutex_enter(&efs_ihash_lock);
ipp = &ihashtbl[INOHASH(eip->ei_dev, eip->ei_number)];
diff -r 8dfd34e13743 -r f9494e617d6b sys/fs/filecorefs/filecore_node.c
--- a/sys/fs/filecorefs/filecore_node.c Thu Feb 27 16:47:48 2014 +0000
+++ b/sys/fs/filecorefs/filecore_node.c Thu Feb 27 16:51:37 2014 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: filecore_node.c,v 1.25 2011/06/12 03:35:52 rmind Exp $ */
+/* $NetBSD: filecore_node.c,v 1.26 2014/02/27 16:51:38 hannken Exp $ */
/*-
* Copyright (c) 1982, 1986, 1989, 1994
@@ -67,7 +67,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: filecore_node.c,v 1.25 2011/06/12 03:35:52 rmind Exp $");
+__KERNEL_RCSID(0, "$NetBSD: filecore_node.c,v 1.26 2014/02/27 16:51:38 hannken Exp $");
#include <sys/param.h>
#include <sys/systm.h>
@@ -188,13 +188,15 @@
filecore_ihashins(struct filecore_node *ip)
{
struct ihashhead *ipp;
+ int error __diagused;
mutex_enter(&filecore_ihash_lock);
ipp = &filecorehashtbl[INOHASH(ip->i_dev, ip->i_number)];
LIST_INSERT_HEAD(ipp, ip, i_hash);
mutex_exit(&filecore_ihash_lock);
- VOP_LOCK(ITOV(ip), LK_EXCLUSIVE);
+ error = VOP_LOCK(ITOV(ip), LK_EXCLUSIVE);
+ KASSERT(error == 0);
}
/*
diff -r 8dfd34e13743 -r f9494e617d6b sys/fs/hfs/hfs_nhash.c
--- a/sys/fs/hfs/hfs_nhash.c Thu Feb 27 16:47:48 2014 +0000
+++ b/sys/fs/hfs/hfs_nhash.c Thu Feb 27 16:51:37 2014 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: hfs_nhash.c,v 1.12 2011/06/12 03:35:53 rmind Exp $ */
+/* $NetBSD: hfs_nhash.c,v 1.13 2014/02/27 16:51:38 hannken Exp $ */
/*-
* Copyright (c) 2005, 2007 The NetBSD Foundation, Inc.
@@ -59,7 +59,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: hfs_nhash.c,v 1.12 2011/06/12 03:35:53 rmind Exp $");
+__KERNEL_RCSID(0, "$NetBSD: hfs_nhash.c,v 1.13 2014/02/27 16:51:38 hannken Exp $");
#include <sys/param.h>
#include <sys/systm.h>
@@ -148,9 +148,11 @@
hfs_nhashinsert(struct hfsnode *hp)
{
struct nhashhead *hpp;
+ int error __diagused;
/* lock the inode, then put it on the appropriate hash list */
- VOP_LOCK(HTOV(hp), LK_EXCLUSIVE);
+ error = VOP_LOCK(HTOV(hp), LK_EXCLUSIVE);
+ KASSERT(error == 0);
mutex_enter(&hfs_nhash_lock);
hpp = &nhashtbl[HNOHASH(hp->h_dev, hp->h_rec.u.cnid, hp->h_fork)];
diff -r 8dfd34e13743 -r f9494e617d6b sys/fs/ptyfs/ptyfs_subr.c
--- a/sys/fs/ptyfs/ptyfs_subr.c Thu Feb 27 16:47:48 2014 +0000
+++ b/sys/fs/ptyfs/ptyfs_subr.c Thu Feb 27 16:51:37 2014 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: ptyfs_subr.c,v 1.25 2012/10/24 23:36:15 christos Exp $ */
+/* $NetBSD: ptyfs_subr.c,v 1.26 2014/02/27 16:51:38 hannken Exp $ */
/*
* Copyright (c) 1993
@@ -73,7 +73,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ptyfs_subr.c,v 1.25 2012/10/24 23:36:15 christos Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ptyfs_subr.c,v 1.26 2014/02/27 16:51:38 hannken Exp $");
#include <sys/param.h>
#include <sys/systm.h>
@@ -397,9 +397,11 @@
ptyfs_hashins(struct ptyfsnode *pp)
{
struct ptyfs_hashhead *ppp;
+ int error __diagused;
Home |
Main Index |
Thread Index |
Old Index