Date: Sat, 11 Aug 2001 23:34:52 +0300 From: Peter Pentchev <roam@ringlet.net> To: arch@FreeBSD.org Cc: audit@FreeBSD.org Subject: sysctl_register_oid() breakage at unload [PATCH] Message-ID: <20010811233452.A510@ringworld.oblivion.bg>
next in thread | raw e-mail | index | archive | help
Hi,
Well, it seems that I broke things with the panic at attempts to
register oid's higher than the first dynamic oid. Specifically,
this broke the case of unregistering sysctl's, esp. at module unload.
The algorithm described in the sysctl_ctx_free(9) manpage is indeed
so very weird (I won't go so far as calling it 'stupid', because
I cannot really suggest any way to improve it right now).
So, if a sysctl context is freed, most of the time the first pass
of freeing will fail, and sysctl_ctx_free() will attempt to reregister
the sysctls with the same oid's; this, of course, causes a panic,
because sysctl_register_oid() does not like so high a "static" oid :(
I just noticed that on my -stable laptop, when I tried to MFC
the patch - my sound driver is only available as a module, and
the kernel panicked at shutdown after attempting to unload it.
For various reasons I cannot run -current on this laptop (not least
because this is the machine I'm using for developing an application
that is supposed to run under -stable), and my -current box did not
really have any need for loadable modules, so that's how this slipped
in unnoticed :(
So here's a proposed fix: add a "this is actually a re-registering,
stay cool" flag to sysctl_register_oid(), and update all the calls
to it that I could find under src/sys. This flag needs only be set
in sysctl_ctx_free(), all the other callers put a 0.
The funniest part is that the original problem that I tried to fix
does not really exist under -stable - or at least, it is not all that
bad. All dynamic sysctl oid's there are obtained by a search through
the parent's subtree for an oid higher than 99, and registering the new
one at one bigger. The 99 is still there, so the problem described
in PR 29131 still *might* happen, it is just very, very much harder
to reproduce. So I do not think that there should be any concern with
MFC'ing this before 4.4-RELEASE.
G'luck,
Peter
--
Nostalgia ain't what it used to be.
Index: src/sys/kern/kern_linker.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/kern_linker.c,v
retrieving revision 1.64
diff -u -r1.64 kern_linker.c
--- src/sys/kern/kern_linker.c 2001/07/31 03:51:07 1.64
+++ src/sys/kern/kern_linker.c 2001/08/11 19:29:59
@@ -209,7 +209,7 @@
return;
for (oidp = start; oidp < stop; oidp++)
- sysctl_register_oid(*oidp);
+ sysctl_register_oid(*oidp, 0);
}
static void
Index: src/sys/kern/kern_sysctl.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/kern_sysctl.c,v
retrieving revision 1.112
diff -u -r1.112 kern_sysctl.c
--- src/sys/kern/kern_sysctl.c 2001/07/25 17:21:15 1.112
+++ src/sys/kern/kern_sysctl.c 2001/08/11 19:29:59
@@ -88,7 +88,7 @@
*/
void
-sysctl_register_oid(struct sysctl_oid *oidp)
+sysctl_register_oid(struct sysctl_oid *oidp, int rereg)
{
struct sysctl_oid_list *parent = oidp->oid_parent;
struct sysctl_oid *p;
@@ -121,7 +121,7 @@
oidp->oid_number = newoid++;
if (newoid == 0x7fffffff)
panic("out of oids");
- } else if (oidp->oid_number >= CTL_AUTO_START) {
+ } else if ((oidp->oid_number >= CTL_AUTO_START) && !rereg) {
panic("static sysctl oid too high: %d", oidp->oid_number);
}
@@ -187,7 +187,7 @@
else
e1 = TAILQ_LAST(clist, sysctl_ctx_list);
while (e1 != NULL) {
- sysctl_register_oid(e1->entry);
+ sysctl_register_oid(e1->entry, 1);
e1 = TAILQ_PREV(e1, sysctl_ctx_list, link);
}
if (error)
@@ -368,7 +368,7 @@
if (clist != NULL)
sysctl_ctx_entry_add(clist, oidp);
/* Register this oid */
- sysctl_register_oid(oidp);
+ sysctl_register_oid(oidp, 0);
return (oidp);
}
@@ -383,7 +383,7 @@
struct sysctl_oid **oidp;
SET_FOREACH(oidp, sysctl_set)
- sysctl_register_oid(*oidp);
+ sysctl_register_oid(*oidp, 0);
}
SYSINIT(sysctl, SI_SUB_KMEM, SI_ORDER_ANY, sysctl_register_all, 0);
Index: src/sys/kern/vfs_init.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/vfs_init.c,v
retrieving revision 1.55
diff -u -r1.55 vfs_init.c
--- src/sys/kern/vfs_init.c 2001/04/29 02:44:49 1.55
+++ src/sys/kern/vfs_init.c 2001/08/11 19:29:59
@@ -335,7 +335,7 @@
if (strcmp(oidp->oid_name, vfc->vfc_name) == 0) {
sysctl_unregister_oid(oidp);
oidp->oid_number = vfc->vfc_typenum;
- sysctl_register_oid(oidp);
+ sysctl_register_oid(oidp, 0);
}
/*
Index: src/sys/sys/sysctl.h
===================================================================
RCS file: /home/ncvs/src/sys/sys/sysctl.h,v
retrieving revision 1.98
diff -u -r1.98 sysctl.h
--- src/sys/sys/sysctl.h 2001/07/25 17:21:18 1.98
+++ src/sys/sys/sysctl.h 2001/08/11 19:29:59
@@ -153,7 +153,7 @@
/*
* These functions are used to add/remove an oid from the mib.
*/
-void sysctl_register_oid(struct sysctl_oid *oidp);
+void sysctl_register_oid(struct sysctl_oid *oidp, int);
void sysctl_unregister_oid(struct sysctl_oid *oidp);
/* Declare a static oid to allow child oids to be added to it. */
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-audit" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20010811233452.A510>