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 [email protected]
with "unsubscribe freebsd-current" in the body of the message

Reply via email to