From owner-freebsd-audit@FreeBSD.ORG Mon Nov 1 21:52:00 2004 Return-Path: Delivered-To: freebsd-audit@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id D934A16A4CE for ; Mon, 1 Nov 2004 21:52:00 +0000 (GMT) Received: from moutng.kundenserver.de (moutng.kundenserver.de [212.227.126.177]) by mx1.FreeBSD.org (Postfix) with ESMTP id 6864F43D31 for ; Mon, 1 Nov 2004 21:52:00 +0000 (GMT) (envelope-from se@freebsd.org) Received: from [212.227.126.208] (helo=mrelayng.kundenserver.de) by moutng.kundenserver.de with esmtp (Exim 3.35 #1) id 1COk5n-0001Ku-00 for audit@freebsd.org; Mon, 01 Nov 2004 22:51:59 +0100 Received: from [80.132.237.31] (helo=Gatekeeper.FreeBSD.org) by mrelayng.kundenserver.de with asmtp (Exim 3.35 #1) id 1COk5n-0004Na-00; Mon, 01 Nov 2004 22:51:59 +0100 Received: from StefanEsser.FreeBSD.org (StefanEsser [192.168.0.10]) by Gatekeeper.FreeBSD.org (Postfix) with ESMTP id 60FAF5F4B; Mon, 1 Nov 2004 22:51:57 +0100 (CET) Received: by StefanEsser.FreeBSD.org (Postfix, from userid 200) id 502692305; Mon, 1 Nov 2004 22:51:46 +0100 (CET) Date: Mon, 1 Nov 2004 22:51:46 +0100 From: Stefan =?iso-8859-1?Q?E=DFer?= To: audit@freebsd.org Message-ID: <20041101215146.GA42768@StefanEsser.FreeBSD.org> Mail-Followup-To: Stefan =?iso-8859-1?Q?E=DFer?= , audit@freebsd.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.6i X-Provags-ID: kundenserver.de abuse@kundenserver.de auth:fa3fae9b6ca38d745862a668565919f6 Subject: [Patch] ISDN+NETGRAPH+INET6 => Panic X-BeenThere: freebsd-audit@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: FreeBSD Security Audit List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 01 Nov 2004 21:52:01 -0000 There is a problem with the initialization order of network devices and protocols/address families, leading to a later panic because of an incomplete initialization of a data structure, if INET6 configured. To reproduce, build a kernel with ISDN, Netgraph and INET6. The IPv6 neighbour discovery function will panic after exactly one hour, because of a NULL pointer dereference. The cause is the initialization of both the ISDN (pseudo) devices and Netgraph in unspecified order due to: SYSINIT(... ,SI_SUB_PSEUDO, SI_ORDER_ANY); A side effect of the initialization of Netgraph is that the "netgraph" protocol domain is defined early. It should not happen before phase SI_SUB_PROTO_DOMAIN: SI_SUB_INIT_IF = 0x3000000, /* prep for net interfaces */ [ ... ] SI_SUB_PSEUDO = 0x7000000, /* pseudo devices*/ SI_SUB_EXEC = 0x7400000, /* execve() handlers */ SI_SUB_PROTO_BEGIN = 0x8000000, /* XXX: set splimp (kludge)*/ SI_SUB_PROTO_IF = 0x8400000, /* interfaces*/ SI_SUB_PROTO_DOMAIN = 0x8800000, /* domains (address families?)*/ SI_SUB_PROTO_IFATTACHDOMAIN = 0x8800001, /* domain dependent data init*/ SI_SUB_PROTO_END = 0x8ffffff, /* XXX: set splx (kludge)*/ As soon as the netgraph domain is defined, if_attachdomain1() will be called during if_attach(), resulting in if_afdata_initialized=1, which will prevent the correct and required initialisation during sysinit phase SI_SUB_PROTO_IFATTACHDOMAIN. The least intrusive patch is attached below. It forces an order into the initialisation of i4b and netgraph by use of SI_ORDER_MIDDLE for the i4b init. This is not a complete fix, since it is netgraph that is violating the rules. It should not call net_add_domain() before phase SI_PROTO_DOMAIN. Currently only SI_ORDER_ANY is used with SI_SUB_PSEUDO, which means that no order is enforced, the order is effectively unspecified. Changing the order parameter to SI_ORDER_MIDDLE leads to an order that might have resulted from the use of SI_ORDER_ANY by accident. I'd rather leave the moving of the net_add_domain(netgraph) into phase SI_SUB_PROTO_DOMAIN (assuming that it really belongs there) to the Netgraph maintainers. Any objections to adding the following work-around to RELENG_5 via -CURRENT ? I'd really want to see it in 5.3, since it does fix a real problem well enough to make ISDN work in kernels with NETGRAPH and INET6. That configuration will carsh after one hour of uptime, else: Index: i4b/include/i4b_global.h --- i4b/include/i4b_global.h 15 Jul 2004 08:26:05 -0000 1.12 +++ i4b/include/i4b_global.h 1 Nov 2004 18:48:07 -0000 @@ -67,7 +67,8 @@ name ## _modevent, \ (void *)sym \ }; \ - DECLARE_MODULE(name, name ## _mod, SI_SUB_PSEUDO, SI_ORDER_ANY) + /* XXX work-around: i4b must be initialized before netgraph! */ \ + DECLARE_MODULE(name, name ## _mod, SI_SUB_PSEUDO, SI_ORDER_MIDDLE) #endif /*---------------*/ >From a mail threat that discussed a similar panic caused by the neighbour discovery function I got the following patch that was attributed to Robert Watson's netperf branch: Index: netinet6/nd6.c --- netinet6/nd6.c 11 Oct 2004 03:46:38 -0000 1.43.2.3 +++ netinet6/nd6.c 30 Oct 2004 21:08:17 -0000 @@ -1798,6 +1798,8 @@ nd6_slowtimo, NULL); IFNET_RLOCK(); for (ifp = TAILQ_FIRST(&ifnet); ifp; ifp = TAILQ_NEXT(ifp, if_list)) { + if (ifp->if_afdata[AF_INET6] == NULL) + continue; nd6if = ND_IFINFO(ifp); if (nd6if->basereachable && /* already initialized */ (nd6if->recalctm -= ND6_SLOWTIMER_INTERVAL) <= 0) { That patch did not prevent the panic for me, since there is another similar loop in in6_tmpaddrtimer(): Index: netinet6/in6_ifattach.c --- netinet6/in6_ifattach.c 27 Aug 2004 04:46:27 -0000 1.23.2.1 +++ netinet6/in6_ifattach.c 1 Nov 2004 00:06:30 -0000 @@ -895,6 +895,8 @@ bzero(nullbuf, sizeof(nullbuf)); for (ifp = TAILQ_FIRST(&ifnet); ifp; ifp = TAILQ_NEXT(ifp, if_list)) { + if (ifp->if_afdata[AF_INET6] == NULL) + continue; ndi = ND_IFINFO(ifp); if (bcmp(ndi->randomid, nullbuf, sizeof(nullbuf)) != 0) { /* These two NULL pointer checks are not necessary in the ISDN+NETGRAPH+ INET6 scenario, but may well be if there are other calls of if_attach() during SI_SUB_PSEUDO/SI_ORDER_ANY. In fact, a KASSERT() may be more appropriate, since the premature call of net_add_domain() will cause incomplete initialisation of network device structures until after phase SI_SUB_PROTO_DOMAIN. Or just a plain panic("%s: incomplete initialisation of %s!", __func__, ifp->if_xname); instead of the "continue" in both patches above ... I think all three places should be fixed. The first one does make ISDN work again in a kernel with NETGRAPH and INET6 by fixing the initialisation of i4b devices. The latter two either mask or report the incomplete initialisation of pseudo network devices. Any comments on the patches ? Any objections to me commiting them to -CURRENT ? I'd want to see the i4b SYSINIT patch in 5.3, since it is a fix and does just enforce an order in an as of now random sequence, thus can't have a negative impact. Regards, STefan