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
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;
}
}
_______________________________________________
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "[EMAIL PROTECTED]"
