From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933884AbcLPKAK convert rfc822-to-8bit (ORCPT ); Fri, 16 Dec 2016 05:00:10 -0500 Received: from smtp-out4.electric.net ([192.162.216.191]:62068 "EHLO smtp-out4.electric.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932253AbcLPJ7r (ORCPT ); Fri, 16 Dec 2016 04:59:47 -0500 From: David Laight To: "'Jason A. Donenfeld'" , Netdev , "kernel-hardening@lists.openwall.com" , LKML , "linux-crypto@vger.kernel.org" , Ted Tso , Hannes Frederic Sowa , Linus Torvalds , Eric Biggers , Tom Herbert , "George Spelvin" , Vegard Nossum , "ak@linux.intel.com" , "davem@davemloft.net" , "luto@amacapital.net" Subject: RE: [PATCH v5 3/4] secure_seq: use SipHash in place of MD5 Thread-Topic: [PATCH v5 3/4] secure_seq: use SipHash in place of MD5 Thread-Index: AQHSVxIZA+ZpGUB4K0iQG5qqvkHZUaEKVSRA Date: Fri, 16 Dec 2016 09:59:32 +0000 Message-ID: <063D6719AE5E284EB5DD2968C1650D6DB0240E66@AcuExch.aculab.com> References: <20161215203003.31989-1-Jason@zx2c4.com> <20161215203003.31989-4-Jason@zx2c4.com> In-Reply-To: <20161215203003.31989-4-Jason@zx2c4.com> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.202.99.200] Content-Type: text/plain; charset="Windows-1252" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 X-Outbound-IP: 213.249.233.130 X-Env-From: David.Laight@ACULAB.COM X-Proto: esmtps X-Revdns: X-HELO: AcuExch.aculab.com X-TLS: TLSv1:AES128-SHA:128 X-Authenticated_ID: X-PolicySMART: 3396946, 3397078 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Jason A. Donenfeld > Sent: 15 December 2016 20:30 > This gives a clear speed and security improvement. Siphash is both > faster and is more solid crypto than the aging MD5. > > Rather than manually filling MD5 buffers, for IPv6, we simply create > a layout by a simple anonymous struct, for which gcc generates > rather efficient code. For IPv4, we pass the values directly to the > short input convenience functions. ... > diff --git a/net/core/secure_seq.c b/net/core/secure_seq.c > index 88a8e429fc3e..c80583bf3213 100644 ... > + const struct { > + struct in6_addr saddr; > + struct in6_addr daddr; > + __be16 sport; > + __be16 dport; > + u32 padding; > + } __aligned(SIPHASH_ALIGNMENT) combined = { > + .saddr = *(struct in6_addr *)saddr, > + .daddr = *(struct in6_addr *)daddr, > + .sport = sport, > + .dport = dport > + }; I think you should explicitly initialise the 'padding'. It can do no harm and makes it obvious that it is necessary. You are still putting over-aligned data on stack. You only need to align it to the alignment of u64 (not the size of u64). If an on-stack item has a stronger alignment requirement than the stack the gcc has to generate two stack frames for the function. If you assign to each field (instead of using initialisers) then you can get the alignment by making the first member an anonymous union of in6_addr and u64. Oh - and wait a bit longer between revisions. David