So rtadvd has this complex code that tries to parse multiple routing
messages, when it actually reads only one message at a time from the
routing socket.  The diff below attempts to acknowledge this and tries
to be as mechanical as possible, variable renaming/gc'ing can happen
later.  Better use cvs diff -b to review this diff.

Thoughts?  ok?


Index: if.c
===================================================================
RCS file: /d/cvs/src/usr.sbin/rtadvd/if.c,v
retrieving revision 1.44
diff -u -p -r1.44 if.c
--- if.c        12 Jul 2017 06:11:45 -0000      1.44
+++ if.c        8 Aug 2017 07:45:27 -0000
@@ -186,80 +186,70 @@ lladdropt_fill(struct sockaddr_dl *sdl, 
 }
 
 #define SIN6(s) ((struct sockaddr_in6 *)(s))
-char *
-get_next_msg(char *buf, char *lim, size_t *lenp)
+int
+validate_msg(char *buf)
 {
-       struct rt_msghdr *rtm;
+       struct rt_msghdr *rtm = (struct rt_msghdr *)buf;
        struct ifa_msghdr *ifam;
        struct sockaddr *sa, *dst, *ifa, *rti_info[RTAX_MAX];
 
-       *lenp = 0;
-       for (rtm = (struct rt_msghdr *)buf;
-            rtm < (struct rt_msghdr *)lim;
-            rtm = (struct rt_msghdr *)((char *)rtm + rtm->rtm_msglen)) {
-               /* just for safety */
-               if (!rtm->rtm_msglen) {
-                       log_warnx("rtm_msglen is 0 (buf=%p lim=%p rtm=%p)",
-                           buf, lim, rtm);
-                       break;
-               }
-               if (rtm->rtm_version != RTM_VERSION)
-                       continue;
-
-               switch (rtm->rtm_type) {
-               case RTM_ADD:
-               case RTM_DELETE:
-                       if (rtm->rtm_tableid != 0)
-                               continue;
-
-                       /* address related checks */
-                       sa = (struct sockaddr *)((char *)rtm + rtm->rtm_hdrlen);
-                       get_rtaddrs(rtm->rtm_addrs, sa, rti_info);
-                       if ((dst = rti_info[RTAX_DST]) == NULL ||
-                           dst->sa_family != AF_INET6)
-                               continue;
-
-                       if (IN6_IS_ADDR_LINKLOCAL(&SIN6(dst)->sin6_addr) ||
-                           IN6_IS_ADDR_MULTICAST(&SIN6(dst)->sin6_addr))
-                               continue;
-
-                       if (rti_info[RTAX_NETMASK] == NULL)
-                               continue;
-
-                       /* found */
-                       *lenp = rtm->rtm_msglen;
-                       return (char *)rtm;
-                       /* NOTREACHED */
-               case RTM_NEWADDR:
-               case RTM_DELADDR:
-                       ifam = (struct ifa_msghdr *)rtm;
-
-                       /* address related checks */
-                       sa = (struct sockaddr *)((char *)rtm + rtm->rtm_hdrlen);
-                       get_rtaddrs(ifam->ifam_addrs, sa, rti_info);
-                       if ((ifa = rti_info[RTAX_IFA]) == NULL ||
-                           (ifa->sa_family != AF_INET &&
-                            ifa->sa_family != AF_INET6))
-                               continue;
-
-                       if (ifa->sa_family == AF_INET6 &&
-                           (IN6_IS_ADDR_LINKLOCAL(&SIN6(ifa)->sin6_addr) ||
-                            IN6_IS_ADDR_MULTICAST(&SIN6(ifa)->sin6_addr)))
-                               continue;
-
-                       /* found */
-                       *lenp = rtm->rtm_msglen;
-                       return (char *)rtm;
-                       /* NOTREACHED */
-               case RTM_IFINFO:
-                       /* found */
-                       *lenp = rtm->rtm_msglen;
-                       return (char *)rtm;
-                       /* NOTREACHED */
-               }
+       /* just for safety */
+       if (!rtm->rtm_msglen) {
+               log_warnx("rtm_msglen is 0 (rtm=%p)", rtm);
+               return -1;
        }
+       if (rtm->rtm_version != RTM_VERSION)
+               return -1;
 
-       return (char *)rtm;
+       switch (rtm->rtm_type) {
+       case RTM_ADD:
+       case RTM_DELETE:
+               if (rtm->rtm_tableid != 0)
+                       return -1;
+
+               /* address related checks */
+               sa = (struct sockaddr *)((char *)rtm + rtm->rtm_hdrlen);
+               get_rtaddrs(rtm->rtm_addrs, sa, rti_info);
+               if ((dst = rti_info[RTAX_DST]) == NULL ||
+                   dst->sa_family != AF_INET6)
+                       return -1;
+
+               if (IN6_IS_ADDR_LINKLOCAL(&SIN6(dst)->sin6_addr) ||
+                   IN6_IS_ADDR_MULTICAST(&SIN6(dst)->sin6_addr))
+                       return -1;
+
+               if (rti_info[RTAX_NETMASK] == NULL)
+                       return -1;
+
+               /* found */
+               return 0;
+               /* NOTREACHED */
+       case RTM_NEWADDR:
+       case RTM_DELADDR:
+               ifam = (struct ifa_msghdr *)rtm;
+
+               /* address related checks */
+               sa = (struct sockaddr *)((char *)rtm + rtm->rtm_hdrlen);
+               get_rtaddrs(ifam->ifam_addrs, sa, rti_info);
+               if ((ifa = rti_info[RTAX_IFA]) == NULL ||
+                   (ifa->sa_family != AF_INET &&
+                   ifa->sa_family != AF_INET6))
+                       return -1;
+
+               if (ifa->sa_family == AF_INET6 &&
+                   (IN6_IS_ADDR_LINKLOCAL(&SIN6(ifa)->sin6_addr) ||
+                   IN6_IS_ADDR_MULTICAST(&SIN6(ifa)->sin6_addr)))
+                       return -1;
+
+               /* found */
+               return 0;
+               /* NOTREACHED */
+       case RTM_IFINFO:
+               /* found */
+               return 0;
+               /* NOTREACHED */
+       }
+       return -1;
 }
 
 struct in6_addr *
Index: if.h
===================================================================
RCS file: /d/cvs/src/usr.sbin/rtadvd/if.h,v
retrieving revision 1.13
diff -u -p -r1.13 if.h
--- if.h        29 Jun 2016 14:19:38 -0000      1.13
+++ if.h        8 Aug 2017 07:45:27 -0000
@@ -40,7 +40,7 @@ int if_getmtu(char *);
 int if_getflags(int, int);
 int lladdropt_length(struct sockaddr_dl *);
 void lladdropt_fill(struct sockaddr_dl *, struct nd_opt_hdr *);
-char *get_next_msg(char *, char *, size_t *);
+int validate_msg(char *);
 struct in6_addr *get_addr(char *);
 int get_rtm_ifindex(char *);
 int get_ifm_ifindex(char *);
Index: rtadvd.c
===================================================================
RCS file: /d/cvs/src/usr.sbin/rtadvd/rtadvd.c,v
retrieving revision 1.87
diff -u -p -r1.87 rtadvd.c
--- rtadvd.c    12 Jul 2017 06:11:07 -0000      1.87
+++ rtadvd.c    8 Aug 2017 07:45:27 -0000
@@ -303,9 +303,8 @@ die_cb(int sig, short event, void *arg)
 static void
 rtsock_cb(int fd, short event, void *arg)
 {
-       int n, type, ifindex = 0, plen;
-       size_t len;
-       char *next, *lim;
+       int n, type, ifindex = 0, oldifflags, plen;
+       char *next;
        u_char ifname[IF_NAMESIZE];
        struct prefix *prefix;
        struct rainfo *rai;
@@ -315,148 +314,129 @@ rtsock_cb(int fd, short event, void *arg
        n = read(rtsock, rtsockbuf, rtsockbuflen);
        log_debug("received a routing message "
            "(type = %d, len = %d)", rtmsg_type(rtsockbuf), n);
-       if (n > rtmsg_len(rtsockbuf)) {
-               /*
-                * This usually won't happen for messages received on
-                * a routing socket.
-                */
-               log_debug("received data length is larger than "
-                   "1st routing message len. multiple messages? "
-                   "read %d bytes, but 1st msg len = %d",
-                   n, rtmsg_len(rtsockbuf));
-#if 0
-               /* adjust length */
-               n = rtmsg_len(rtsockbuf);
-#endif
-       }
 
-       lim = rtsockbuf + n;
-       for (next = rtsockbuf; next < lim; next += len) {
-               int oldifflags;
+       next = rtsockbuf;
+       if (validate_msg(next) == -1)
+               return;
+
+       type = rtmsg_type(next);
+       switch (type) {
+       case RTM_ADD:
+       case RTM_DELETE:
+               ifindex = get_rtm_ifindex(next);
+               break;
+       case RTM_NEWADDR:
+       case RTM_DELADDR:
+               ifindex = get_ifam_ifindex(next);
+               break;
+       case RTM_IFINFO:
+               ifindex = get_ifm_ifindex(next);
+               break;
+       default:
+               /* should not reach here */
+               log_debug("unknown rtmsg %d on %s",
+                   type, if_indextoname(ifindex, ifname));
+               return;
+       }
 
-               next = get_next_msg(next, lim, &len);
+       if ((rai = if_indextorainfo(ifindex)) == NULL) {
+               log_debug("route changed on "
+                   "non advertising interface(%s)",
+                   if_indextoname(ifindex, ifname));
+               return;
+       }
+       oldifflags = iflist[ifindex]->ifm_flags;
 
-               if (len == 0)
-                       break;
-               type = rtmsg_type(next);
-               switch (type) {
-               case RTM_ADD:
-               case RTM_DELETE:
-                       ifindex = get_rtm_ifindex(next);
+       switch (type) {
+       case RTM_ADD:
+               /* init ifflags because it may have changed */
+               iflist[ifindex]->ifm_flags =
+                       if_getflags(ifindex, iflist[ifindex]->ifm_flags);
+
+               if (sflag)
+                       break;  /* we aren't interested in prefixes  */
+
+               addr = get_addr(next);
+               plen = get_prefixlen(next);
+               /* sanity check for plen */
+               /* as RFC2373, prefixlen is at least 4 */
+               if (plen < 4 || plen > 127) {
+                       log_info("new interface route's"
+                           " plen %d is invalid for a prefix", plen);
                        break;
-               case RTM_NEWADDR:
-               case RTM_DELADDR:
-                       ifindex = get_ifam_ifindex(next);
-                       break;
-               case RTM_IFINFO:
-                       ifindex = get_ifm_ifindex(next);
-                       break;
-               default:
-                       /* should not reach here */
-                       log_debug("unknown rtmsg %d on %s",
-                           type, if_indextoname(ifindex, ifname));
-                       continue;
-               }
-
-               if ((rai = if_indextorainfo(ifindex)) == NULL) {
-                       log_debug("route changed on "
-                           "non advertising interface(%s)",
-                           if_indextoname(ifindex, ifname));
-                       continue;
                }
-               oldifflags = iflist[ifindex]->ifm_flags;
-
-               switch (type) {
-               case RTM_ADD:
-                       /* init ifflags because it may have changed */
-                       iflist[ifindex]->ifm_flags =
-                           if_getflags(ifindex, iflist[ifindex]->ifm_flags);
-
-                       if (sflag)
-                               break;  /* we aren't interested in prefixes  */
-
-                       addr = get_addr(next);
-                       plen = get_prefixlen(next);
-                       /* sanity check for plen */
-                       /* as RFC2373, prefixlen is at least 4 */
-                       if (plen < 4 || plen > 127) {
-                               log_info("new interface route's"
-                                   " plen %d is invalid for a prefix", plen);
-                               break;
-                       }
-                       prefix = find_prefix(rai, addr, plen);
-                       if (prefix) {
-                               log_debug("new prefix(%s/%d) "
-                                   "added on %s, "
-                                   "but it was already in list",
-                                   inet_ntop(AF_INET6, addr,
-                                       addrbuf, INET6_ADDRSTRLEN),
-                                   plen, rai->ifname);
-                               break;
-                       }
-                       make_prefix(rai, ifindex, addr, plen);
+               prefix = find_prefix(rai, addr, plen);
+               if (prefix) {
+                       log_debug("new prefix(%s/%d) "
+                           "added on %s, "
+                           "but it was already in list",
+                           inet_ntop(AF_INET6, addr,
+                               addrbuf, INET6_ADDRSTRLEN),
+                           plen, rai->ifname);
                        break;
-               case RTM_DELETE:
-                       /* init ifflags because it may have changed */
-                       iflist[ifindex]->ifm_flags =
-                           if_getflags(ifindex, iflist[ifindex]->ifm_flags);
-
-                       if (sflag)
-                               break;
-
-                       addr = get_addr(next);
-                       plen = get_prefixlen(next);
-                       /* sanity check for plen */
-                       /* as RFC2373, prefixlen is at least 4 */
-                       if (plen < 4 || plen > 127) {
-                               log_info("deleted interface route's "
-                                   "plen %d is invalid for a prefix", plen);
-                               break;
-                       }
-                       prefix = find_prefix(rai, addr, plen);
-                       if (prefix == NULL) {
-                               log_debug("prefix(%s/%d) was "
-                                   "deleted on %s, "
-                                   "but it was not in list",
-                                   inet_ntop(AF_INET6, addr,
-                                       addrbuf, INET6_ADDRSTRLEN),
-                                   plen, rai->ifname);
-                               break;
-                       }
-                       delete_prefix(rai, prefix);
-                       break;
-               case RTM_NEWADDR:
-               case RTM_DELADDR:
-                       /* init ifflags because it may have changed */
-                       iflist[ifindex]->ifm_flags =
-                           if_getflags(ifindex, iflist[ifindex]->ifm_flags);
+               }
+               make_prefix(rai, ifindex, addr, plen);
+               break;
+       case RTM_DELETE:
+               /* init ifflags because it may have changed */
+               iflist[ifindex]->ifm_flags =
+                       if_getflags(ifindex, iflist[ifindex]->ifm_flags);
+
+               if (sflag)
+                       break;
+
+               addr = get_addr(next);
+               plen = get_prefixlen(next);
+               /* sanity check for plen */
+               /* as RFC2373, prefixlen is at least 4 */
+               if (plen < 4 || plen > 127) {
+                       log_info("deleted interface route's "
+                           "plen %d is invalid for a prefix", plen);
                        break;
-               case RTM_IFINFO:
-                       iflist[ifindex]->ifm_flags = get_ifm_flags(next);
+               }
+               prefix = find_prefix(rai, addr, plen);
+               if (prefix == NULL) {
+                       log_debug("prefix(%s/%d) was "
+                           "deleted on %s, "
+                           "but it was not in list",
+                           inet_ntop(AF_INET6, addr,
+                               addrbuf, INET6_ADDRSTRLEN),
+                           plen, rai->ifname);
                        break;
-               default:
-                       /* should not reach here */
-                       log_debug("unknown rtmsg %d on %s",
-                           type, if_indextoname(ifindex, ifname));
-                       return;
                }
+               delete_prefix(rai, prefix);
+               break;
+       case RTM_NEWADDR:
+       case RTM_DELADDR:
+               /* init ifflags because it may have changed */
+               iflist[ifindex]->ifm_flags =
+                       if_getflags(ifindex, iflist[ifindex]->ifm_flags);
+               break;
+       case RTM_IFINFO:
+               iflist[ifindex]->ifm_flags = get_ifm_flags(next);
+               break;
+       default:
+               /* should not reach here */
+               log_debug("unknown rtmsg %d on %s",
+                   type, if_indextoname(ifindex, ifname));
+               return;
+       }
 
-               /* check if an interface flag is changed */
-               if ((oldifflags & IFF_UP) != 0 &&       /* UP to DOWN */
-                   (iflist[ifindex]->ifm_flags & IFF_UP) == 0) {
-                       log_info("interface %s becomes down. stop timer.",
-                           rai->ifname);
-                       evtimer_del(&rai->timer.ev);
-               } else if ((oldifflags & IFF_UP) == 0 && /* DOWN to UP */
-                        (iflist[ifindex]->ifm_flags & IFF_UP) != 0) {
-                       log_info("interface %s becomes up. restart timer.",
-                           rai->ifname);
-
-                       rai->initcounter = 0; /* reset the counter */
-                       rai->waiting = 0; /* XXX */
-                       ra_timer_update(rai);
-                       evtimer_add(&rai->timer.ev, &rai->timer.tm);
-               }
+       /* check if an interface flag is changed */
+       if ((oldifflags & IFF_UP) != 0 &&       /* UP to DOWN */
+           (iflist[ifindex]->ifm_flags & IFF_UP) == 0) {
+               log_info("interface %s becomes down. stop timer.",
+                   rai->ifname);
+               evtimer_del(&rai->timer.ev);
+       } else if ((oldifflags & IFF_UP) == 0 && /* DOWN to UP */
+           (iflist[ifindex]->ifm_flags & IFF_UP) != 0) {
+               log_info("interface %s becomes up. restart timer.",
+                   rai->ifname);
+
+               rai->initcounter = 0; /* reset the counter */
+               rai->waiting = 0; /* XXX */
+               ra_timer_update(rai);
+               evtimer_add(&rai->timer.ev, &rai->timer.tm);
        }
 }
 


-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Reply via email to