Author: marius
Date: Sun May 20 05:12:31 2012
New Revision: 235681
URL: http://svn.freebsd.org/changeset/base/235681
Log:
Rewrite nd6_sysctl_{d,p}rlist() to avoid misaligned accesses to char arrays
casted to structs by getting rid of these buffers entirely. In r169832, it
was tried to paper over this issue by 32-bit aligning the buffers. Depending
on compiler optimizations that still was insufficient for 64-bit architectures
with strong alignment requirements though.
While at it, add comments regarding the total lack of locking in this area.
Tested by: bz
Reviewed by: bz (slightly earlier version), yongari (earlier version)
MFC after: 1 week
Modified:
head/sys/netinet6/nd6.c
Modified: head/sys/netinet6/nd6.c
==============================================================================
--- head/sys/netinet6/nd6.c Sun May 20 04:14:29 2012 (r235680)
+++ head/sys/netinet6/nd6.c Sun May 20 05:12:31 2012 (r235681)
@@ -2262,128 +2262,101 @@ SYSCTL_VNET_INT(_net_inet6_icmp6, ICMPV6
static int
nd6_sysctl_drlist(SYSCTL_HANDLER_ARGS)
{
- int error;
- char buf[1024] __aligned(4);
- struct in6_defrouter *d, *de;
+ struct in6_defrouter d;
struct nd_defrouter *dr;
+ int error;
if (req->newptr)
- return EPERM;
- error = 0;
+ return (EPERM);
+ bzero(&d, sizeof(d));
+ d.rtaddr.sin6_family = AF_INET6;
+ d.rtaddr.sin6_len = sizeof(d.rtaddr);
+
+ /*
+ * XXX locking
+ */
TAILQ_FOREACH(dr, &V_nd_defrouter, dr_entry) {
- d = (struct in6_defrouter *)buf;
- de = (struct in6_defrouter *)(buf + sizeof(buf));
-
- if (d + 1 <= de) {
- bzero(d, sizeof(*d));
- d->rtaddr.sin6_family = AF_INET6;
- d->rtaddr.sin6_len = sizeof(d->rtaddr);
- d->rtaddr.sin6_addr = dr->rtaddr;
- error = sa6_recoverscope(&d->rtaddr);
- if (error != 0)
- return (error);
- d->flags = dr->flags;
- d->rtlifetime = dr->rtlifetime;
- d->expire = dr->expire;
- d->if_index = dr->ifp->if_index;
- } else
- panic("buffer too short");
-
- error = SYSCTL_OUT(req, buf, sizeof(*d));
- if (error)
- break;
+ d.rtaddr.sin6_addr = dr->rtaddr;
+ error = sa6_recoverscope(&d.rtaddr);
+ if (error != 0)
+ return (error);
+ d.flags = dr->flags;
+ d.rtlifetime = dr->rtlifetime;
+ d.expire = dr->expire;
+ d.if_index = dr->ifp->if_index;
+ error = SYSCTL_OUT(req, &d, sizeof(d));
+ if (error != 0)
+ return (error);
}
-
- return (error);
+ return (0);
}
static int
nd6_sysctl_prlist(SYSCTL_HANDLER_ARGS)
{
- int error;
- char buf[1024] __aligned(4);
- struct in6_prefix *p, *pe;
+ struct in6_prefix p;
+ struct sockaddr_in6 s6;
struct nd_prefix *pr;
+ struct nd_pfxrouter *pfr;
+ time_t maxexpire;
+ int error;
char ip6buf[INET6_ADDRSTRLEN];
if (req->newptr)
- return EPERM;
- error = 0;
+ return (EPERM);
+ bzero(&p, sizeof(p));
+ p.origin = PR_ORIG_RA;
+ bzero(&s6, sizeof(s6));
+ s6.sin6_family = AF_INET6;
+ s6.sin6_len = sizeof(s6);
+
+ /*
+ * XXX locking
+ */
LIST_FOREACH(pr, &V_nd_prefix, ndpr_entry) {
- u_short advrtrs;
- size_t advance;
- struct sockaddr_in6 *sin6, *s6;
- struct nd_pfxrouter *pfr;
-
- p = (struct in6_prefix *)buf;
- pe = (struct in6_prefix *)(buf + sizeof(buf));
-
- if (p + 1 <= pe) {
- bzero(p, sizeof(*p));
- sin6 = (struct sockaddr_in6 *)(p + 1);
-
- p->prefix = pr->ndpr_prefix;
- if (sa6_recoverscope(&p->prefix)) {
+ p.prefix = pr->ndpr_prefix;
+ if (sa6_recoverscope(&p.prefix)) {
+ log(LOG_ERR, "scope error in prefix list (%s)\n",
+ ip6_sprintf(ip6buf, &p.prefix.sin6_addr));
+ /* XXX: press on... */
+ }
+ 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;
+ if (pr->ndpr_vltime == ND6_INFINITE_LIFETIME)
+ p.expire = 0;
+ else {
+ /* XXX: we assume time_t is signed. */
+ maxexpire = (-1) &
+ ~((time_t)1 << ((sizeof(maxexpire) * 8) - 1));
+ if (pr->ndpr_vltime < maxexpire - pr->ndpr_lastupdate)
+ p.expire = pr->ndpr_lastupdate +
+ pr->ndpr_vltime;
+ else
+ p.expire = maxexpire;
+ }
+ p.refcnt = pr->ndpr_refcnt;
+ p.flags = pr->ndpr_stateflags;
+ p.advrtrs = 0;
+ LIST_FOREACH(pfr, &pr->ndpr_advrtrs, pfr_entry)
+ p.advrtrs++;
+ error = SYSCTL_OUT(req, &p, sizeof(p));
+ if (error != 0)
+ return (error);
+ LIST_FOREACH(pfr, &pr->ndpr_advrtrs, pfr_entry) {
+ s6.sin6_addr = pfr->router->rtaddr;
+ if (sa6_recoverscope(&s6))
log(LOG_ERR,
"scope error in prefix list (%s)\n",
- ip6_sprintf(ip6buf, &p->prefix.sin6_addr));
- /* XXX: press on... */
- }
- 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;
- if (pr->ndpr_vltime == ND6_INFINITE_LIFETIME)
- p->expire = 0;
- else {
- time_t maxexpire;
-
- /* XXX: we assume time_t is signed. */
- maxexpire = (-1) &
- ~((time_t)1 <<
- ((sizeof(maxexpire) * 8) - 1));
- if (pr->ndpr_vltime <
- maxexpire - pr->ndpr_lastupdate) {
- p->expire = pr->ndpr_lastupdate +
- pr->ndpr_vltime;
- } else
- p->expire = maxexpire;
- }
- p->refcnt = pr->ndpr_refcnt;
- p->flags = pr->ndpr_stateflags;
- p->origin = PR_ORIG_RA;
- advrtrs = 0;
- LIST_FOREACH(pfr, &pr->ndpr_advrtrs, pfr_entry) {
- if ((void *)&sin6[advrtrs + 1] > (void *)pe) {
- advrtrs++;
- continue;
- }
- s6 = &sin6[advrtrs];
- bzero(s6, sizeof(*s6));
- s6->sin6_family = AF_INET6;
- s6->sin6_len = sizeof(*sin6);
- s6->sin6_addr = pfr->router->rtaddr;
- if (sa6_recoverscope(s6)) {
- log(LOG_ERR,
- "scope error in "
- "prefix list (%s)\n",
- ip6_sprintf(ip6buf,
- &pfr->router->rtaddr));
- }
- advrtrs++;
- }
- p->advrtrs = advrtrs;
- } else
- panic("buffer too short");
-
- advance = sizeof(*p) + sizeof(*sin6) * advrtrs;
- error = SYSCTL_OUT(req, buf, advance);
- if (error)
- break;
+ ip6_sprintf(ip6buf, &pfr->router->rtaddr));
+ error = SYSCTL_OUT(req, &s6, sizeof(s6));
+ if (error != 0)
+ return (error);
+ }
}
-
- return (error);
+ return (0);
}
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "[email protected]"
