d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

vfs: fix lock recursion in vrele

Browse Source 
vrele is supposed to be called with an unlocked vnode, but this was never
asserted for if v_usecount was > 0. For such counts the lock is never touched
by the routine. As a result the kernel has several consumers which expect
vunref semantics and get away with calling vrele since they happen to never do
it when this is the last reference (and for some of them this may happen to be
a guarantee).

Work around the problem by changing vrele semantics to tolerate being called
with a lock. This eliminates a possible bug where the lock is already held and
vputx takes it anyway.

Reviewed by:	kib
Tested by:	pho
Differential Revision:	https://reviews.freebsd.org/D23528
This commit is contained in:
Mateusz Guzik 2020-02-10 13:54:34 +00:00
parent d1e5538758
commit cd951a0d8e
Notes: svn2git 2020-12-20 02:59:44 +00:00 
svn path=/head/; revision=357729
1 changed files with 23 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
sys/kern/vfs_subr.c
View File

@ -3170,6 +3170,7 @@ static void
vputx(struct vnode *vp, enum vputx_op func) vputx(struct vnode *vp, enum vputx_op func)
{ {
int error; int error;
bool want_unlock;
KASSERT(vp != NULL, ("vputx: null vp")); KASSERT(vp != NULL, ("vputx: null vp"));
if (func == VPUTX_VUNREF) if (func == VPUTX_VUNREF)
@ -3221,13 +3222,31 @@ vputx(struct vnode *vp, enum vputx_op func)
* as VI_DOINGINACT to avoid recursion. * as VI_DOINGINACT to avoid recursion.
*/ */
vp->v_iflag |= VI_OWEINACT; vp->v_iflag |= VI_OWEINACT;
want_unlock = false;
error = 0;
switch (func) { switch (func) {
case VPUTX_VRELE: case VPUTX_VRELE:
error = vn_lock(vp, LK_EXCLUSIVE | LK_INTERLOCK); switch (VOP_ISLOCKED(vp)) {
VI_LOCK(vp); case LK_EXCLUSIVE:
break;
case LK_EXCLOTHER:
case 0:
want_unlock = true;
error = vn_lock(vp, LK_EXCLUSIVE | LK_INTERLOCK);
VI_LOCK(vp);
break;
default:
/*
* The lock has at least one sharer, but we have no way
* to conclude whether this is us. Play it safe and
* defer processing.
*/
error = EAGAIN;
break;
}
break; break;
case VPUTX_VPUT: case VPUTX_VPUT:
error = 0; want_unlock = true;
if (VOP_ISLOCKED(vp) != LK_EXCLUSIVE) { if (VOP_ISLOCKED(vp) != LK_EXCLUSIVE) {
error = VOP_LOCK(vp, LK_UPGRADE | LK_INTERLOCK | error = VOP_LOCK(vp, LK_UPGRADE | LK_INTERLOCK |
LK_NOWAIT); LK_NOWAIT);
@ -3235,7 +3254,6 @@ vputx(struct vnode *vp, enum vputx_op func)
} }
break; break;
case VPUTX_VUNREF: case VPUTX_VUNREF:
error = 0;
if (VOP_ISLOCKED(vp) != LK_EXCLUSIVE) { if (VOP_ISLOCKED(vp) != LK_EXCLUSIVE) {
error = VOP_LOCK(vp, LK_TRYUPGRADE | LK_INTERLOCK); error = VOP_LOCK(vp, LK_TRYUPGRADE | LK_INTERLOCK);
VI_LOCK(vp); VI_LOCK(vp);
@ -3244,7 +3262,7 @@ vputx(struct vnode *vp, enum vputx_op func)
} }
if (error == 0) { if (error == 0) {
vinactive(vp); vinactive(vp);
if (func != VPUTX_VUNREF) if (want_unlock)
VOP_UNLOCK(vp); VOP_UNLOCK(vp);
vdropl(vp); vdropl(vp);
} else { } else {