Hi,
The algorithm in route(8) and arp(6) is still not correct. While
the values written to the kernel are fine, the bytes for padding
are taken from memory after the sockaddr structs.
In route(8) the union of sockaddr can be made larger so that the
padding is taken from there.
In arp(8) we know the size of the struct. Copy only the struct and
advance over the padding. The memory has been zeroed before.
ndp(8) can take all the fixes from arp(8).
ok?
bluhm
Index: sbin/route/route.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sbin/route/route.c,v
retrieving revision 1.232
diff -u -p -r1.232 route.c
--- sbin/route/route.c 29 Aug 2019 22:42:16 -0000 1.232
+++ sbin/route/route.c 30 Aug 2019 13:24:31 -0000
@@ -137,10 +137,6 @@ usage(char *cp)
exit(1);
}
-#define ROUNDUP(a) \
- ((a) > 0 ? (1 + (((a) - 1) | (sizeof(long) - 1))) : sizeof(long))
-#define ADVANCE(x, n) (x += ROUNDUP((n)->sa_len))
-
int
main(int argc, char **argv)
{
Index: sbin/route/show.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sbin/route/show.h,v
retrieving revision 1.14
diff -u -p -r1.14 show.h
--- sbin/route/show.h 1 May 2018 18:13:21 -0000 1.14
+++ sbin/route/show.h 30 Aug 2019 13:22:57 -0000
@@ -19,6 +19,18 @@
#ifndef __SHOW_H__
#define __SHOW_H__
+#define ROUNDUP(a) \
+ ((a) > 0 ? (1 + (((a) - 1) | (sizeof(long) - 1))) : sizeof(long))
+#define ADVANCE(x, n) (x += ROUNDUP((n)->sa_len))
+#define MAXIMUM(a, b) ((a) > (b) ? (a) : (b))
+#define SAMAXSIZE \
+ MAXIMUM(sizeof(struct sockaddr), \
+ MAXIMUM(sizeof(struct sockaddr_in), \
+ MAXIMUM(sizeof(struct sockaddr_in6), \
+ MAXIMUM(sizeof(struct sockaddr_dl), \
+ MAXIMUM(sizeof(struct sockaddr_rtlabel), \
+ sizeof(struct sockaddr_mpls))))))
+
union sockunion {
struct sockaddr sa;
struct sockaddr_in sin;
@@ -26,6 +38,7 @@ union sockunion {
struct sockaddr_dl sdl;
struct sockaddr_rtlabel rtlabel;
struct sockaddr_mpls smpls;
+ char padding[ROUNDUP(SAMAXSIZE)];
};
void get_rtaddrs(int, struct sockaddr *, struct sockaddr **);
Index: usr.sbin/arp/arp.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/arp/arp.c,v
retrieving revision 1.85
diff -u -p -r1.85 arp.c
--- usr.sbin/arp/arp.c 29 Aug 2019 19:11:15 -0000 1.85
+++ usr.sbin/arp/arp.c 30 Aug 2019 13:57:15 -0000
@@ -675,9 +675,8 @@ rtmsg(int cmd)
#define NEXTADDR(w, s) \
if (rtm->rtm_addrs & (w)) { \
- l = ROUNDUP(((struct sockaddr *)&(s))->sa_len); \
- memcpy(cp, &(s), l); \
- cp += l; \
+ memcpy(cp, &(s), sizeof(s)); \
+ ADVANCE(cp, (struct sockaddr *)&(s)); \
}
NEXTADDR(RTA_DST, sin_m);
Index: usr.sbin/ndp/ndp.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/ndp/ndp.c,v
retrieving revision 1.97
diff -u -p -r1.97 ndp.c
--- usr.sbin/ndp/ndp.c 27 Aug 2019 20:42:40 -0000 1.97
+++ usr.sbin/ndp/ndp.c 30 Aug 2019 14:08:52 -0000
@@ -108,6 +108,7 @@
/* packing rule for routing socket */
#define ROUNDUP(a) \
((a) > 0 ? (1 + (((a) - 1) | (sizeof(long) - 1))) : sizeof(long))
+#define ADVANCE(x, n) (x += ROUNDUP((n)->sa_len))
static pid_t pid;
static int nflag;
@@ -323,7 +324,7 @@ parse_host(const char *host, struct in6_
struct sockaddr_in6 so_mask = {sizeof(so_mask), AF_INET6 };
struct sockaddr_in6 blank_sin = {sizeof(blank_sin), AF_INET6 }, sin_m;
struct sockaddr_dl blank_sdl = {sizeof(blank_sdl), AF_LINK }, sdl_m;
-struct sockaddr_dl ifp_m = { sizeof(&ifp_m), AF_LINK };
+struct sockaddr_dl ifp_m = { sizeof(ifp_m), AF_LINK };
time_t expire_time;
int flags, found_entry;
struct {
@@ -766,9 +767,12 @@ rtmsg(int cmd)
case RTM_GET:
rtm->rtm_addrs |= (RTA_DST | RTA_IFP);
}
-#define NEXTADDR(w, s) \
- if (rtm->rtm_addrs & (w)) { \
- bcopy((char *)&s, cp, sizeof(s)); cp += ROUNDUP(sizeof(s));}
+
+#define NEXTADDR(w, s) \
+ if (rtm->rtm_addrs & (w)) { \
+ memcpy(cp, &(s), sizeof(s)); \
+ ADVANCE(cp, (struct sockaddr *)&(s)); \
+ }
NEXTADDR(RTA_DST, sin_m);
NEXTADDR(RTA_GATEWAY, sdl_m);
@@ -825,7 +829,7 @@ rtget(struct sockaddr_in6 **sinp, struct
default:
break;
}
- cp += ROUNDUP(sa->sa_len);
+ ADVANCE(cp, sa);
}
}
}
