From 054148f215bd88d828e93f776269e94548111428 Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Fri, 21 Jan 2022 12:09:14 +0000 Subject: [PATCH 24/36] specfs: Avoid race with revoke freeing specnode during spec_open. --- sys/miscfs/specfs/spec_vnops.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/sys/miscfs/specfs/spec_vnops.c b/sys/miscfs/specfs/spec_vnops.c index fec1519a6e0d..689e37134891 100644 --- a/sys/miscfs/specfs/spec_vnops.c +++ b/sys/miscfs/specfs/spec_vnops.c @@ -615,11 +615,6 @@ spec_open(void *v) * many other subsystems, we must not hold the vnode lock while * calling .d_open, so release it now and reacquire it when * done. - * - * XXX The vnode may be revoked concurrently while we have the - * vnode lock released. If this happens, the sn and sd - * pointers may be invalidated, but we don't do anything to - * avoid touching them after we're done here. */ VOP_UNLOCK(vp); switch (vp->v_type) { @@ -681,7 +676,8 @@ spec_open(void *v) /* * If it has been revoked since we released the vnode lock and * reacquired it, then spec_node_revoke has closed it, and we - * must fail with EBADF. + * must fail with EBADF without touching sn or sd which may be + * freed at this point. * * Otherwise, if opening it failed, back out and release the * open reference. @@ -709,16 +705,20 @@ spec_open(void *v) * but no .d_close (***) */ mutex_enter(&device_lock); - if (sn->sn_gone) { + mutex_enter(vp->v_interlock); + if (vdead_check(vp, VDEAD_NOWAIT) != 0) { if (error == 0) error = EBADF; - } else if (error != 0) { - sd->sd_opencnt--; - sn->sn_opencnt--; - if (vp->v_type == VBLK) - sd->sd_bdevvp = NULL; - + } else { + KASSERT(!sn->sn_gone); + if (error != 0) { + sd->sd_opencnt--; + sn->sn_opencnt--; + if (vp->v_type == VBLK) + sd->sd_bdevvp = NULL; + } } + mutex_exit(vp->v_interlock); mutex_exit(&device_lock); /* If anything went wrong, we're done. */