tech-kern archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Cache_revlookup() and vfinddev() races
Both cache_revlookup() and vfinddev() lead to vnode races as they return
an unlocked and unreferenced vnode that may disappear before the caller
has a chance to reference the vnode.
With the attached diffs both functions reference the vnode while the list
they are working on is locked and return the vnode referenced.
Any objections?
--
Juergen Hannken-Illjes - hannken%eis.cs.tu-bs.de@localhost - TU Braunschweig
(Germany)
Index: share/man/man9/namecache.9
===================================================================
RCS file: /cvsroot/src/share/man/man9/namecache.9,v
retrieving revision 1.13
diff -p -u -2 -r1.13 namecache.9
--- share/man/man9/namecache.9 13 May 2009 22:46:04 -0000 1.13
+++ share/man/man9/namecache.9 18 Jul 2010 09:34:05 -0000
@@ -28,5 +28,5 @@
.\" POSSIBILITY OF SUCH DAMAGE.
.\"
-.Dd June 25, 2007
+.Dd July 9, 2010
.Dt NAMECACHE 9
.Os
@@ -159,4 +159,5 @@ 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"
Index: sys/kern/vfs_cache.c
===================================================================
RCS file: /cvsroot/src/sys/kern/vfs_cache.c,v
retrieving revision 1.85
diff -p -u -2 -r1.85 vfs_cache.c
--- sys/kern/vfs_cache.c 24 Jun 2010 13:03:11 -0000 1.85
+++ sys/kern/vfs_cache.c 18 Jul 2010 09:34:05 -0000
@@ -493,5 +493,5 @@ cache_lookup_raw(struct vnode *dvp, stru
* 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
@@ -509,4 +509,5 @@ cache_revlookup(struct vnode *vp, struct
struct ncvhashhead *nvcpp;
char *bp;
+ int error, nlen;
if (!doingcache)
@@ -533,8 +534,9 @@ cache_revlookup(struct vnode *vp, struct
#endif
COUNT(nchstats, ncs_revhits);
+ nlen = ncp->nc_nlen;
if (bufp) {
bp = *bpp;
- bp -= ncp->nc_nlen;
+ bp -= nlen;
if (bp <= bufp) {
*dvpp = NULL;
@@ -543,12 +545,25 @@ cache_revlookup(struct vnode *vp, struct
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);
}
Index: sys/kern/vfs_getcwd.c
===================================================================
RCS file: /cvsroot/src/sys/kern/vfs_getcwd.c,v
retrieving revision 1.45
diff -p -u -2 -r1.45 vfs_getcwd.c
--- sys/kern/vfs_getcwd.c 24 Jun 2010 13:03:11 -0000 1.45
+++ sys/kern/vfs_getcwd.c 18 Jul 2010 09:34:05 -0000
@@ -282,5 +282,4 @@ getcwd_getcache(struct vnode **lvpp, str
{
struct vnode *lvp, *uvp = NULL;
- char *obp = *bpp;
int error;
@@ -308,22 +307,5 @@ getcwd_getcache(struct vnode **lvpp, str
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;
@@ -559,5 +541,6 @@ out:
* 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
@@ -570,21 +553,21 @@ vnode_to_path(char *path, size_t len, st
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);
/*
Index: share/man/man9/vnode.9
===================================================================
RCS file: /cvsroot/src/share/man/man9/vnode.9,v
retrieving revision 1.49
diff -p -u -2 -r1.49 vnode.9
--- share/man/man9/vnode.9 6 Jun 2010 08:01:31 -0000 1.49
+++ share/man/man9/vnode.9 18 Jul 2010 09:35:13 -0000
@@ -28,5 +28,5 @@
.\" POSSIBILITY OF SUCH DAMAGE.
.\"
-.Dd June 6, 2010
+.Dd July 17, 2010
.Dt VNODE 9
.Os
@@ -655,5 +655,5 @@ is used for the console and kernfs speci
.It Fn vfinddev "dev" "vtype" "vpp"
Lookup a vnode by device number.
-The vnode is returned in the address specified by
+The vnode is referenced and returned in the address specified by
.Fa vpp .
.It Fn vdevgone "int maj" "int min" "int minh" "enum vtype type"
Index: sys/kern/vfs_subr.c
===================================================================
RCS file: /cvsroot/src/sys/kern/vfs_subr.c,v
retrieving revision 1.408
diff -p -u -2 -r1.408 vfs_subr.c
--- sys/kern/vfs_subr.c 1 Jul 2010 13:00:56 -0000 1.408
+++ sys/kern/vfs_subr.c 18 Jul 2010 09:35:14 -0000
@@ -1994,5 +1994,5 @@ vgone(vnode_t *vp)
/*
- * Lookup a vnode by device number.
+ * Lookup a vnode by device number and return it referenced.
*/
int
@@ -2000,16 +2000,20 @@ vfinddev(dev_t dev, enum vtype type, vno
{
vnode_t *vp;
- int rc = 0;
mutex_enter(&device_lock);
for (vp = specfs_hash[SPECHASH(dev)]; vp; vp = vp->v_specnext) {
- if (dev != vp->v_rdev || type != vp->v_type)
- continue;
- *vpp = vp;
- rc = 1;
- break;
+ if (dev == vp->v_rdev && type == vp->v_type)
+ break;
}
+ if (vp == NULL) {
+ mutex_exit(&device_lock);
+ return 0;
+ }
+ mutex_enter(&vp->v_interlock);
mutex_exit(&device_lock);
- return (rc);
+ if (vget(vp, LK_INTERLOCK) != 0)
+ return 0;
+ *vpp = vp;
+ return 1;
}
@@ -3367,7 +3371,9 @@ rawdev_mounted(struct vnode *vp, struct
blkdev = devsw_chr2blk(dev);
if (blkdev != NODEV) {
- vfinddev(blkdev, VBLK, &bvp);
- if (bvp != NULL)
+ if (vfinddev(blkdev, VBLK, &bvp) != 0) {
d_type = (cdev->d_flag & D_TYPEMASK);
+ /* XXX: what if bvp disappears? */
+ vrele(bvp);
+ }
}
}
Index: sys/miscfs/kernfs/kernfs_subr.c
===================================================================
RCS file: /cvsroot/src/sys/miscfs/kernfs/kernfs_subr.c,v
retrieving revision 1.21
diff -p -u -2 -r1.21 kernfs_subr.c
--- sys/miscfs/kernfs/kernfs_subr.c 1 Jul 2010 13:00:56 -0000 1.21
+++ sys/miscfs/kernfs/kernfs_subr.c 18 Jul 2010 09:35:14 -0000
@@ -177,6 +177,8 @@ kernfs_allocvp(struct mount *mp, struct
}
vp = fvp;
- if (vget(fvp, LK_EXCLUSIVE))
+ if (vn_lock(fvp, LK_EXCLUSIVE)) {
+ vrele(fvp);
goto loop;
+ }
*vpp = vp;
mutex_exit(&kfs_hashlock);
Index: sys/miscfs/kernfs/kernfs_vnops.c
===================================================================
RCS file: /cvsroot/src/sys/miscfs/kernfs/kernfs_vnops.c,v
retrieving revision 1.142
diff -p -u -2 -r1.142 kernfs_vnops.c
--- sys/miscfs/kernfs/kernfs_vnops.c 24 Jun 2010 13:03:16 -0000 1.142
+++ sys/miscfs/kernfs/kernfs_vnops.c 18 Jul 2010 09:35:14 -0000
@@ -1169,4 +1169,5 @@ kernfs_readdir(void *v)
!vfinddev(*dp, kt->kt_vtype, &fvp))
continue;
+ vrele(fvp);
}
if (kt->kt_tag == KFSmsgbuf) {
@@ -1251,4 +1252,5 @@ kernfs_readdir(void *v)
!vfinddev(*dp, kt->kt_vtype, &fvp))
continue;
+ vrele(fvp);
}
d.d_namlen = kt->kt_namlen;
Index: sys/external/bsd/drm/dist/bsd-core/drm_bufs.c
===================================================================
RCS file: /cvsroot/src/sys/external/bsd/drm/dist/bsd-core/drm_bufs.c,v
retrieving revision 1.7
diff -p -u -2 -r1.7 drm_bufs.c
--- sys/external/bsd/drm/dist/bsd-core/drm_bufs.c 26 Jan 2010 08:01:26
-0000 1.7
+++ sys/external/bsd/drm/dist/bsd-core/drm_bufs.c 18 Jul 2010 09:35:14
-0000
@@ -1147,4 +1147,7 @@ int drm_mapbufs(struct drm_device *dev,
done:
request->count = dma->buf_count;
+#if defined(__NetBSD__)
+ vrele(vn);
+#endif
DRM_DEBUG("%d buffers, retcode = %d\n", request->count, retcode);
Home |
Main Index |
Thread Index |
Old Index