From owner-freebsd-net@FreeBSD.ORG Fri Sep 21 00:56:27 2007 Return-Path: Delivered-To: freebsd-net@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 7C8A816A417 for ; Fri, 21 Sep 2007 00:56:27 +0000 (UTC) (envelope-from thompsa@FreeBSD.org) Received: from heff.fud.org.nz (203-109-251-39.static.bliink.ihug.co.nz [203.109.251.39]) by mx1.freebsd.org (Postfix) with ESMTP id AAC3313C45D for ; Fri, 21 Sep 2007 00:56:26 +0000 (UTC) (envelope-from thompsa@FreeBSD.org) Received: by heff.fud.org.nz (Postfix, from userid 1001) id 6A9D71CC2F; Fri, 21 Sep 2007 12:56:23 +1200 (NZST) Date: Fri, 21 Sep 2007 12:56:23 +1200 From: Andrew Thompson To: Brooks Davis Message-ID: <20070921005623.GD46172@heff.fud.org.nz> References: <20070920235427.GA46172@heff.fud.org.nz> <20070921003927.GB77167@lor.one-eyed-alien.net> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="0eh6TmSyL6TZE2Uz" Content-Disposition: inline In-Reply-To: <20070921003927.GB77167@lor.one-eyed-alien.net> User-Agent: Mutt/1.5.13 (2006-08-11) Cc: FreeBSD-net Subject: Re: ifconfig patch X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 21 Sep 2007 00:56:27 -0000 --0eh6TmSyL6TZE2Uz Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Thu, Sep 20, 2007 at 07:39:27PM -0500, Brooks Davis wrote: > On Fri, Sep 21, 2007 at 11:54:27AM +1200, Andrew Thompson wrote: > > Hi, > > > > > > I have been digging into why the edsc module wasnt being loaded by > > ifconfig and now have a patch. > > > > A few printfs showed the problem. > > > > # ifconfig edsc0 create > > ifmaybeload(edsc0) > > trying to find if_edsc or edsc0 > > found @ ed > > > > Its comparing using the string length of the module name so any partial > > matches are going through. I have changed it so it strips the number > > from the interface name and uses the full string to match. > > > > I want to ask re@ soon so any feedback would be great. > > Conceptually the patch seems right. A couple comments below (I saw the strlcpy > change). > > -- Brooks > > > Index: ifconfig.c > > =================================================================== > > RCS file: /home/ncvs/src/sbin/ifconfig/ifconfig.c,v > > retrieving revision 1.133 > > diff -u -p -r1.133 ifconfig.c > > --- ifconfig.c 13 Jun 2007 18:07:59 -0000 1.133 > > +++ ifconfig.c 20 Sep 2007 23:47:28 -0000 > > @@ -897,7 +897,7 @@ ifmaybeload(const char *name) > > { > > struct module_stat mstat; > > int fileid, modid; > > - char ifkind[35], *dp; > > + char ifkind[35], ifname[32], *dp; > > const char *cp; > > Any reason ifname[32] shouldn't be ifname[IF_NAMESIZE]? > Should the if statement terminate the loop? fixed. I have found that the loop to create ifkind does not properly check the bounds of the passed string. I have reorganised the code to fix this, patch attached. Andrew --0eh6TmSyL6TZE2Uz Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="ifconfig_kldload2.diff" Index: ifconfig.c =================================================================== RCS file: /home/ncvs/src/sbin/ifconfig/ifconfig.c,v retrieving revision 1.133 diff -u -p -r1.133 ifconfig.c --- ifconfig.c 13 Jun 2007 18:07:59 -0000 1.133 +++ ifconfig.c 21 Sep 2007 00:49:45 -0000 @@ -897,19 +897,24 @@ ifmaybeload(const char *name) { struct module_stat mstat; int fileid, modid; - char ifkind[35], *dp; + char ifkind[IFNAMSIZ + 3], ifname[IFNAMSIZ], *dp; const char *cp; /* loading suppressed by the user */ if (noload) return; + /* trim the interface number off the end */ + strlcpy(ifname, name, sizeof(ifname)); + for (dp = ifname; *dp != 0; dp++) + if (isdigit(*dp)) { + *dp = 0; + break; + } + /* turn interface and unit into module name */ strcpy(ifkind, "if_"); - for (cp = name, dp = ifkind + 3; - (*cp != 0) && !isdigit(*cp); cp++, dp++) - *dp = *cp; - *dp = 0; + strlcpy(ifkind + 3, ifname, sizeof(ifkind) - 3); /* scan files in kernel */ mstat.version = sizeof(struct module_stat); @@ -926,8 +931,8 @@ ifmaybeload(const char *name) cp = mstat.name; } /* already loaded? */ - if (strncmp(name, cp, strlen(cp)) == 0 || - strncmp(ifkind, cp, strlen(cp)) == 0) + if (strncmp(ifname, cp, strlen(ifname)) == 0 || + strncmp(ifkind, cp, strlen(ifkind)) == 0) return; } } --0eh6TmSyL6TZE2Uz--