From owner-freebsd-arch Sat Aug 11 13:36:34 2001 Delivered-To: freebsd-arch@freebsd.org Received: from ringworld.nanolink.com (discworld.nanolink.com [217.75.135.248]) by hub.freebsd.org (Postfix) with SMTP id 8E1E637B403 for ; Sat, 11 Aug 2001 13:36:18 -0700 (PDT) (envelope-from roam@ringlet.net) Received: (qmail 691 invoked by uid 1000); 11 Aug 2001 20:34:52 -0000 Date: Sat, 11 Aug 2001 23:34:52 +0300 From: Peter Pentchev To: arch@FreeBSD.org Cc: audit@FreeBSD.org Subject: sysctl_register_oid() breakage at unload [PATCH] Message-ID: <20010811233452.A510@ringworld.oblivion.bg> Mail-Followup-To: arch@FreeBSD.org, audit@FreeBSD.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5i Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG 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-arch" in the body of the message