Re: [PATCH v2 3/4] secure_seq: use siphash24 instead of md5_transform

On Wed, Dec 14, 2016 at 4:53 AM, Jason A. Donenfeld [off-list ref] wrote:

Hi David,

On Wed, Dec 14, 2016 at 10:51 AM, David Laight [off-list ref] wrote:

quoted

From: Jason A. Donenfeld

quoted

Sent: 14 December 2016 00:17
This gives a clear speed and security improvement. Rather than manually
filling MD5 buffers, we simply create a layout by a simple anonymous
struct, for which gcc generates rather efficient code.

...

quoted

+     const struct {
+             struct in6_addr saddr;
+             struct in6_addr daddr;
+             __be16 sport;
+             __be16 dport;
+     } __packed combined = {
+             .saddr = *(struct in6_addr *)saddr,
+             .daddr = *(struct in6_addr *)daddr,
+             .sport = sport,
+             .dport = dport
+     };

You need to look at the effect of marking this (and the other)
structures 'packed' on architectures like sparc64.

In all current uses of __packed in the code, I think the impact is
precisely zero, because all structures have members in descending
order of size, with each member being a perfect multiple of the one
below it. The __packed is therefore just there for safety, in case
somebody comes in and screws everything up by sticking a u8 in
between. In that case, it wouldn't be desirable to hash the structure
padding bits. In the worst case, I don't believe the impact would be
worse than a byte-by-byte memcpy, which is what the old code did. But
anyway, these structures are already naturally packed anyway, so the
present impact is nil.

If you pad the data structure to 64 bits then we can call the version
of siphash that only deals in 64 bit words. Writing a zero in the
padding will be cheaper than dealing with odd lengths in siphash24.

Tom

Jason