Module Name: src
Committed By: hannken
Date: Wed Jul 21 09:01:36 UTC 2010
Modified Files:
src/share/man/man9: namecache.9
src/sys/kern: vfs_cache.c vfs_getcwd.c
Log Message:
Using cache_revlookup() leads to vnode races as it returns an unreferenced
vnode that may disappear before the caller has a chance to reference it.
Reference the vnode while the name cache is locked.
No objections on tech-kern.
To generate a diff of this commit:
cvs rdiff -u -r1.13 -r1.14 src/share/man/man9/namecache.9
cvs rdiff -u -r1.85 -r1.86 src/sys/kern/vfs_cache.c
cvs rdiff -u -r1.45 -r1.46 src/sys/kern/vfs_getcwd.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/share/man/man9/namecache.9
diff -u src/share/man/man9/namecache.9:1.13 src/share/man/man9/namecache.9:1.14
--- src/share/man/man9/namecache.9:1.13 Wed May 13 22:46:04 2009
+++ src/share/man/man9/namecache.9 Wed Jul 21 09:01:35 2010
@@ -1,4 +1,4 @@
-.\" $NetBSD: namecache.9,v 1.13 2009/05/13 22:46:04 wiz Exp $
+.\" $NetBSD: namecache.9,v 1.14 2010/07/21 09:01:35 hannken Exp $
.\"
.\" Copyright (c) 2001 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 June 25, 2007
+.Dd July 21, 2010
.Dt NAMECACHE 9
.Os
.Sh NAME
@@ -158,6 +158,7 @@
immediately before
.Fa bpp ,
and move bpp backwards to point at the start of it.
+If the lookup succeeds, the vnode is referenced.
Returns 0 on success, -1 on cache miss, positive errno on failure.
.It Fn cache_enter "dvp" "vp" "cnp"
Add an entry to the cache.
Index: src/sys/kern/vfs_cache.c
diff -u src/sys/kern/vfs_cache.c:1.85 src/sys/kern/vfs_cache.c:1.86
--- src/sys/kern/vfs_cache.c:1.85 Thu Jun 24 13:03:11 2010
+++ src/sys/kern/vfs_cache.c Wed Jul 21 09:01:36 2010
@@ -1,4 +1,4 @@
-/* $NetBSD: vfs_cache.c,v 1.85 2010/06/24 13:03:11 hannken Exp $ */
+/* $NetBSD: vfs_cache.c,v 1.86 2010/07/21 09:01:36 hannken Exp $ */
/*-
* Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: vfs_cache.c,v 1.85 2010/06/24 13:03:11 hannken Exp $");
+__KERNEL_RCSID(0, "$NetBSD: vfs_cache.c,v 1.86 2010/07/21 09:01:36 hannken Exp $");
#include "opt_ddb.h"
#include "opt_revcache.h"
@@ -492,7 +492,7 @@
/*
* Scan cache looking for name of directory entry pointing at vp.
*
- * Fill in dvpp.
+ * If the lookup succeeds the vnode is referenced and stored in dvpp.
*
* If bufp is non-NULL, also place the name in the buffer which starts
* at bufp, immediately before *bpp, and move bpp backwards to point
@@ -508,6 +508,7 @@
struct vnode *dvp;
struct ncvhashhead *nvcpp;
char *bp;
+ int error, nlen;
if (!doingcache)
goto out;
@@ -532,24 +533,38 @@
panic("cache_revlookup: found entry for ..");
#endif
COUNT(nchstats, ncs_revhits);
+ nlen = ncp->nc_nlen;
if (bufp) {
bp = *bpp;
- bp -= ncp->nc_nlen;
+ bp -= nlen;
if (bp <= bufp) {
*dvpp = NULL;
mutex_exit(&ncp->nc_lock);
mutex_exit(namecache_lock);
return (ERANGE);
}
- memcpy(bp, ncp->nc_name, ncp->nc_nlen);
+ memcpy(bp, ncp->nc_name, nlen);
*bpp = bp;
}
- /* XXX MP: how do we know dvp won't evaporate? */
+ if (vtryget(dvp)) {
+ mutex_exit(&ncp->nc_lock);
+ mutex_exit(namecache_lock);
+ } else {
+ mutex_enter(&dvp->v_interlock);
+ mutex_exit(&ncp->nc_lock);
+ mutex_exit(namecache_lock);
+ error = vget(dvp, LK_NOWAIT | LK_INTERLOCK);
+ if (error) {
+ KASSERT(error == EBUSY);
+ if (bufp)
+ (*bpp) += nlen;
+ *dvpp = NULL;
+ return -1;
+ }
+ }
*dvpp = dvp;
- mutex_exit(&ncp->nc_lock);
- mutex_exit(namecache_lock);
return (0);
}
mutex_exit(&ncp->nc_lock);
Index: src/sys/kern/vfs_getcwd.c
diff -u src/sys/kern/vfs_getcwd.c:1.45 src/sys/kern/vfs_getcwd.c:1.46
--- src/sys/kern/vfs_getcwd.c:1.45 Thu Jun 24 13:03:11 2010
+++ src/sys/kern/vfs_getcwd.c Wed Jul 21 09:01:36 2010
@@ -1,4 +1,4 @@
-/* $NetBSD: vfs_getcwd.c,v 1.45 2010/06/24 13:03:11 hannken Exp $ */
+/* $NetBSD: vfs_getcwd.c,v 1.46 2010/07/21 09:01:36 hannken Exp $ */
/*-
* Copyright (c) 1999 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: vfs_getcwd.c,v 1.45 2010/06/24 13:03:11 hannken Exp $");
+__KERNEL_RCSID(0, "$NetBSD: vfs_getcwd.c,v 1.46 2010/07/21 09:01:36 hannken Exp $");
#include <sys/param.h>
#include <sys/systm.h>
@@ -281,7 +281,6 @@
char *bufp)
{
struct vnode *lvp, *uvp = NULL;
- char *obp = *bpp;
int error;
lvp = *lvpp;
@@ -307,24 +306,7 @@
*/
VOP_UNLOCK(lvp);
- error = vget(uvp, LK_EXCLUSIVE | LK_RETRY);
-
- /*
- * Verify that vget succeeded while we were waiting for the
- * lock.
- */
- if (error) {
-
- /*
- * Oops, we missed. If the vget failed, get our lock back
- * then rewind the `bp' and tell the caller to try things
- * the hard way.
- */
- *uvpp = NULL;
- vn_lock(lvp, LK_EXCLUSIVE | LK_RETRY);
- *bpp = obp;
- return -1;
- }
+ vn_lock(uvp, LK_EXCLUSIVE | LK_RETRY);
vrele(lvp);
*lvpp = NULL;
@@ -558,7 +540,8 @@
/*
* Try to find a pathname for a vnode. Since there is no mapping
* vnode -> parent directory, this needs the NAMECACHE_ENTER_REVERSE
- * option to work (to make cache_revlookup succeed).
+ * option to work (to make cache_revlookup succeed). Caller holds a
+ * reference to the vnode.
*/
int
vnode_to_path(char *path, size_t len, struct vnode *vp, struct lwp *curl,
@@ -569,23 +552,23 @@
char *bp, *bend;
struct vnode *dvp;
+ KASSERT(vp->v_usecount > 0);
+
bp = bend = &path[len];
*(--bp) = '\0';
- error = vget(vp, LK_EXCLUSIVE | LK_RETRY);
+ error = vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
if (error != 0)
return error;
error = cache_revlookup(vp, &dvp, &bp, path);
- vput(vp);
+ VOP_UNLOCK(vp);
if (error != 0)
return (error == -1 ? ENOENT : error);
- error = vget(dvp, 0);
- if (error != 0)
- return error;
*(--bp) = '/';
- /* XXX GETCWD_CHECK_ACCESS == 0x0001 */
- error = getcwd_common(dvp, NULL, &bp, path, len / 2, 1, curl);
+ error = getcwd_common(dvp, NULL, &bp, path, len / 2,
+ GETCWD_CHECK_ACCESS, curl);
+ vrele(dvp);
/*
* Strip off emulation path for emulated processes looking at
