From owner-freebsd-bugs@FreeBSD.ORG Wed Apr 25 18:50:08 2007 Return-Path: X-Original-To: freebsd-bugs@hub.freebsd.org Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id E047716A400 for ; Wed, 25 Apr 2007 18:50:08 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [69.147.83.40]) by mx1.freebsd.org (Postfix) with ESMTP id 993A813C44C for ; Wed, 25 Apr 2007 18:50:08 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.13.4/8.13.4) with ESMTP id l3PIo8Op048830 for ; Wed, 25 Apr 2007 18:50:08 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.13.4/8.13.4/Submit) id l3PIo8ff048824; Wed, 25 Apr 2007 18:50:08 GMT (envelope-from gnats) Date: Wed, 25 Apr 2007 18:50:08 GMT Message-Id: <200704251850.l3PIo8ff048824@freefall.freebsd.org> To: freebsd-bugs@FreeBSD.org From: Maxim Konovalov Cc: Subject: Re: misc/112126: netstat segfaults on unusual ICMP statistics X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Maxim Konovalov List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 25 Apr 2007 18:50:09 -0000 The following reply was made to PR bin/112126; it has been noted by GNATS. From: Maxim Konovalov To: Christoph Weber-Fahr Cc: bug-followup@freebsd.org Subject: Re: misc/112126: netstat segfaults on unusual ICMP statistics Date: Wed, 25 Apr 2007 22:45:35 +0400 (MSD) Hi Christoph, On Wed, 25 Apr 2007, 15:29-0000, Christoph Weber-Fahr wrote: > >Description: > we have a number of very busy publicly available production servers > running FreeBSD. > > On some of these systems, netstat -s fails with a segmentation > fault. This is especially problematic, since system monitoring > tools use netstat periodically to gather statistics. > > I looked into the issue in the netstats sources. The netstat utility > mainatins an internal array for ICMP type names. But for listing the > icmp statistics it relies on the ICMP_MAXTYPE kernel define pulled > from the syestem include files. > > Those values are badly out of sync - ICMP_MAXTYPE is at 40, while > netstat's name array has just 19 entries. So, as soon as the kernel > has logged a stat for an icmp type > 19, netstat -s dumps core. > [...] > the appended patch > > - enhances netstat's array to 40 with names from the include file > - provides a mechanism to deal gracefully with such a situation > schould it arise in the future again. > > The patch was created and tested on FreeBSD 6.1-p15. It was also > tested on FreeBSD 6.2-p3. > > Patch attached with submission follows: > > --- usr.bin/netstat/inet.c.org Wed Apr 25 15:44:51 2007 > +++ usr.bin/netstat/inet.c Wed Apr 25 17:19:17 2007 > @@ -653,8 +653,32 @@ > "information request reply", > "address mask request", > "address mask reply", > + "#19", > + "#20", > + "#21", > + "#22", > + "#23", > + "#24", > + "#25", > + "#26", > + "#27", > + "#28", > + "#29", > + "icmp traceroute", > + "data conversion error", > + "mobile host redirect", > + "IPv6 where-are-you", > + "IPv6 i-am-here", > + "mobile registration req", > + "mobile registration reply", > + "#37", > + "#38", > + "icmp SKIP", > + "icmp photuris", > }; > > +static const int max_known_icmpname=40; > + > > /* > * Dump ICMP statistics. > */ > @@ -698,8 +722,13 @@ > printf("\tOutput histogram:\n"); > first = 0; > } > - printf("\t\t%s: %lu\n", icmpnames[i], > - icmpstat.icps_outhist[i]); > + if (i <= max_known_icmpname) { > + printf("\t\t%s: %lu\n", icmpnames[i], > + icmpstat.icps_outhist[i]); > + } else { > + printf("\t\tunknown ICMP #%d: %lu\n", i, > + icmpstat.icps_outhist[i]); > + } > } > p(icps_badcode, "\t%lu message%s with bad code fields\n"); > p(icps_tooshort, "\t%lu message%s < minimum length\n"); > @@ -713,8 +742,13 @@ > printf("\tInput histogram:\n"); > first = 0; > } > - printf("\t\t%s: %lu\n", icmpnames[i], > - icmpstat.icps_inhist[i]); > + if (i <= max_known_icmpname) { > + printf("\t\t%s: %lu\n", icmpnames[i], > + icmpstat.icps_inhist[i]); > + } else { > + printf("\t\tunknown ICMP #%d: %lu\n", i, > + icmpstat.icps_inhist[i]); > + } > } > p(icps_reflect, "\t%lu message response%s generated\n"); > p2(icps_badaddr, "\t%lu invalid return address%s\n"); It's not clear why do you need to check i <= max_known_icmpname when the upper loop already checked that: for (first = 1, i = 0; i < ICMP_MAXTYPE + 1; i++). Can I suggest an alternative patch: Index: inet.c =================================================================== RCS file: /home/ncvs/src/usr.bin/netstat/inet.c,v retrieving revision 1.74 diff -u -p -r1.74 inet.c --- inet.c 26 Feb 2007 22:25:21 -0000 1.74 +++ inet.c 25 Apr 2007 18:32:13 -0000 @@ -636,7 +636,7 @@ ip_stats(u_long off __unused, const char #undef p1a } -static const char *icmpnames[] = { +static const char *icmpnames[ICMP_MAXTYPE + 1] = { "echo reply", "#1", "#2", @@ -656,6 +656,28 @@ static const char *icmpnames[] = { "information request reply", "address mask request", "address mask reply", + "#19", + "#20", + "#21", + "#22", + "#23", + "#24", + "#25", + "#26", + "#27", + "#28", + "#29", + "icmp traceroute", + "data conversion error", + "mobile host redirect", + "IPv6 where-are-you", + "IPv6 i-am-here", + "mobile registration req", + "mobile registration reply", + "domain name request", + "domain name reply", + "icmp SKIP", + "icmp photuris", }; /* %%% -- Maxim Konovalov