Date: Wed, 26 Dec 2001 08:20:01 -0800 (PST) From: Robert Watson <rwatson@freebsd.org> To: freebsd-bugs@FreeBSD.org Subject: Re: kern/33201: net/net_osdep.c:if_name is broken Message-ID: <200112261620.fBQGK1W53302@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/33201; it has been noted by GNATS.
From: Robert Watson <rwatson@freebsd.org>
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200112261620.fBQGK1W53302>