Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src Using cache_revlookup() leads to vnode races as it returns a...
details: https://anonhg.NetBSD.org/src/rev/f3a0fc85bcd0
branches: trunk
changeset: 756523:f3a0fc85bcd0
user: hannken <hannken%NetBSD.org@localhost>
date: Wed Jul 21 09:01:35 2010 +0000
description:
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.
diffstat:
share/man/man9/namecache.9 | 5 +++--
sys/kern/vfs_cache.c | 31 +++++++++++++++++++++++--------
sys/kern/vfs_getcwd.c | 41 ++++++++++++-----------------------------
3 files changed, 38 insertions(+), 39 deletions(-)
diffs (198 lines):
diff -r 968039ec97b8 -r f3a0fc85bcd0 share/man/man9/namecache.9
--- a/share/man/man9/namecache.9 Wed Jul 21 06:58:25 2010 +0000
+++ b/share/man/man9/namecache.9 Wed Jul 21 09:01:35 2010 +0000
@@ -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.
diff -r 968039ec97b8 -r f3a0fc85bcd0 sys/kern/vfs_cache.c
--- a/sys/kern/vfs_cache.c Wed Jul 21 06:58:25 2010 +0000
+++ b/sys/kern/vfs_cache.c Wed Jul 21 09:01:35 2010 +0000
@@ -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);
diff -r 968039ec97b8 -r f3a0fc85bcd0 sys/kern/vfs_getcwd.c
--- a/sys/kern/vfs_getcwd.c Wed Jul 21 06:58:25 2010 +0000
+++ b/sys/kern/vfs_getcwd.c Wed Jul 21 09:01:35 2010 +0000
@@ -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
Home |
Main Index |
Thread Index |
Old Index