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
