[src/trunk]: src/sys crashme(9): Use sysctl mib numbers, not node pointers.

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

details:   https://anonhg.NetBSD.org/src/rev/51395d898243
branches:  trunk
changeset: 370181:51395d898243
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Wed Sep 21 10:50:11 2022 +0000

description:
crashme(9): Use sysctl mib numbers, not node pointers.

The node pointers are not stable across insertions of siblings,
because they are pointers into arrays that may be reallocated and
moved elsewhere.

XXX Need to audit the tree for other bugs of this class, or change
sysctl(9) so it returns stable node pointers.

diffstat:

 sys/kern/kern_crashme.c |  52 +++++++++++++++++++++++++++---------------------
 sys/sys/crashme.h       |   4 +-
 2 files changed, 31 insertions(+), 25 deletions(-)

diffs (145 lines):

diff -r 31984cae00b6 -r 51395d898243 sys/kern/kern_crashme.c
--- a/sys/kern/kern_crashme.c   Wed Sep 21 10:36:14 2022 +0000
+++ b/sys/kern/kern_crashme.c   Wed Sep 21 10:50:11 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: kern_crashme.c,v 1.7 2022/08/30 22:38:26 riastradh Exp $       */
+/*     $NetBSD: kern_crashme.c,v 1.8 2022/09/21 10:50:11 riastradh Exp $       */
 
 /*
  * Copyright (c) 2018, 2019 Matthew R. Green
@@ -91,7 +91,7 @@
 };
 static crashme_node *first_node;
 static kmutex_t crashme_lock;
-static const struct sysctlnode *crashme_root = NULL;
+static int crashme_root = -1;
 static bool crashme_enable = 0;
 
 /*
@@ -103,8 +103,9 @@
        int rv = -1;
        crashme_node *cn;
        crashme_node *last = NULL;
+       const struct sysctlnode *cnode;
 
-       if (crashme_root == NULL)
+       if (crashme_root == -1)
                return -1;
 
        mutex_enter(&crashme_lock);
@@ -115,17 +116,16 @@
        if (!cn) {
                ncn->cn_next = NULL;
 
-               rv = sysctl_createv(NULL, 0,
-                                   &crashme_root, &ncn->cn_sysctl,
-                                   CTLFLAG_PERMANENT | CTLFLAG_READWRITE,
-                                   CTLTYPE_INT, ncn->cn_name,
-                                   SYSCTL_DESCR(ncn->cn_longname),
-                                   crashme_sysctl_forwarder, 0,
-                                   NULL, 0,
-                                   CTL_CREATE, CTL_EOL);
+               rv = sysctl_createv(NULL, 0, NULL, &cnode,
+                   CTLFLAG_PERMANENT | CTLFLAG_READWRITE,
+                   CTLTYPE_INT, ncn->cn_name,
+                   SYSCTL_DESCR(ncn->cn_longname),
+                   crashme_sysctl_forwarder, 0, NULL, 0,
+                   CTL_DEBUG, crashme_root, CTL_CREATE, CTL_EOL);
 
                /* don't insert upon failure */
                if (rv == 0) {
+                       ncn->cn_sysctl = cnode->sysctl_num;
                        if (last)
                                last->cn_next = ncn;
                        if (first_node == NULL)
@@ -158,9 +158,9 @@
                        prev->cn_next = cn->cn_next;
 
                if ((rv = sysctl_destroyv(NULL, CTL_DEBUG, crashme_root,
-                                         cn->cn_name, CTL_EOL)) == 0)
+                           cn->cn_sysctl, CTL_EOL)) == 0)
                        printf("%s: unable to remove %s from sysctl\n",
-                              __func__, cn->cn_name);
+                           __func__, cn->cn_name);
                break;
        }
        mutex_exit(&crashme_lock);
@@ -182,7 +182,7 @@
        int error, arg = 0;
 
        for (cn = first_node; cn; cn = cn->cn_next) {
-               if (cn->cn_sysctl == rnode)
+               if (cn->cn_sysctl == rnode->sysctl_num)
                        break;
        }
        if (!cn) {
@@ -209,20 +209,27 @@
  */
 SYSCTL_SETUP(selfdebug_crashme, "sysctl crashme setup")
 {
+       const struct sysctlnode *rnode;
        int rv;
        size_t n;
 
        mutex_init(&crashme_lock, MUTEX_DEFAULT, IPL_NONE);
 
-       rv = sysctl_createv(NULL, 0, NULL, &crashme_root,
+       KASSERT(crashme_root == -1);
+
+       rv = sysctl_createv(NULL, 0, NULL, &rnode,
            CTLFLAG_PERMANENT,
            CTLTYPE_NODE, "crashme",
            SYSCTL_DESCR("Crashme options"),
            NULL, 0, NULL, 0,
            CTL_DEBUG, CTL_CREATE, CTL_EOL);
 
-       if (rv != 0 || crashme_root == NULL)
+       if (rv != 0) {
+               printf("%s: failed to create sysctl debug.crashme: %d\n",
+                   __func__, rv);
                return;
+       }
+       crashme_root = rnode->sysctl_num;
 
        rv = sysctl_createv(NULL, 0, NULL, NULL,
            CTLFLAG_PERMANENT|CTLFLAG_READWRITE,
@@ -230,15 +237,14 @@
            SYSCTL_DESCR("Enable crashme"),
            NULL, 0, &crashme_enable, 0,
            CTL_DEBUG, CTL_CREATE, CTL_EOL);
+       if (rv != 0)
+               printf("%s: failed to create sysctl debug.crashme_enable:"
+                   " %d\n", __func__, rv);
 
        for (n = 0; n < __arraycount(nodes); n++) {
-               crashme_node *cn = &nodes[n];
-
-               rv = crashme_add(cn);
-
-               /* don't insert */
-               if (rv != 0)
-                       continue;
+               if (crashme_add(&nodes[n]))
+                       printf("%s: failed to create sysctl"
+                           " debug.crashme.%s\n", __func__, nodes[n].cn_name);
        }
 }
 
diff -r 31984cae00b6 -r 51395d898243 sys/sys/crashme.h
--- a/sys/sys/crashme.h Wed Sep 21 10:36:14 2022 +0000
+++ b/sys/sys/crashme.h Wed Sep 21 10:50:11 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: crashme.h,v 1.1 2019/01/09 04:01:20 mrg Exp $  */
+/*     $NetBSD: crashme.h,v 1.2 2022/09/21 10:50:11 riastradh Exp $    */
 
 /*
  * Copyright (c) 2018, 2019 Matthew R. Green
@@ -47,7 +47,7 @@
        const char              *cn_name;
        const char              *cn_longname;
        crashme_fn               cn_fn;
-       const struct sysctlnode *cn_sysctl;
+       int                      cn_sysctl;
        struct crashme_node     *cn_next;
 } crashme_node;