From owner-freebsd-net@FreeBSD.ORG Mon Oct 31 18:20:11 2011 Return-Path: Delivered-To: freebsd-net@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 6B967106564A for ; Mon, 31 Oct 2011 18:20:11 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 50EDE8FC16 for ; Mon, 31 Oct 2011 18:20:11 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id p9VIKB6Q065160 for ; Mon, 31 Oct 2011 18:20:11 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.4/8.14.4/Submit) id p9VIKBfH065159; Mon, 31 Oct 2011 18:20:11 GMT (envelope-from gnats) Date: Mon, 31 Oct 2011 18:20:11 GMT Message-Id: <201110311820.p9VIKBfH065159@freefall.freebsd.org> To: freebsd-net@FreeBSD.org From: Gleb Smirnoff Cc: Subject: Re: misc/162201: [patch] multicast forwarding cache hash always allocated with size 0, resulting in buffer overrun X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Gleb Smirnoff List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 31 Oct 2011 18:20:11 -0000 The following reply was made to PR kern/162201; it has been noted by GNATS. From: Gleb Smirnoff To: Stevan Markovic Cc: freebsd-gnats-submit@FreeBSD.org, zec@FreeBSD.org, bms@FreeBSD.org, bz@FreeBSD.org Subject: Re: misc/162201: [patch] multicast forwarding cache hash always allocated with size 0, resulting in buffer overrun Date: Mon, 31 Oct 2011 21:10:22 +0300 Hi, On Mon, Oct 31, 2011 at 04:22:00PM +0000, Stevan Markovic wrote: S> >Description: S> Bug is observed as kernel panic shortly after stopping XORP (www.xorp.org) configured for PIM/SM routing. Debugging discovered that at the time of MALLOC for V_nexpire in ip_mroute.c::vnet_mroute_init() size variable mfchashsize always has value 0. S> S> Why: variable mfchashsize is initialized in module event handler which is executed with SI_ORDER_ANY ordering tag which happens _after_ variable usage in MALLOC in VNET_SYSINIT with SI_ORDER_MIDDLE. S> S> Fix simply moves variable initialization before its usage in vnet_mroute_init. S> S> This bug is discovered and fixed in McAfee Inc development. S> >How-To-Repeat: S> Hard to reproduce since system behavior after memory overwrite is unpredictable. Multicast forwarding cashe hash overrun always happens after: S> a) configuring xorp to use PIM/SM S> b) starting xorp_rtrmgr S> c) stopping xorp_rtrmgr. S> S> >Fix: S> Fix simply moves mfchashsize variable initialization before its usage in vnet_mroute_init. S> S> Patch attached with submission follows: S> S> Index: ip_mroute.c S> =================================================================== S> RCS file: /projects/freebsd/src_cvsup/src/sys/netinet/ip_mroute.c,v S> retrieving revision 1.161 S> diff -u -r1.161 ip_mroute.c S> --- ip_mroute.c 22 Nov 2010 19:32:54 -0000 1.161 S> +++ ip_mroute.c 31 Oct 2011 15:54:53 -0000 S> @@ -2814,7 +2814,13 @@ S> static void S> vnet_mroute_init(const void *unused __unused) S> { S> - S> + mfchashsize = MFCHASHSIZE; S> + if (TUNABLE_ULONG_FETCH("net.inet.ip.mfchashsize", &mfchashsize) && S> + !powerof2(mfchashsize)) { S> + printf("WARNING: %s not a power of 2; using default\n", S> + "net.inet.ip.mfchashsize"); S> + mfchashsize = MFCHASHSIZE; S> + } S> MALLOC(V_nexpire, u_char *, mfchashsize, M_MRTABLE, M_WAITOK|M_ZERO); S> bzero(V_bw_meter_timers, sizeof(V_bw_meter_timers)); S> callout_init(&V_expire_upcalls_ch, CALLOUT_MPSAFE); S> @@ -2855,13 +2861,6 @@ S> MFC_LOCK_INIT(); S> VIF_LOCK_INIT(); S> S> - mfchashsize = MFCHASHSIZE; S> - if (TUNABLE_ULONG_FETCH("net.inet.ip.mfchashsize", &mfchashsize) && S> - !powerof2(mfchashsize)) { S> - printf("WARNING: %s not a power of 2; using default\n", S> - "net.inet.ip.mfchashsize"); S> - mfchashsize = MFCHASHSIZE; S> - } S> S> pim_squelch_wholepkt = 0; S> TUNABLE_ULONG_FETCH("net.inet.pim.squelch_wholepkt", Have you tried to remove these VNET_SYSINITs at all and do all the initialization in the ip_mroute_modevent() itself? From first glance I see no reason for separate malloc() + callout_init()s. I am putting guys, who made and reviewed the commit, into Cc. -- Totus tuus, Glebius.