Date: Fri, 21 Sep 2007 12:56:23 +1200 From: Andrew Thompson <thompsa@FreeBSD.org> To: Brooks Davis <brooks@FreeBSD.org> Cc: FreeBSD-net <freebsd-net@FreeBSD.org> Subject: Re: ifconfig patch Message-ID: <20070921005623.GD46172@heff.fud.org.nz> In-Reply-To: <20070921003927.GB77167@lor.one-eyed-alien.net> References: <20070920235427.GA46172@heff.fud.org.nz> <20070921003927.GB77167@lor.one-eyed-alien.net>
next in thread | previous in thread | raw e-mail | index | archive | help
--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--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20070921005623.GD46172>