Re: A blast from the past - strange sysctl behavior

[Paul Goyette <paul%whooppee.com@localhost>]

In September of last year I wrote:

> Nearly a year ago, I posted [1] on the current-users list. Since that
> time I've been running with the attached patch, in an attempt to find
> out what was going on. And as is usually the case, the debug code
> didn't get triggered, until now - 11 months later!
>
> I haven't had a chance to investigate in any detail, but the problem
> (and associated KASSERT) occurred while starting firefox. A bit more
> detail shows that it was installing the sysctl tree for the sysv_ipc
> module. However, after the reboot I was able to successfully launch
> firefox without triggering the KASSERT.
>
> If anyone has any clues about how this anomaly can happen (a newly
> created sysctl node has a wrong parent), I'd love to hear them. I am
> guessing that there's some sort of locking botch in the sysctl code,
> however I haven't been able to spot it.
>
> [1] https://mail-index.netbsd.org/current-users/2015/10/27/msg028285.html


Once again, my locally-inserted KASSERT has triggered while starting 
firefox.  Unfortunately I did not get a crash dump, and after a restart 
I cannot reproduce the problem.

Since this has happened several times (with multi-month-long gaps 
between occurrences), I'd like to commit the attached patch in the hopes 
that we can track down what's happening.  It seems as though there might 
be a locking issue within the sysctl_createv() code, but I can't see it. 
With this patch, perhaps someone else will have more luck reproducing 
the problem and tracking it down.

Note that, despite the sysctl tree being incorrectly created/linked, the 
only impact of this bug that I've actually seen is the inability to 
sysctl_teardown() something that was mis-parented.  Other sysctl usage 
seems to be unaffected.

Any objections to having the patch committed?


+------------------+--------------------------+----------------------------+
| Paul Goyette     | PGP Key fingerprint:     | E-mail addresses:          |
| (Retired)        | FA29 0E3B 35AF E8AE 6651 | paul at whooppee dot com   |
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd dot org |
+------------------+--------------------------+----------------------------+

Index: kern_sysctl.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_sysctl.c,v
retrieving revision 1.258
diff -u -p -r1.258 kern_sysctl.c
--- kern_sysctl.c	23 Oct 2015 01:58:43 -0000	1.258
+++ kern_sysctl.c	5 Sep 2016 09:26:26 -0000
@@ -1955,6 +1955,7 @@ sysctl_createv(struct sysctllog **log, i
 	const struct sysctlnode *root, *pnode;
 	struct sysctlnode nnode, onode, *dnode;
 	size_t sz;
+	const struct sysctlnode *snode __diagused;
 
 	/*
 	 * where are we putting this?
@@ -2111,6 +2112,7 @@ sysctl_createv(struct sysctllog **log, i
 		 */
 		pnode = root;
 		error = sysctl_locate(NULL, &name[0], namelen - 1, &pnode, &ni);
+		snode = pnode;
 
 		/*
 		 * manual scan of last layer so that aliased nodes
@@ -2131,6 +2133,8 @@ sysctl_createv(struct sysctllog **log, i
 		 * not expecting an error here, but...
 		 */
 		if (error == 0) {
+			KASSERTMSG(pnode->sysctl_parent == snode,
+			    "sysctl parent mis-match");
 			if (log != NULL)
 				sysctl_log_add(log, pnode);
 			if (cnode != NULL)