After multiple ifconfig create and destroy of bridge and tun interfaces rtadvd
crashed just seconds after being started. I had 7 interfaces, but tun0
had ifm_index == 16. rtadvd allocated insufficient space for iflist. iflist
got initialized but then it's tail got overwritten. This diff determines the
maximum ifm_index (instead of "roughly estimate") and allocates the correct
number of bytes.

Index: if.c
===================================================================
RCS file: /cvs/src/usr.sbin/rtadvd/if.c,v
retrieving revision 1.23
diff -u -r1.23 if.c
--- if.c        21 May 2010 13:41:23 -0000      1.23
+++ if.c        28 May 2011 11:16:36 -0000
@@ -455,20 +455,11 @@
 static void
 parse_iflist(struct if_msghdr ***ifmlist_p, char *buf, size_t bufsize)
 {
-       int iflentry_size, malloc_size;
+       int malloc_size;
        struct if_msghdr *ifm;
        struct ifa_msghdr *ifam;
        char *lim;
-
-       /*
-        * Estimate least size of an iflist entry, to be obtained from kernel.
-        * Should add sizeof(sockaddr) ??
-        */
-       iflentry_size = sizeof(struct if_msghdr);
-       /* roughly estimate max list size of pointers to each if_msghdr */
-       malloc_size = (bufsize/iflentry_size) * sizeof(size_t);
-       if ((*ifmlist_p = (struct if_msghdr **)malloc(malloc_size)) == NULL)
-               fatal("malloc");
+       int max_ifm_index = 0;
 
        lim = buf + bufsize;
        for (ifm = (struct if_msghdr *)buf; ifm < (struct if_msghdr *)lim;) {
@@ -479,7 +470,8 @@
                }
                if (ifm->ifm_type == RTM_IFINFO) {
                        if (ifm->ifm_version == RTM_VERSION)
-                               (*ifmlist_p)[ifm->ifm_index] = ifm;
+                               if (ifm->ifm_index > max_ifm_index)
+                                       max_ifm_index = ifm->ifm_index;
                } else {
                        log_warn("out of sync parsing NET_RT_IFLIST,"
                            " expected %d, got %d, msglen = %d,"
@@ -487,6 +479,34 @@
                            RTM_IFINFO, ifm->ifm_type, ifm->ifm_msglen,
                            buf, ifm, lim);
                        exit(1);
+               }
+               for (ifam = (struct ifa_msghdr *)
+                       ((char *)ifm + ifm->ifm_msglen);
+                    ifam < (struct ifa_msghdr *)lim;
+                    ifam = (struct ifa_msghdr *)
+                       ((char *)ifam + ifam->ifam_msglen)) {
+                       /* just for safety */
+                       if (!ifam->ifam_msglen) {
+                               log_warnx("ifa_msglen is 0 "
+                                   "(buf=%p lim=%p ifam=%p)",
+                                   buf, lim, ifam);
+                               return;
+                       }
+                       if (ifam->ifam_type != RTM_NEWADDR)
+                               break;
+               }
+               ifm = (struct if_msghdr *)ifam;
+       }
+
+       malloc_size = (max_ifm_index + 1) * sizeof(size_t);
+       if ((*ifmlist_p = (struct if_msghdr **)malloc(malloc_size)) == NULL)
+               fatal("malloc");
+
+       for (ifm = (struct if_msghdr *)buf; ifm < (struct if_msghdr *)lim;) {
+               if (ifm->ifm_type == RTM_IFINFO) {
+                       if (ifm->ifm_version == RTM_VERSION) {
+                               (*ifmlist_p)[ifm->ifm_index] = ifm;
+                       }
                }
                for (ifam = (struct ifa_msghdr *)
                        ((char *)ifm + ifm->ifm_msglen);

-- 
Michal Mazurek

Reply via email to