From owner-freebsd-bugs Wed Dec 26 8:20:16 2001 Delivered-To: freebsd-bugs@hub.freebsd.org Received: from freefall.freebsd.org (freefall.FreeBSD.org [216.136.204.21]) by hub.freebsd.org (Postfix) with ESMTP id A2F9D37B405 for ; Wed, 26 Dec 2001 08:20:01 -0800 (PST) Received: (from gnats@localhost) by freefall.freebsd.org (8.11.6/8.11.6) id fBQGK1W53302; Wed, 26 Dec 2001 08:20:01 -0800 (PST) (envelope-from gnats) Date: Wed, 26 Dec 2001 08:20:01 -0800 (PST) Message-Id: <200112261620.fBQGK1W53302@freefall.freebsd.org> To: freebsd-bugs@FreeBSD.org Cc: From: Robert Watson Subject: Re: kern/33201: net/net_osdep.c:if_name is broken Reply-To: Robert Watson Sender: owner-freebsd-bugs@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.org The following reply was made to PR kern/33201; it has been noted by GNATS. From: Robert Watson To: FreeBSD-gnats-submit@freebsd.org Cc: Subject: Re: kern/33201: net/net_osdep.c:if_name is broken Date: Wed, 26 Dec 2001 11:12:24 -0500 (EST) Slight update on this: I misread the function, it's evil due to its use of MAXNUMBUF rather than its use of stack space. Comments about IFNAMSIZE still apply. If we want to formally maintain an interface name string somewhere, I wouldn't mind, but it should probably be in struct ifnet. It might reduce the number of potential if_name/if_unit/string-based bugs substantially. Robert N M Watson FreeBSD Core Team, TrustedBSD Project robert@fledge.watson.org NAI Labs, Safeport Network Services On Wed, 26 Dec 2001, Robert Watson wrote: > > >Number: 33201 > >Category: kern > >Synopsis: net/net_osdep.c:if_name is broken > >Confidential: no > >Severity: serious > >Priority: low > >Responsible: freebsd-bugs > >State: open > >Quarter: > >Keywords: > >Date-Required: > >Class: sw-bug > >Submitter-Id: current-users > >Arrival-Date: Wed Dec 26 08:00:01 PST 2001 > >Closed-Date: > >Last-Modified: > >Originator: Robert Watson > >Release: FreeBSD 5.0-CURRENT i386 > >Organization: > NAI Labs > >Environment: > System: > > FreeBSD curry.decoverly.watson.org 5.0-CURRENT FreeBSD 5.0-CURRENT #0: Sun Dec 23 23:15:06 EST 2001 rwatson@curry.decoverly.watson.org:/usr/obj/usr/src/sys/BKTR i386 > > >Description: > > While it doesn't actually currently result in a failure that I can find, > if_name() is written in a very failure-prone style: > > const char * > if_name(ifp) > struct ifnet *ifp; > { > #define MAXNUMBUF 8 > static char nam[MAXNUMBUF][IFNAMSIZ + 10]; /*enough?*/ > static int ifbufround = 0; > char *cp; > > ifbufround = (ifbufround + 1) % MAXNUMBUF; > cp = nam[ifbufround]; > > snprintf(cp, IFNAMSIZ + 10, "%s%d", ifp->if_name, ifp->if_unit); > return((const char *)cp); > #undef MAXNUMBUF > } > > Returning pointers into the stack space of a function that is exiting > is *never* a good idea. > > Also, this code suffers from a continuing confusion about the possible > size of interface name strings that seems widespread through the sack. > IFNAMSIZ should either be sufficient and used everywhere, or be fixed. > As it stands, I found several hard-coded variants on the them, and > the ifunit() code to build a device name also looked fairly suspect. > > >How-To-Repeat: > >Fix: > >Release-Note: > >Audit-Trail: > >Unformatted: > > To Unsubscribe: send mail to majordomo@FreeBSD.org > with "unsubscribe freebsd-bugs" in the body of the message > To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-bugs" in the body of the message