[src/trunk]: src/sys/miscfs/specfs specfs: Prevent new opens while close is w...

[riastradh <riastradh%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/5937d1a6daa7
branches:  trunk
changeset: 364515:5937d1a6daa7
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Mon Mar 28 12:37:09 2022 +0000

description:
specfs: Prevent new opens while close is waiting to drain.

Otherwise, bdev/cdev_close could have cancelled all _existing_ opens,
and waited for them to complete (and freed resources used by them) --
but a new one could start, and hang (e.g., a tty), at the same time
spec_close tries to drain all pending I/O operations, one of which
(the new open) is now hanging indefinitely.

Preventing the new open from even starting until bdev/cdev_close is
finished and all I/O operations have drained avoids this deadlock.

diffstat:

 sys/miscfs/specfs/spec_vnops.c |  32 ++++++++++++++++++++++++++++----
 sys/miscfs/specfs/specdev.h    |   3 ++-
 2 files changed, 30 insertions(+), 5 deletions(-)

diffs (93 lines):

diff -r dbe8bce6a8b7 -r 5937d1a6daa7 sys/miscfs/specfs/spec_vnops.c
--- a/sys/miscfs/specfs/spec_vnops.c    Mon Mar 28 12:37:01 2022 +0000
+++ b/sys/miscfs/specfs/spec_vnops.c    Mon Mar 28 12:37:09 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: spec_vnops.c,v 1.203 2022/03/28 12:37:01 riastradh Exp $       */
+/*     $NetBSD: spec_vnops.c,v 1.204 2022/03/28 12:37:09 riastradh Exp $       */
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.203 2022/03/28 12:37:01 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.204 2022/03/28 12:37:09 riastradh Exp $");
 
 #include <sys/param.h>
 #include <sys/proc.h>
@@ -397,6 +397,7 @@
                sd->sd_bdevvp = NULL;
                sd->sd_iocnt = 0;
                sd->sd_opened = false;
+               sd->sd_closing = false;
                sn->sn_dev = sd;
                sd = NULL;
        } else {
@@ -734,8 +735,17 @@
        case VCHR:
                /*
                 * Character devices can accept opens from multiple
-                * vnodes.
+                * vnodes.  But first, wait for any close to finish.
+                * Wait under the vnode lock so we don't have to worry
+                * about the vnode being revoked while we wait.
                 */
+               while (sd->sd_closing) {
+                       error = cv_wait_sig(&specfs_iocv, &device_lock);
+                       if (error)
+                               break;
+               }
+               if (error)
+                       break;
                sd->sd_opencnt++;
                sn->sn_opencnt++;
                break;
@@ -1605,8 +1615,10 @@
                    count + 1);
                sd->sd_bdevvp = NULL;
        }
-       if (count == 0)
+       if (count == 0) {
                sd->sd_opened = false;
+               sd->sd_closing = true;
+       }
        mutex_exit(&device_lock);
 
        if (count != 0)
@@ -1631,6 +1643,18 @@
         */
        spec_io_drain(sd);
 
+       /*
+        * Wake any spec_open calls waiting for close to finish -- do
+        * this before reacquiring the vnode lock, because spec_open
+        * holds the vnode lock while waiting, so doing this after
+        * reacquiring the lock would deadlock.
+        */
+       mutex_enter(&device_lock);
+       KASSERT(sd->sd_closing);
+       sd->sd_closing = false;
+       cv_broadcast(&specfs_iocv);
+       mutex_exit(&device_lock);
+
        if (!(flags & FNONBLOCK))
                vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
 
diff -r dbe8bce6a8b7 -r 5937d1a6daa7 sys/miscfs/specfs/specdev.h
--- a/sys/miscfs/specfs/specdev.h       Mon Mar 28 12:37:01 2022 +0000
+++ b/sys/miscfs/specfs/specdev.h       Mon Mar 28 12:37:09 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: specdev.h,v 1.49 2022/03/28 12:36:51 riastradh Exp $   */
+/*     $NetBSD: specdev.h,v 1.50 2022/03/28 12:37:09 riastradh Exp $   */
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -80,6 +80,7 @@
        dev_t           sd_rdev;
        volatile u_int  sd_iocnt;       /* # bdev/cdev_* operations active */
        bool            sd_opened;      /* true if successfully opened */
+       bool            sd_closing;     /* true when bdev/cdev_close ongoing */
 } specdev_t;
 
 /*