Messages in this thread Patch in this message | | | Subject | Re: [PATCH] tcp: fix tcp_fastopen unaligned access complaints on sparc | From | Eric Dumazet <> | Date | Thu, 12 Jan 2017 13:36:30 -0800 |
| |
On Thu, 2017-01-12 at 16:18 -0500, David Miller wrote: > From: Shannon Nelson <shannon.nelson@oracle.com> > Date: Thu, 12 Jan 2017 12:56:08 -0800 > > > > > > > On 1/12/2017 12:41 PM, David Miller wrote: > >> From: Shannon Nelson <shannon.nelson@oracle.com> > >> Date: Thu, 12 Jan 2017 12:30:38 -0800 > >> > >>> On 1/12/2017 12:25 PM, Eric Dumazet wrote: > >>>> On Thu, 2017-01-12 at 13:15 -0700, Rob Gardner wrote: > >>>> > >>>>> > >>>>> I suspect that someplace, somebody is casting val to an int * or > >>>>> something like that. > >>>> > >>>> Then that would be the bug. Can we root cause this please ? > >>>> > >>>> > >>> > >>> Look in net/ipv4/tcp_fastopen.c:tcp_fastopen_cookie_gen() for the line > >>> > >>> struct in6_addr *buf = (struct in6_addr *) tmp.val; > >> > >> Oh yeah, that's it. I didn't notice that at all. > >> > > > > It looked to me like swapping the data fields would be the easiest and > > least impactive way to fix this. I didn't want to mess with the > > logic. I'm certainly open to other suggestions. > > Given the nature of the problem, your fix is probably fine. > > Eric, any objections?
I am still objecting to this fix.
gcc makes no provision for aligning an variable that has alignof() = 1
We had such issues in the past.
We need the proper annotation on ->val field itself, to get proper alignment.
Then moving around the other field is a matter of avoiding a hole.
val should be an union, so that proper alignment is enforced by one member.
diff --git a/include/linux/tcp.h b/include/linux/tcp.h index fc5848dad7a43216b3f124c4afdaa6b64b23910c..5b790abf4c16313c9110996683be3a7fb368b66f 100644 --- a/include/linux/tcp.h +++ b/include/linux/tcp.h @@ -62,8 +62,13 @@ static inline unsigned int tcp_optlen(const struct sk_buff *skb) /* TCP Fast Open Cookie as stored in memory */ struct tcp_fastopen_cookie { + union { + u8 val[TCP_FASTOPEN_COOKIE_MAX]; +#if IS_ENABLED(CONFIG_IPV6) + struct in6_addr addr; +#endif + }; s8 len; - u8 val[TCP_FASTOPEN_COOKIE_MAX]; bool exp; /* In RFC6994 experimental option format */ };
| |