On 20/08/15(Thu) 17:48, Christian Weisgerber wrote:
> Currently, ndp -p will trigger unaligned accesses in the kernel and
> userland on architectures that require strict alignment for 64-bit
> types (e.g. sparc64).
>
> This is actually fallout from the time_t change.
>
> The sysctl used to pass the prefix list uses a packed format:
> one struct in6_prefix, several struct sockaddr_in6,
> one struct in6_prefix, several struct sockaddr_in6,
> ...
> This worked dandily when both structs had the same alignment
> requirement (int32_t). However, there's a time_t in in6_prefix
> which pushed that struct's alignment to int64_t... boom!
>
> The patches below are taken from NetBSD (Martin Husemann):
> http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/netinet6/nd6.c.diff?r1=1.145&r2=1.146
> http://cvsweb.netbsd.org/bsdweb.cgi/src/usr.sbin/ndp/ndp.c.diff?r1=1.41&r2=1.42
>
> Use simple byte pointer arithmetic and memcpy from/to aligned stack
> variables to handle the "packed" binary format passed out to userland
> when querying the prefix/router list.
>
> OK?
ok mpi@
>
> Index: sys/netinet6/nd6.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/nd6.c,v
> retrieving revision 1.143
> diff -u -p -r1.143 nd6.c
> --- sys/netinet6/nd6.c 16 Jul 2015 15:31:35 -0000 1.143
> +++ sys/netinet6/nd6.c 20 Aug 2015 14:36:19 -0000
> @@ -1877,45 +1877,43 @@ fill_prlist(void *oldp, size_t *oldlenp,
> {
> int error = 0, s;
> struct nd_prefix *pr;
> - struct in6_prefix *p = NULL;
> - struct in6_prefix *pe = NULL;
> + char *p = NULL, *ps = NULL;
> + char *pe = NULL;
> size_t l;
>
> s = splsoftnet();
>
> if (oldp) {
> - p = (struct in6_prefix *)oldp;
> - pe = (struct in6_prefix *)((caddr_t)oldp + *oldlenp);
> + ps = p = (char *)oldp;
> + pe = (char *)oldp + *oldlenp;
> }
> l = 0;
>
> LIST_FOREACH(pr, &nd_prefix, ndpr_entry) {
> u_short advrtrs;
> - size_t advance;
> - struct sockaddr_in6 *sin6;
> - struct sockaddr_in6 *s6;
> + struct sockaddr_in6 sin6;
> struct nd_pfxrouter *pfr;
> + struct in6_prefix pfx;
> char addr[INET6_ADDRSTRLEN];
>
> - if (oldp && p + 1 <= pe)
> - {
> - bzero(p, sizeof(*p));
> - sin6 = (struct sockaddr_in6 *)(p + 1);
> -
> - p->prefix = pr->ndpr_prefix;
> - if (in6_recoverscope(&p->prefix,
> - &p->prefix.sin6_addr, pr->ndpr_ifp) != 0)
> + if (oldp && p + sizeof(struct in6_prefix) <= pe) {
> + memset(&pfx, 0, sizeof(pfx));
> + ps = p;
> +
> + pfx.prefix = pr->ndpr_prefix;
> + if (in6_recoverscope(&pfx.prefix,
> + &pfx.prefix.sin6_addr, pr->ndpr_ifp) != 0)
> log(LOG_ERR,
> "scope error in prefix list (%s)\n",
> - inet_ntop(AF_INET6, &p->prefix.sin6_addr,
> + inet_ntop(AF_INET6, &pfx.prefix.sin6_addr,
> addr, sizeof(addr)));
> - p->raflags = pr->ndpr_raf;
> - p->prefixlen = pr->ndpr_plen;
> - p->vltime = pr->ndpr_vltime;
> - p->pltime = pr->ndpr_pltime;
> - p->if_index = pr->ndpr_ifp->if_index;
> + pfx.raflags = pr->ndpr_raf;
> + pfx.prefixlen = pr->ndpr_plen;
> + pfx.vltime = pr->ndpr_vltime;
> + pfx.pltime = pr->ndpr_pltime;
> + pfx.if_index = pr->ndpr_ifp->if_index;
> if (pr->ndpr_vltime == ND6_INFINITE_LIFETIME)
> - p->expire = 0;
> + pfx.expire = 0;
> else {
> time_t maxexpire;
>
> @@ -1924,40 +1922,44 @@ fill_prlist(void *oldp, size_t *oldlenp,
> ((sizeof(maxexpire) * 8) - 1));
> if (pr->ndpr_vltime <
> maxexpire - pr->ndpr_lastupdate) {
> - p->expire = pr->ndpr_lastupdate +
> + pfx.expire = pr->ndpr_lastupdate +
> pr->ndpr_vltime;
> } else
> - p->expire = maxexpire;
> + pfx.expire = maxexpire;
> }
> - p->refcnt = pr->ndpr_refcnt;
> - p->flags = pr->ndpr_stateflags;
> - p->origin = PR_ORIG_RA;
> + pfx.refcnt = pr->ndpr_refcnt;
> + pfx.flags = pr->ndpr_stateflags;
> + pfx.origin = PR_ORIG_RA;
> +
> + p += sizeof(pfx); l += sizeof(pfx);
> +
> advrtrs = 0;
> LIST_FOREACH(pfr, &pr->ndpr_advrtrs, pfr_entry) {
> - if ((void *)&sin6[advrtrs + 1] > (void *)pe) {
> + if (p + sizeof(sin6) > pe) {
> advrtrs++;
> continue;
> }
> - s6 = &sin6[advrtrs];
> - s6->sin6_family = AF_INET6;
> - s6->sin6_len = sizeof(struct sockaddr_in6);
> - s6->sin6_addr = pfr->router->rtaddr;
> - in6_recoverscope(s6, &pfr->router->rtaddr,
> + sin6.sin6_family = AF_INET6;
> + sin6.sin6_len = sizeof(struct sockaddr_in6);
> + sin6.sin6_addr = pfr->router->rtaddr;
> + in6_recoverscope(&sin6, &pfr->router->rtaddr,
> pfr->router->ifp);
> advrtrs++;
> + memcpy(p, &sin6, sizeof(sin6));
> + p += sizeof(sin6);
> + l += sizeof(sin6);
> }
> - p->advrtrs = advrtrs;
> + pfx.advrtrs = advrtrs;
> + memcpy(ps, &pfx, sizeof(pfx));
> }
> else {
> + l += sizeof(pfx);
> advrtrs = 0;
> - LIST_FOREACH(pfr, &pr->ndpr_advrtrs, pfr_entry)
> + LIST_FOREACH(pfr, &pr->ndpr_advrtrs, pfr_entry) {
> advrtrs++;
> + l += sizeof(sin6);
> + }
> }
> -
> - advance = sizeof(*p) + sizeof(*sin6) * advrtrs;
> - l += advance;
> - if (p)
> - p = (struct in6_prefix *)((caddr_t)p + advance);
> }
>
> if (oldp) {
> Index: usr.sbin/ndp/ndp.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ndp/ndp.c,v
> retrieving revision 1.61
> diff -u -p -r1.61 ndp.c
> --- usr.sbin/ndp/ndp.c 3 Jun 2015 08:10:53 -0000 1.61
> +++ usr.sbin/ndp/ndp.c 20 Aug 2015 15:09:37 -0000
> @@ -1043,9 +1043,8 @@ void
> plist(void)
> {
> int mib[] = { CTL_NET, PF_INET6, IPPROTO_ICMPV6, ICMPV6CTL_ND6_PRLIST };
> - char *buf;
> - struct in6_prefix *p, *ep, *n;
> - struct sockaddr_in6 *advrtr;
> + char *buf, *p, *ep;
> + struct in6_prefix pfx;
> size_t l;
> struct timeval now;
> const int niflags = NI_NUMERICHOST;
> @@ -1066,17 +1065,17 @@ plist(void)
> /*NOTREACHED*/
> }
>
> - ep = (struct in6_prefix *)(buf + l);
> - for (p = (struct in6_prefix *)buf; p < ep; p = n) {
> - advrtr = (struct sockaddr_in6 *)(p + 1);
> - n = (struct in6_prefix *)&advrtr[p->advrtrs];
> + ep = buf + l;
> + for (p = buf; p < ep; ) {
> + memcpy(&pfx, p, sizeof(pfx));
> + p += sizeof(pfx);
>
> - if (getnameinfo((struct sockaddr *)&p->prefix,
> - p->prefix.sin6_len, namebuf, sizeof(namebuf),
> + if (getnameinfo((struct sockaddr *)&pfx.prefix,
> + pfx.prefix.sin6_len, namebuf, sizeof(namebuf),
> NULL, 0, niflags) != 0)
> strlcpy(namebuf, "?", sizeof(namebuf));
> - printf("%s/%d if=%s\n", namebuf, p->prefixlen,
> - if_indextoname(p->if_index, ifix_buf));
> + printf("%s/%d if=%s\n", namebuf, pfx.prefixlen,
> + if_indextoname(pfx.if_index, ifix_buf));
>
> gettimeofday(&now, 0);
> /*
> @@ -1084,50 +1083,52 @@ plist(void)
> * by origin. notify the difference to the users.
> */
> printf("flags=%s%s%s%s%s",
> - p->raflags.onlink ? "L" : "",
> - p->raflags.autonomous ? "A" : "",
> - (p->flags & NDPRF_ONLINK) != 0 ? "O" : "",
> - (p->flags & NDPRF_DETACHED) != 0 ? "D" : "",
> - (p->flags & NDPRF_HOME) != 0 ? "H" : ""
> + pfx.raflags.onlink ? "L" : "",
> + pfx.raflags.autonomous ? "A" : "",
> + (pfx.flags & NDPRF_ONLINK) != 0 ? "O" : "",
> + (pfx.flags & NDPRF_DETACHED) != 0 ? "D" : "",
> + (pfx.flags & NDPRF_HOME) != 0 ? "H" : ""
> );
> - if (p->vltime == ND6_INFINITE_LIFETIME)
> + if (pfx.vltime == ND6_INFINITE_LIFETIME)
> printf(" vltime=infinity");
> else
> - printf(" vltime=%lu", (unsigned long)p->vltime);
> - if (p->pltime == ND6_INFINITE_LIFETIME)
> + printf(" vltime=%lu", (unsigned long)pfx.vltime);
> + if (pfx.pltime == ND6_INFINITE_LIFETIME)
> printf(", pltime=infinity");
> else
> - printf(", pltime=%lu", (unsigned long)p->pltime);
> - if (p->expire == 0)
> + printf(", pltime=%lu", (unsigned long)pfx.pltime);
> + if (pfx.expire == 0)
> printf(", expire=Never");
> - else if (p->expire >= now.tv_sec)
> + else if (pfx.expire >= now.tv_sec)
> printf(", expire=%s",
> - sec2str(p->expire - now.tv_sec));
> + sec2str(pfx.expire - now.tv_sec));
> else
> printf(", expired");
> - printf(", ref=%d", p->refcnt);
> + printf(", ref=%d", pfx.refcnt);
> printf("\n");
> /*
> * "advertising router" list is meaningful only if the prefix
> * information is from RA.
> */
> - if (p->advrtrs) {
> + if (pfx.advrtrs) {
> int j;
> - struct sockaddr_in6 *sin6;
> + struct sockaddr_in6 sin6;
>
> - sin6 = advrtr;
> printf(" advertised by\n");
> - for (j = 0; j < p->advrtrs; j++) {
> + for (j = 0; j < pfx.advrtrs && p <= ep; j++) {
> struct in6_nbrinfo *nbi;
>
> - if (getnameinfo((struct sockaddr *)sin6,
> - sin6->sin6_len, namebuf, sizeof(namebuf),
> + memcpy(&sin6, p, sizeof(sin6));
> + p += sizeof(sin6);
> +
> + if (getnameinfo((struct sockaddr *)&sin6,
> + sin6.sin6_len, namebuf, sizeof(namebuf),
> NULL, 0, ninflags) != 0)
> strlcpy(namebuf, "?", sizeof(namebuf));
> printf(" %s", namebuf);
>
> - nbi = getnbrinfo(&sin6->sin6_addr,
> - p->if_index, 0);
> + nbi = getnbrinfo(&sin6.sin6_addr,
> + pfx.if_index, 0);
> if (nbi) {
> switch (nbi->state) {
> case ND6_LLINFO_REACHABLE:
> @@ -1141,7 +1142,6 @@ plist(void)
> }
> } else
> printf(" (no neighbor state)\n");
> - sin6++;
> }
> } else
> printf(" No advertising router\n");
> --
> Christian "naddy" Weisgerber [email protected]
>
