On Sat, Aug 31, 2019 at 12:07:21AM +0200, Alexander Bluhm wrote:
> On Fri, Aug 30, 2019 at 09:54:49PM +0200, Claudio Jeker wrote:
> > Just throw a struct sockaddr_storage in that union. It will make sure
> > there is enough space for everything and then you can skip the MAXIMUM
> > dance you do now.
>
> Yes, that is much nicer. Although I have to work around this
> compiler warning.
>
> /home/bluhm/openbsd/cvs/src/sbin/route/route.c:911:19: warning: implicit
> conversion from 'unsigned long' to '__uint8_t' (aka 'unsigned char')
> changes value from 256 to 0 [-Wconstant-conversion]
> su->sa.sa_len = sizeof(*su);
> ~ ^~~~~~~~~~~
> 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 21:58:13 -0000
> @@ -484,7 +484,7 @@ newroute(int argc, char **argv)
> break;
> case K_SA:
> af = PF_ROUTE;
> - aflen = sizeof(union sockunion);
> + aflen = sizeof(struct sockaddr_storage) - 1;
> break;
> case K_MPLS:
> af = AF_MPLS;
> @@ -908,7 +908,7 @@ getaddr(int which, int af, char *s, stru
> case AF_MPLS:
> errx(1, "mpls labels require -in or -out switch");
> case PF_ROUTE:
> - su->sa.sa_len = sizeof(*su);
> + su->sa.sa_len = sizeof(struct sockaddr_storage) - 1;
> sockaddr(s, &su->sa);
> return (1);
>
Ugh, this could be cleaned up but that is a different diff.
I think aflen should be killed and instead the sa_len should be set based
on the af. Also sockaddr() could itself know about the max size and not
have that passed in via su->sa.sa_len.
Guess that is a different diff to make this not more insane.
Also when was the last time someone used -sa in route(8)?
> 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 21:46:34 -0000
> @@ -26,6 +26,7 @@ union sockunion {
> struct sockaddr_dl sdl;
> struct sockaddr_rtlabel rtlabel;
> struct sockaddr_mpls smpls;
> + struct sockaddr_storage padding;
> };
>
> 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);
> }
> }
> }
>
Diff is OK claudio@
--
:wq Claudio
