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