[src/trunk]: src/sys/kern drvctl(4): Hold a deviter while issuing config_detach.

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

details:   https://anonhg.NetBSD.org/src/rev/dbdbab19a9d8
branches:  trunk
changeset: 983858:dbdbab19a9d8
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Sat Jun 12 12:14:03 2021 +0000

description:
drvctl(4): Hold a deviter while issuing config_detach.

Otherwise another concurrent detach -- e.g., from concurrent drvctl
or from USB port disconnection -- can pull the device_t out from
under us.

XXX Need to do this for _every_ operation config_* operation on a
device_t; device_find_by_xname is just fundamentally broken because a
concurrent detach can pull the device_t rug out from under you.

We really need another mechanism for holding a weak reference to a
device, and temporarily getting a strong reference to it; abusing
deviter is a bit of a kludge, and doesn't work very well because it
doesn't properly trigger garbage collection of devices detached while
other concurrent deviters are pending.

diffstat:

 sys/kern/kern_drvctl.c |  29 ++++++++++++++++++++++-------
 1 files changed, 22 insertions(+), 7 deletions(-)

diffs (61 lines):

diff -r ef1cff64ccef -r dbdbab19a9d8 sys/kern/kern_drvctl.c
--- a/sys/kern/kern_drvctl.c    Sat Jun 12 12:13:51 2021 +0000
+++ b/sys/kern/kern_drvctl.c    Sat Jun 12 12:14:03 2021 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: kern_drvctl.c,v 1.47 2021/06/12 12:12:11 riastradh Exp $ */
+/* $NetBSD: kern_drvctl.c,v 1.48 2021/06/12 12:14:03 riastradh Exp $ */
 
 /*
  * Copyright (c) 2004
@@ -27,7 +27,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: kern_drvctl.c,v 1.47 2021/06/12 12:12:11 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: kern_drvctl.c,v 1.48 2021/06/12 12:14:03 riastradh Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -254,11 +254,21 @@
 detachdevbyname(const char *devname)
 {
        device_t d;
+       deviter_t di;
+       int error;
 
        KASSERT(KERNEL_LOCKED_P());
 
-       if ((d = device_find_by_xname(devname)) == NULL)
-               return ENXIO;
+       for (d = deviter_first(&di, DEVITER_F_RW);
+            d != NULL;
+            d = deviter_next(&di)) {
+               if (strcmp(device_xname(d), devname) == 0)
+                       break;
+       }
+       if (d == NULL) {
+               error = ENXIO;
+               goto out;
+       }
 
 #ifndef XXXFULLRISK
        /*
@@ -267,10 +277,15 @@
         * There might be a private notification mechanism,
         * but better play it safe here.
         */
-       if (d->dv_parent && !d->dv_parent->dv_cfattach->ca_childdetached)
-               return ENOTSUP;
+       if (d->dv_parent && !d->dv_parent->dv_cfattach->ca_childdetached) {
+               error = ENOTSUP;
+               goto out;
+       }
 #endif
-       return config_detach(d, 0);
+
+       error = config_detach(d, 0);
+out:   deviter_release(&di);
+       return error;
 }
 
 static int