| Message ID | 20190618093207.13436-3-ard.biesheuvel@linaro.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | net: fastopen: follow-up tweaks for SipHash switch | expand |
On Tue, Jun 18, 2019 at 11:32:07AM +0200, Ard Biesheuvel wrote: > Use an explicit little endian representation of the fastopen > cookie, so that the value no longer depends on the endianness > of the system. This fixes a theoretical issue only, since > fastopen keys are unlikely to be shared across load balancing > server farms that are mixed in endiannes, but it might pop up > in validation/selftests as well, so let's just settle on little > endian across the board. > > Note that this change only affects big endian systems. > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > include/linux/tcp.h | 2 +- > net/ipv4/tcp_fastopen.c | 16 ++++++++-------- > 2 files changed, 9 insertions(+), 9 deletions(-) > What about the TCP_FASTOPEN_KEY option for setsockopt and getsockopt? Those APIs treat the key as an array of bytes (let's say it's little endian), so doesn't it need to be converted to/from the CPU endianness of siphash_key_t? - Eric
On Tue, 18 Jun 2019 at 20:22, Eric Biggers <ebiggers@kernel.org> wrote: > > On Tue, Jun 18, 2019 at 11:32:07AM +0200, Ard Biesheuvel wrote: > > Use an explicit little endian representation of the fastopen > > cookie, so that the value no longer depends on the endianness > > of the system. This fixes a theoretical issue only, since > > fastopen keys are unlikely to be shared across load balancing > > server farms that are mixed in endiannes, but it might pop up > > in validation/selftests as well, so let's just settle on little > > endian across the board. > > > > Note that this change only affects big endian systems. > > > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > --- > > include/linux/tcp.h | 2 +- > > net/ipv4/tcp_fastopen.c | 16 ++++++++-------- > > 2 files changed, 9 insertions(+), 9 deletions(-) > > > > What about the TCP_FASTOPEN_KEY option for setsockopt and getsockopt? Those > APIs treat the key as an array of bytes (let's say it's little endian), so > doesn't it need to be converted to/from the CPU endianness of siphash_key_t? > Yeah, good point. Can we first agree on whether we care about this or not? If so, i can spin a v2.
From: Ard Biesheuvel <ard.biesheuvel@linaro.org> Date: Tue, 18 Jun 2019 20:40:18 +0200 > Can we first agree on whether we care about this or not? If so, i > can spin a v2. Well, how can it possibly work otherwise in deployment scenerios involving both big and little endian hosts?
diff --git a/include/linux/tcp.h b/include/linux/tcp.h index 3d3659c638a6..f3a85a7fb4b1 100644 --- a/include/linux/tcp.h +++ b/include/linux/tcp.h @@ -58,7 +58,7 @@ static inline unsigned int tcp_optlen(const struct sk_buff *skb) /* TCP Fast Open Cookie as stored in memory */ struct tcp_fastopen_cookie { - u64 val[DIV_ROUND_UP(TCP_FASTOPEN_COOKIE_MAX, sizeof(u64))]; + __le64 val[DIV_ROUND_UP(TCP_FASTOPEN_COOKIE_MAX, sizeof(u64))]; s8 len; bool exp; /* In RFC6994 experimental option format */ }; diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c index 61c15c3d3584..2704441a0bb0 100644 --- a/net/ipv4/tcp_fastopen.c +++ b/net/ipv4/tcp_fastopen.c @@ -123,10 +123,10 @@ static bool __tcp_fastopen_cookie_gen_cipher(struct request_sock *req, if (req->rsk_ops->family == AF_INET) { const struct iphdr *iph = ip_hdr(syn); - foc->val[0] = siphash(&iph->saddr, - sizeof(iph->saddr) + - sizeof(iph->daddr), - key); + foc->val[0] = cpu_to_le64(siphash(&iph->saddr, + sizeof(iph->saddr) + + sizeof(iph->daddr), + key)); foc->len = TCP_FASTOPEN_COOKIE_SIZE; return true; } @@ -134,10 +134,10 @@ static bool __tcp_fastopen_cookie_gen_cipher(struct request_sock *req, if (req->rsk_ops->family == AF_INET6) { const struct ipv6hdr *ip6h = ipv6_hdr(syn); - foc->val[0] = siphash(&ip6h->saddr, - sizeof(ip6h->saddr) + - sizeof(ip6h->daddr), - key); + foc->val[0] = cpu_to_le64(siphash(&ip6h->saddr, + sizeof(ip6h->saddr) + + sizeof(ip6h->daddr), + key)); foc->len = TCP_FASTOPEN_COOKIE_SIZE; return true; }
Use an explicit little endian representation of the fastopen cookie, so that the value no longer depends on the endianness of the system. This fixes a theoretical issue only, since fastopen keys are unlikely to be shared across load balancing server farms that are mixed in endiannes, but it might pop up in validation/selftests as well, so let's just settle on little endian across the board. Note that this change only affects big endian systems. Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- include/linux/tcp.h | 2 +- net/ipv4/tcp_fastopen.c | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-)