ifconfig patch

[Andrew Thompson]

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
-------------- next part --------------
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;
 		}
 	}