[src/trunk]: src/sys/kern autoconf(9): Take kernel lock in a few entry points.

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

details:   https://anonhg.NetBSD.org/src/rev/70a298469635
branches:  trunk
changeset: 983873:70a298469635
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Sun Jun 13 00:11:17 2021 +0000

description:
autoconf(9): Take kernel lock in a few entry points.

The arguments to config_attach_pseudo, config_init/fini_component,
and config_cfdata_attach/detach are generally statically allocated
objects in a module or the main kernel and as such are stable, and
there are no data structure invariants they assume the kernel lock
will covers from call to call.  So there should be no need for the
caller to hold the kernel lock.

Should fix panic on modload of, e.g., nvmm.

diffstat:

 sys/kern/subr_autoconf.c |  54 ++++++++++++++++++++++++++++++++----------------
 1 files changed, 36 insertions(+), 18 deletions(-)

diffs (169 lines):

diff -r ec10822be0af -r 70a298469635 sys/kern/subr_autoconf.c
--- a/sys/kern/subr_autoconf.c  Sat Jun 12 15:49:45 2021 +0000
+++ b/sys/kern/subr_autoconf.c  Sun Jun 13 00:11:17 2021 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: subr_autoconf.c,v 1.284 2021/06/12 12:14:13 riastradh Exp $ */
+/* $NetBSD: subr_autoconf.c,v 1.285 2021/06/13 00:11:17 riastradh Exp $ */
 
 /*
  * Copyright (c) 1996, 2000 Christopher G. Demetriou
@@ -79,7 +79,7 @@
 #define        __SUBR_AUTOCONF_PRIVATE /* see <sys/device.h> */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: subr_autoconf.c,v 1.284 2021/06/12 12:14:13 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: subr_autoconf.c,v 1.285 2021/06/13 00:11:17 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_ddb.h"
@@ -377,27 +377,31 @@
 {
        int error;
 
-       KASSERT(KERNEL_LOCKED_P());
+       KERNEL_LOCK(1, NULL);
 
        if ((error = frob_cfdrivervec(cfdriverv,
            config_cfdriver_attach, config_cfdriver_detach, "init", false))!= 0)
-               return error;
+               goto out;
        if ((error = frob_cfattachvec(cfattachv,
            config_cfattach_attach, config_cfattach_detach,
            "init", false)) != 0) {
                frob_cfdrivervec(cfdriverv,
                    config_cfdriver_detach, NULL, "init rollback", true);
-               return error;
+               goto out;
        }
        if ((error = config_cfdata_attach(cfdatav, 1)) != 0) {
                frob_cfattachvec(cfattachv,
                    config_cfattach_detach, NULL, "init rollback", true);
                frob_cfdrivervec(cfdriverv,
                    config_cfdriver_detach, NULL, "init rollback", true);
-               return error;
+               goto out;
        }
 
-       return 0;
+       /* Success!  */
+       error = 0;
+
+out:   KERNEL_UNLOCK_ONE(NULL);
+       return error;
 }
 
 int
@@ -406,16 +410,16 @@
 {
        int error;
 
-       KASSERT(KERNEL_LOCKED_P());
+       KERNEL_LOCK(1, NULL);
 
        if ((error = config_cfdata_detach(cfdatav)) != 0)
-               return error;
+               goto out;
        if ((error = frob_cfattachvec(cfattachv,
            config_cfattach_detach, config_cfattach_attach,
            "fini", false)) != 0) {
                if (config_cfdata_attach(cfdatav, 0) != 0)
                        panic("config_cfdata fini rollback failed");
-               return error;
+               goto out;
        }
        if ((error = frob_cfdrivervec(cfdriverv,
            config_cfdriver_detach, config_cfdriver_attach,
@@ -424,10 +428,14 @@
                    config_cfattach_attach, NULL, "fini rollback", true);
                if (config_cfdata_attach(cfdatav, 0) != 0)
                        panic("config_cfdata fini rollback failed");
-               return error;
+               goto out;
        }
 
-       return 0;
+       /* Success!  */
+       error = 0;
+
+out:   KERNEL_UNLOCK_ONE(NULL);
+       return error;
 }
 
 void
@@ -946,7 +954,7 @@
 {
        struct cftable *ct;
 
-       KASSERT(KERNEL_LOCKED_P());
+       KERNEL_LOCK(1, NULL);
 
        ct = kmem_alloc(sizeof(*ct), KM_SLEEP);
        ct->ct_cfdata = cf;
@@ -955,6 +963,8 @@
        if (scannow)
                rescan_with_cfdata(cf);
 
+       KERNEL_UNLOCK_ONE();
+
        return 0;
 }
 
@@ -986,6 +996,8 @@
        struct cftable *ct;
        deviter_t di;
 
+       KERNEL_LOCK(1, NULL);
+
        for (d = deviter_first(&di, DEVITER_F_RW); d != NULL;
             d = deviter_next(&di)) {
                if (!dev_in_cfdata(d, cf))
@@ -996,19 +1008,23 @@
        deviter_release(&di);
        if (error) {
                aprint_error_dev(d, "unable to detach instance\n");
-               return error;
+               goto out;
        }
 
        TAILQ_FOREACH(ct, &allcftables, ct_list) {
                if (ct->ct_cfdata == cf) {
                        TAILQ_REMOVE(&allcftables, ct, ct_list);
                        kmem_free(ct, sizeof(*ct));
-                       return 0;
+                       error = 0;
+                       goto out;
                }
        }
 
        /* not found -- shouldn't happen */
-       return EINVAL;
+       error = EINVAL;
+
+out:   KERNEL_UNLOCK_ONE(NULL);
+       return error;
 }
 
 /*
@@ -1824,11 +1840,11 @@
 {
        device_t dev;
 
-       KASSERT(KERNEL_LOCKED_P());
+       KERNEL_LOCK(1, NULL);
 
        dev = config_devalloc(ROOT, cf, CFARG_EOL);
        if (!dev)
-               return NULL;
+               goto out;
 
        /* XXX mark busy in cfdata */
 
@@ -1851,6 +1867,8 @@
        config_pending_decr(dev);
 
        config_process_deferred(&deferred_config_queue, dev);
+
+out:   KERNEL_UNLOCK_ONE(NULL);
        return dev;
 }