From 78b0bfe2a53e16b6f7b367a489752589e5447f02 Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Sun, 23 Jan 2022 19:30:41 +0000 Subject: [PATCH 31/36] specfs: Take an I/O reference across bdev/cdev_open. Needed because revoke must wait for all I/O operations, including pending opens (which can hang indefinitely, e.g. ttys), to stop using data structures before detach can free them. --- sys/miscfs/specfs/spec_vnops.c | 38 +++++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 7 deletions(-) diff --git a/sys/miscfs/specfs/spec_vnops.c b/sys/miscfs/specfs/spec_vnops.c index ca57aee74dba..279fe4a8991f 100644 --- a/sys/miscfs/specfs/spec_vnops.c +++ b/sys/miscfs/specfs/spec_vnops.c @@ -641,10 +641,10 @@ spec_open(void *v) } */ *ap = v; struct lwp *l = curlwp; struct vnode *vp = ap->a_vp; - dev_t dev; + dev_t dev, dev1; int error; enum kauth_device_req req; - specnode_t *sn; + specnode_t *sn, *sn1; specdev_t *sd; spec_ioctl_t ioctl; u_int gen = 0; @@ -752,18 +752,34 @@ spec_open(void *v) } /* - * Open the device. If .d_open returns ENXIO (device not - * configured), the driver may not be loaded, so try - * autoloading a module and then try .d_open again if anything - * got loaded. - * * Because the opening the device may block indefinitely, * e.g. when opening a tty, and loading a module may cross into * many other subsystems, we must not hold the vnode lock while * calling .d_open, so release it now and reacquire it when * done. + * + * Take an I/O reference so that any concurrent + * spec_node_revoke will wait for us to finish calling .d_open. + * The vnode can't be dead at this point because we have it + * locked. Note that if revoked, the driver will interrupt + * .d_open before spec_node_revoke starts waiting for I/O to + * drain so this doesn't deadlock. */ VOP_UNLOCK(vp); + error = spec_io_enter(vp, &sn1, &dev1); + if (error) { + vn_lock(vp, LK_EXCLUSIVE | LK_RETRY); + return error; + } + KASSERT(sn1 == sn); + KASSERT(dev1 == dev); + + /* + * Open the device. If .d_open returns ENXIO (device not + * configured), the driver may not be loaded, so try + * autoloading a module and then try .d_open again if anything + * got loaded. + */ switch (vp->v_type) { case VCHR: do { @@ -818,6 +834,14 @@ spec_open(void *v) default: __unreachable(); } + + /* + * Release the I/O reference now that we have called .d_open, + * and reacquire the vnode lock. At this point, the device may + * have been revoked, so we must tread carefully -- can't touch + * sn or sd until we verify the vnode is not dead. + */ + spec_io_exit(vp, sn); vn_lock(vp, LK_EXCLUSIVE | LK_RETRY); /*