Subject: if_clone_lookup bogus?
To: None <tech-net@netbsd.org>
From: Quentin Garnier <netbsd@quatriemek.com>
List: tech-net
Date: 08/14/2003 01:40:48
Maybe I'm mistaken, but if_clone_lookup in sys/net/if.c looks bogus to me.
Imagine the case where both ppp(4) and pppoe(4) are clonable and compiled,
and "ppp" appears before "pppoe" in the cloners list.
When requesting creation of a pppoe interface, if_clone_lookup will
mistakenly match "ppp" and then fails because it is not followed by a
number.
I propose the following patch. It makes the thing (slightly) less
obfuscated and make some boundary check on the name parameter.
Index: if.c
===================================================================
RCS file: /cvsroot/src/sys/net/if.c,v
retrieving revision 1.127
diff -u -r1.127 if.c
--- if.c 2003/08/09 18:13:03 1.127
+++ if.c 2003/08/13 23:29:10
@@ -795,24 +795,27 @@
int *unitp;
{
struct if_clone *ifc;
- const char *cp;
- size_t i;
+ const char *cp = name;
+ size_t i, j;
- for (ifc = LIST_FIRST(&if_cloners); ifc != NULL;) {
- for (cp = name, i = 0; i < ifc->ifc_namelen; i++, cp++) {
- if (ifc->ifc_name[i] != *cp)
- goto next_ifc;
- }
- goto found_name;
- next_ifc:
- ifc = LIST_NEXT(ifc, ifc_list);
- }
+ /* separate interface name from unit */
+ for (j = 0;
+ j < IFNAMSIZ && *cp && *cp < '0' && *cp > '9';
+ j++, cp++)
+ continue;
- /* No match. */
- return (NULL);
+ if (!j || j == IFNAMSIZ || !*cp)
+ return (NULL); /* No name or unit number */
- found_name:
- for (i = 0; *cp != '\0'; cp++) {
+ LIST_FOREACH(ifc, &if_cloners, ifc_list)
+ if (strlen(ifc->ifc_name) == j &&
+ !strncmp(name, ifc->ifc_name, j))
+ break;
+
+ if (ifc == NULL)
+ return (NULL);
+
+ for (i = 0; j < IFNAMSIZ && *cp != '\0'; cp++, j++) {
if (*cp < '0' || *cp > '9') {
/* Bogus unit number. */
return (NULL);
--
Quentin Garnier - cube@cubidou.net
"Feels like I'm fiddling while Rome is burning down.
Should I lay my fiddle down and take a rifle from the ground ?"
Leigh Nash/Sixpence None The Richer, Paralyzed, Divine Discontents, 2002.