Date: Fri, 4 Jun 1999 17:18:57 -0700 (PDT) From: Archie Cobbs <archie@whistle.com> To: freebsd-current@freebsd.org Cc: freebsd-net@freebsd.org Subject: subtle SIOCGIFCONF bug Message-ID: <199906050018.RAA85167@bubba.whistle.com>
next in thread | raw e-mail | index | archive | help
I've noticed that several programs have a subtle problem when scanning the list returned by the SIOCGIFCONF ioctl. The problem is that these programs do this computation to increment the pointer in the list: ifr = (struct ifreq *) ((char *) &ifr->ifr_addr + ifr->ifr_addr.sa_len); There are cases where some elements in the list (e.g., tunnel interfaces) have ifr->ifr_addr.sa_len set to a value LESS than sizeof(ifr->ifr_addr). This causes these programs to fail because the kernel always enforces a minimum length of (IFNAMSIZ + sizeof(ifr->ifr_addr)) for each entry in the list, even if ifr->ifr_addr.sa_len < sizeof(ifr->ifr_addr) (see net/if.c:ifconf()). First question: Should the kernel insure that ifr->ifr_addr.sa_len is always at least sizeof(ifr->ifr_addr), or should the user programs adjust their pointer increment algorithm? At first I assumed the latter answer (patches below) but now am not so sure. Second question: It doesn't appear the net/if.c:ifioctl() function is protected at all by splnet(), even though it is accessing all kinds of networking information. Is this a race condition? Thanks, -Archie ___________________________________________________________________________ Archie Cobbs * Whistle Communications, Inc. * http://www.whistle.com Index: usr.sbin/arp/arp.c =================================================================== RCS file: /home/ncvs/src/usr.sbin/arp/arp.c,v retrieving revision 1.15 diff -u -r1.15 arp.c --- arp.c 1999/03/10 10:11:43 1.15 +++ arp.c 1999/06/04 23:45:27 @@ -696,8 +696,8 @@ break; } nextif: - ifr = (struct ifreq *) - ((char *)&ifr->ifr_addr + ifr->ifr_addr.sa_len); + ifr = (struct ifreq *) ((char *)&ifr->ifr_addr + + MAX(ifr->ifr_addr.sa_len, sizeof(ifr->ifr_addr))); } if (ifr >= ifend) { @@ -725,8 +725,8 @@ printf("\n"); return dla->sdl_alen; } - ifr = (struct ifreq *) - ((char *)&ifr->ifr_addr + ifr->ifr_addr.sa_len); + ifr = (struct ifreq *) ((char *)&ifr->ifr_addr + + MAX(ifr->ifr_addr.sa_len, sizeof(ifr->ifr_addr))); } return 0; } Index: usr.sbin/pppd/sys-bsd.c =================================================================== RCS file: /home/ncvs/src/usr.sbin/pppd/sys-bsd.c,v retrieving revision 1.15 diff -u -r1.15 sys-bsd.c --- sys-bsd.c 1998/06/21 04:47:21 1.15 +++ sys-bsd.c 1999/06/04 23:45:32 @@ -1378,8 +1378,9 @@ * address on the same subnet as `ipaddr'. */ ifend = (struct ifreq *) (ifc.ifc_buf + ifc.ifc_len); - for (ifr = ifc.ifc_req; ifr < ifend; ifr = (struct ifreq *) - ((char *)&ifr->ifr_addr + ifr->ifr_addr.sa_len)) { + for (ifr = ifc.ifc_req; ifr < ifend; + ifr = (struct ifreq *) ((char *)&ifr->ifr_addr + + MAX(ifr->ifr_addr.sa_len, sizeof(ifr->ifr_addr)))) { if (ifr->ifr_addr.sa_family == AF_INET) { ina = ((struct sockaddr_in *) &ifr->ifr_addr)->sin_addr.s_addr; strncpy(ifreq.ifr_name, ifr->ifr_name, sizeof(ifreq.ifr_name)); @@ -1425,7 +1426,8 @@ BCOPY(dla, hwaddr, dla->sdl_len); return 1; } - ifr = (struct ifreq *) ((char *)&ifr->ifr_addr + ifr->ifr_addr.sa_len); + ifr = (struct ifreq *) ((char *)&ifr->ifr_addr + + MAX(ifr->ifr_addr.sa_len, sizeof(ifr->ifr_addr))); } return 0; @@ -1468,8 +1470,9 @@ return mask; } ifend = (struct ifreq *) (ifc.ifc_buf + ifc.ifc_len); - for (ifr = ifc.ifc_req; ifr < ifend; ifr = (struct ifreq *) - ((char *)&ifr->ifr_addr + ifr->ifr_addr.sa_len)) { + for (ifr = ifc.ifc_req; ifr < ifend; + ifr = (struct ifreq *) ((char *)&ifr->ifr_addr + + MAX(ifr->ifr_addr.sa_len, sizeof(ifr->ifr_addr)))) { /* * Check the interface's internet address. */ Index: sbin/natd/natd.c =================================================================== RCS file: /home/ncvs/src/sbin/natd/natd.c,v retrieving revision 1.17 diff -u -r1.17 natd.c --- natd.c 1999/05/13 17:09:44 1.17 +++ natd.c 1999/06/04 23:54:32 @@ -762,6 +762,8 @@ } extra = ifPtr->ifr_addr.sa_len - sizeof (struct sockaddr); + if (extra < 0) + extra = 0; ifPtr++; ifPtr = (struct ifreq*) ((char*) ifPtr + extra); Index: sbin/route/route.c =================================================================== RCS file: /home/ncvs/src/sbin/route/route.c,v retrieving revision 1.30 diff -u -r1.30 route.c --- route.c 1999/06/01 13:14:07 1.30 +++ route.c 1999/06/04 23:54:36 @@ -794,7 +794,8 @@ (ifconf.ifc_buf + ifconf.ifc_len); ifr < ifr_end; ifr = (struct ifreq *) ((char *) &ifr->ifr_addr - + ifr->ifr_addr.sa_len)) { + + MAX(ifr->ifr_addr.sa_len, + sizeof(ifr->ifr_addr)))) { dl = (struct sockaddr_dl *)&ifr->ifr_addr; if (ifr->ifr_addr.sa_family == AF_LINK && (ifr->ifr_flags & IFF_POINTOPOINT) To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-net" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199906050018.RAA85167>