* Re: [kernel-hardening] Re: [PATCH v2 1/4] siphash: add cryptographically secure hashtable function
From: Jason A. Donenfeld @ 2016-12-15 21:11 UTC (permalink / raw)
To: kernel-hardening
Cc: Hannes Frederic Sowa, David Laight, Netdev,
Jean-Philippe Aumasson, LKML, Linux Crypto Mailing List,
Daniel J . Bernstein, Linus Torvalds, Eric Biggers
In-Reply-To: <20161215210933.GY3207@twins.programming.kicks-ass.net>
On Thu, Dec 15, 2016 at 10:09 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Thu, Dec 15, 2016 at 07:50:36PM +0100, Jason A. Donenfeld wrote:
>> There's no 32-bit platform
>> that will trap on a 64-bit unaligned access because there's no such
>> thing as a 64-bit access there. In short, we're fine.
>
> ARMv7 LPAE is a 32bit architecture that has 64bit load/stores IIRC.
>
> x86 has cmpxchg8b that can do 64bit things and very much wants the u64
> aligned.
>
> Also, IIRC we have a few platforms where u64 doesn't carry 8 byte
> alignment, m68k or something like that, but yes, you likely don't care.
Indeed, I stand corrected. But in any case, the use of __aligned(8) in
the patchset ensures that things are fixed and that we don't have this
issue.
^ permalink raw reply
* Re: [PATCH v2 1/4] siphash: add cryptographically secure hashtable function
From: Peter Zijlstra @ 2016-12-15 21:09 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: Hannes Frederic Sowa, David Laight, Netdev,
kernel-hardening@lists.openwall.com, Jean-Philippe Aumasson, LKML,
Linux Crypto Mailing List, Daniel J . Bernstein, Linus Torvalds,
Eric Biggers
In-Reply-To: <CAHmME9pTLFu3-4n6m_OMj5jVWGE-+yC-4CnkynD--H4Nt8_cpA@mail.gmail.com>
On Thu, Dec 15, 2016 at 07:50:36PM +0100, Jason A. Donenfeld wrote:
> There's no 32-bit platform
> that will trap on a 64-bit unaligned access because there's no such
> thing as a 64-bit access there. In short, we're fine.
ARMv7 LPAE is a 32bit architecture that has 64bit load/stores IIRC.
x86 has cmpxchg8b that can do 64bit things and very much wants the u64
aligned.
Also, IIRC we have a few platforms where u64 doesn't carry 8 byte
alignment, m68k or something like that, but yes, you likely don't care.
Just to make life interesting...
^ permalink raw reply
* Re: [PATCH v2 1/4] siphash: add cryptographically secure hashtable function
From: Hannes Frederic Sowa @ 2016-12-15 21:09 UTC (permalink / raw)
To: Peter Zijlstra, Jason A. Donenfeld
Cc: David Laight, Netdev, kernel-hardening@lists.openwall.com,
Jean-Philippe Aumasson, LKML, Linux Crypto Mailing List,
Daniel J . Bernstein, Linus Torvalds, Eric Biggers
In-Reply-To: <20161215210425.GX3207@twins.programming.kicks-ass.net>
On 15.12.2016 22:04, Peter Zijlstra wrote:
> On Thu, Dec 15, 2016 at 09:43:04PM +0100, Jason A. Donenfeld wrote:
>> On Thu, Dec 15, 2016 at 9:31 PM, Hannes Frederic Sowa
>> <hannes@stressinduktion.org> wrote:
>>> ARM64 and x86-64 have memory operations that are not vector operations
>>> that operate on 128 bit memory.
>>
>> Fair enough. imull I guess.
>
> imull is into rdx:rax, not memory. I suspect he's talking about
> cmpxchg16b.
Exactly and I think I saw a ll/sc 128 bit on armv8 to atomically
manipulate linked lists.
Bye,
Hannes
^ permalink raw reply
* Re: [PATCH v2 1/4] siphash: add cryptographically secure hashtable function
From: Peter Zijlstra @ 2016-12-15 21:04 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: Hannes Frederic Sowa, David Laight, Netdev,
kernel-hardening@lists.openwall.com, Jean-Philippe Aumasson, LKML,
Linux Crypto Mailing List, Daniel J . Bernstein, Linus Torvalds,
Eric Biggers
In-Reply-To: <CAHmME9rDCb=2rojJba13Uew9V9qAbxv1qcJGHwEAKoahxyE9QA@mail.gmail.com>
On Thu, Dec 15, 2016 at 09:43:04PM +0100, Jason A. Donenfeld wrote:
> On Thu, Dec 15, 2016 at 9:31 PM, Hannes Frederic Sowa
> <hannes@stressinduktion.org> wrote:
> > ARM64 and x86-64 have memory operations that are not vector operations
> > that operate on 128 bit memory.
>
> Fair enough. imull I guess.
imull is into rdx:rax, not memory. I suspect he's talking about
cmpxchg16b.
^ permalink raw reply
* Re: [PATCH v2 1/4] siphash: add cryptographically secure hashtable function
From: Jason A. Donenfeld @ 2016-12-15 20:43 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: David Laight, Netdev, kernel-hardening@lists.openwall.com,
Jean-Philippe Aumasson, LKML, Linux Crypto Mailing List,
Daniel J . Bernstein, Linus Torvalds, Eric Biggers
In-Reply-To: <18d1e9d1-1e52-b9a6-de26-2f33859ec052@stressinduktion.org>
On Thu, Dec 15, 2016 at 9:31 PM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
> ARM64 and x86-64 have memory operations that are not vector operations
> that operate on 128 bit memory.
Fair enough. imull I guess.
> How do you know that the compiler for some architecture will not chose a
> more optimized instruction to load a 64 bit memory value into two 32 bit
> registers if you tell the compiler it is 8 byte aligned but it actually
> isn't? I don't know the answer but telling the compiler some data is 8
> byte aligned while it isn't really pretty much seems like a call for
> trouble.
If a compiler is in the business of using special 64-bit instructions
on 64-bit aligned data, then it is also the job of the compiler to
align structs to 64-bits when passed __aligned(8), which is what we've
done in this code. If the compiler were to hypothetically choose to
ignore that and internally convert it to a __aligned(4), then it would
only be able to do so with the knowledge that it will never use 64-bit
aligned data instructions. But so far as I can tell, gcc always
respects __aligned(8), which is why I use it in this patchset.
I think there might have been confusion here, because perhaps someone
was hoping that since in6_addr is 128-bits, that the __aligned
attribute would not be required and that the struct would just
automatically be aligned to at least 8 bytes. But in fact, as I
mentioned, in6_addr is actually composed of u32[4] and not u64[2], so
it will only be aligned to 4 bytes, making the __aligned(8) necessary.
I think for the purposes of this patchset, this is a solved problem.
There's the unaligned version of the function if you don't know about
the data, and there's the aligned version if you're using
__aligned(SIPHASH_ALIGNMENT) on your data. Plain and simple.
Jason
^ permalink raw reply
* Re: [PATCH v2 1/4] siphash: add cryptographically secure hashtable function
From: Hannes Frederic Sowa @ 2016-12-15 20:31 UTC (permalink / raw)
To: Jason A. Donenfeld, David Laight
Cc: Netdev, kernel-hardening@lists.openwall.com,
Jean-Philippe Aumasson, LKML, Linux Crypto Mailing List,
Daniel J . Bernstein, Linus Torvalds, Eric Biggers
In-Reply-To: <CAHmME9pTLFu3-4n6m_OMj5jVWGE-+yC-4CnkynD--H4Nt8_cpA@mail.gmail.com>
Hello,
On 15.12.2016 19:50, Jason A. Donenfeld wrote:
> Hi David & Hannes,
>
> This conversation is veering off course.
Why?
> I think this doesn't really
> matter at all. Gcc converts u64 into essentially a pair of u32 on
> 32-bit platforms, so the alignment requirements for 32-bit is at a
> maximum 32 bits. On 64-bit platforms the alignment requirements are
> related at a maximum to the biggest register size, so 64-bit
> alignment. For this reason, no matter the behavior of __aligned(8),
> we're okay. Likewise, even without __aligned(8), if gcc aligns structs
> by their biggest member, then we get 4 byte alignment on 32-bit and 8
> byte alignment on 64-bit, which is fine. There's no 32-bit platform
> that will trap on a 64-bit unaligned access because there's no such
> thing as a 64-bit access there. In short, we're fine.
ARM64 and x86-64 have memory operations that are not vector operations
that operate on 128 bit memory.
How do you know that the compiler for some architecture will not chose a
more optimized instruction to load a 64 bit memory value into two 32 bit
registers if you tell the compiler it is 8 byte aligned but it actually
isn't? I don't know the answer but telling the compiler some data is 8
byte aligned while it isn't really pretty much seems like a call for
trouble.
Why can't a compiler not vectorize this code if it can prove that it
doesn't conflict with other register users?
Bye,
Hannes
^ permalink raw reply
* [PATCH v5 4/4] random: use SipHash in place of MD5
From: Jason A. Donenfeld @ 2016-12-15 20:30 UTC (permalink / raw)
To: Netdev, kernel-hardening, LKML, linux-crypto, David Laight,
Ted Tso, Hannes Frederic Sowa, Linus Torvalds, Eric Biggers,
Tom Herbert, George Spelvin, Vegard Nossum, ak, davem, luto
Cc: Jason A. Donenfeld, Jean-Philippe Aumasson
In-Reply-To: <20161215203003.31989-1-Jason@zx2c4.com>
This duplicates the current algorithm for get_random_int/long, but uses
siphash instead. This comes with several benefits. It's certainly
faster and more cryptographically secure than MD5. This patch also
separates hashed fields into three values instead of one, in order to
increase diffusion.
The previous MD5 algorithm used a per-cpu MD5 state, which caused
successive calls to the function to chain upon each other. While it's
not entirely clear that this kind of chaining is absolutely necessary
when using a secure PRF like siphash, it can't hurt, and the timing of
the call chain does add a degree of natural entropy. So, in keeping with
this design, instead of the massive per-cpu 64-byte MD5 state, there is
instead a per-cpu previously returned value for chaining.
The speed benefits are substantial:
| siphash | md5 | speedup |
------------------------------
get_random_long | 137130 | 415983 | 3.03x |
get_random_int | 86384 | 343323 | 3.97x |
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Jean-Philippe Aumasson <jeanphilippe.aumasson@gmail.com>
Cc: Ted Tso <tytso@mit.edu>
---
drivers/char/random.c | 32 +++++++++++++-------------------
1 file changed, 13 insertions(+), 19 deletions(-)
diff --git a/drivers/char/random.c b/drivers/char/random.c
index d6876d506220..a51f0ff43f00 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -262,6 +262,7 @@
#include <linux/syscalls.h>
#include <linux/completion.h>
#include <linux/uuid.h>
+#include <linux/siphash.h>
#include <crypto/chacha20.h>
#include <asm/processor.h>
@@ -2042,7 +2043,7 @@ struct ctl_table random_table[] = {
};
#endif /* CONFIG_SYSCTL */
-static u32 random_int_secret[MD5_MESSAGE_BYTES / 4] ____cacheline_aligned;
+static siphash_key_t random_int_secret;
int random_int_secret_init(void)
{
@@ -2050,8 +2051,7 @@ int random_int_secret_init(void)
return 0;
}
-static DEFINE_PER_CPU(__u32 [MD5_DIGEST_WORDS], get_random_int_hash)
- __aligned(sizeof(unsigned long));
+static DEFINE_PER_CPU(u64, get_random_int_chaining);
/*
* Get a random word for internal kernel use only. Similar to urandom but
@@ -2061,19 +2061,16 @@ static DEFINE_PER_CPU(__u32 [MD5_DIGEST_WORDS], get_random_int_hash)
*/
unsigned int get_random_int(void)
{
- __u32 *hash;
unsigned int ret;
+ u64 *chaining;
if (arch_get_random_int(&ret))
return ret;
- hash = get_cpu_var(get_random_int_hash);
-
- hash[0] += current->pid + jiffies + random_get_entropy();
- md5_transform(hash, random_int_secret);
- ret = hash[0];
- put_cpu_var(get_random_int_hash);
-
+ chaining = &get_cpu_var(get_random_int_chaining);
+ ret = *chaining = siphash_3u64(*chaining, jiffies, random_get_entropy() +
+ current->pid, random_int_secret);
+ put_cpu_var(get_random_int_chaining);
return ret;
}
EXPORT_SYMBOL(get_random_int);
@@ -2083,19 +2080,16 @@ EXPORT_SYMBOL(get_random_int);
*/
unsigned long get_random_long(void)
{
- __u32 *hash;
unsigned long ret;
+ u64 *chaining;
if (arch_get_random_long(&ret))
return ret;
- hash = get_cpu_var(get_random_int_hash);
-
- hash[0] += current->pid + jiffies + random_get_entropy();
- md5_transform(hash, random_int_secret);
- ret = *(unsigned long *)hash;
- put_cpu_var(get_random_int_hash);
-
+ chaining = &get_cpu_var(get_random_int_chaining);
+ ret = *chaining = siphash_3u64(*chaining, jiffies, random_get_entropy() +
+ current->pid, random_int_secret);
+ put_cpu_var(get_random_int_chaining);
return ret;
}
EXPORT_SYMBOL(get_random_long);
--
2.11.0
^ permalink raw reply related
* [PATCH v5 3/4] secure_seq: use SipHash in place of MD5
From: Jason A. Donenfeld @ 2016-12-15 20:30 UTC (permalink / raw)
To: Netdev, kernel-hardening, LKML, linux-crypto, David Laight,
Ted Tso, Hannes Frederic Sowa, Linus Torvalds, Eric Biggers,
Tom Herbert, George Spelvin, Vegard Nossum, ak, davem, luto
Cc: Jason A. Donenfeld
In-Reply-To: <20161215203003.31989-1-Jason@zx2c4.com>
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.
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: David Miller <davem@davemloft.net>
Cc: David Laight <David.Laight@aculab.com>
Cc: Tom Herbert <tom@herbertland.com>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
net/core/secure_seq.c | 133 ++++++++++++++++++++------------------------------
1 file changed, 52 insertions(+), 81 deletions(-)
diff --git a/net/core/secure_seq.c b/net/core/secure_seq.c
index 88a8e429fc3e..c80583bf3213 100644
--- a/net/core/secure_seq.c
+++ b/net/core/secure_seq.c
@@ -1,3 +1,5 @@
+/* Copyright (C) 2016 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved. */
+
#include <linux/kernel.h>
#include <linux/init.h>
#include <linux/cryptohash.h>
@@ -8,14 +10,14 @@
#include <linux/ktime.h>
#include <linux/string.h>
#include <linux/net.h>
-
+#include <linux/siphash.h>
#include <net/secure_seq.h>
#if IS_ENABLED(CONFIG_IPV6) || IS_ENABLED(CONFIG_INET)
+#include <linux/in6.h>
#include <net/tcp.h>
-#define NET_SECRET_SIZE (MD5_MESSAGE_BYTES / 4)
-static u32 net_secret[NET_SECRET_SIZE] ____cacheline_aligned;
+static siphash_key_t net_secret;
static __always_inline void net_secret_init(void)
{
@@ -44,44 +46,42 @@ static u32 seq_scale(u32 seq)
u32 secure_tcpv6_sequence_number(const __be32 *saddr, const __be32 *daddr,
__be16 sport, __be16 dport, u32 *tsoff)
{
- u32 secret[MD5_MESSAGE_BYTES / 4];
- u32 hash[MD5_DIGEST_WORDS];
- u32 i;
-
+ 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
+ };
+ u64 hash;
net_secret_init();
- memcpy(hash, saddr, 16);
- for (i = 0; i < 4; i++)
- secret[i] = net_secret[i] + (__force u32)daddr[i];
- secret[4] = net_secret[4] +
- (((__force u16)sport << 16) + (__force u16)dport);
- for (i = 5; i < MD5_MESSAGE_BYTES / 4; i++)
- secret[i] = net_secret[i];
-
- md5_transform(hash, secret);
-
- *tsoff = sysctl_tcp_timestamps == 1 ? hash[1] : 0;
- return seq_scale(hash[0]);
+ hash = siphash(&combined, sizeof(combined), net_secret);
+ *tsoff = sysctl_tcp_timestamps == 1 ? (hash >> 32) : 0;
+ return seq_scale(hash);
}
EXPORT_SYMBOL(secure_tcpv6_sequence_number);
u32 secure_ipv6_port_ephemeral(const __be32 *saddr, const __be32 *daddr,
__be16 dport)
{
- u32 secret[MD5_MESSAGE_BYTES / 4];
- u32 hash[MD5_DIGEST_WORDS];
- u32 i;
-
+ const struct {
+ struct in6_addr saddr;
+ struct in6_addr daddr;
+ __be16 dport;
+ u16 padding1;
+ u32 padding2;
+ } __aligned(SIPHASH_ALIGNMENT) combined = {
+ .saddr = *(struct in6_addr *)saddr,
+ .daddr = *(struct in6_addr *)daddr,
+ .dport = dport
+ };
net_secret_init();
- memcpy(hash, saddr, 16);
- for (i = 0; i < 4; i++)
- secret[i] = net_secret[i] + (__force u32) daddr[i];
- secret[4] = net_secret[4] + (__force u32)dport;
- for (i = 5; i < MD5_MESSAGE_BYTES / 4; i++)
- secret[i] = net_secret[i];
-
- md5_transform(hash, secret);
-
- return hash[0];
+ return siphash(&combined, sizeof(combined), net_secret);
}
EXPORT_SYMBOL(secure_ipv6_port_ephemeral);
#endif
@@ -91,33 +91,17 @@ EXPORT_SYMBOL(secure_ipv6_port_ephemeral);
u32 secure_tcp_sequence_number(__be32 saddr, __be32 daddr,
__be16 sport, __be16 dport, u32 *tsoff)
{
- u32 hash[MD5_DIGEST_WORDS];
-
+ u64 hash;
net_secret_init();
- hash[0] = (__force u32)saddr;
- hash[1] = (__force u32)daddr;
- hash[2] = ((__force u16)sport << 16) + (__force u16)dport;
- hash[3] = net_secret[15];
-
- md5_transform(hash, net_secret);
-
- *tsoff = sysctl_tcp_timestamps == 1 ? hash[1] : 0;
- return seq_scale(hash[0]);
+ hash = siphash_4u32(saddr, daddr, sport, dport, net_secret);
+ *tsoff = sysctl_tcp_timestamps == 1 ? (hash >> 32) : 0;
+ return seq_scale(hash);
}
u32 secure_ipv4_port_ephemeral(__be32 saddr, __be32 daddr, __be16 dport)
{
- u32 hash[MD5_DIGEST_WORDS];
-
net_secret_init();
- hash[0] = (__force u32)saddr;
- hash[1] = (__force u32)daddr;
- hash[2] = (__force u32)dport ^ net_secret[14];
- hash[3] = net_secret[15];
-
- md5_transform(hash, net_secret);
-
- return hash[0];
+ return siphash_4u32(saddr, daddr, dport, 0, net_secret);
}
EXPORT_SYMBOL_GPL(secure_ipv4_port_ephemeral);
#endif
@@ -126,21 +110,11 @@ EXPORT_SYMBOL_GPL(secure_ipv4_port_ephemeral);
u64 secure_dccp_sequence_number(__be32 saddr, __be32 daddr,
__be16 sport, __be16 dport)
{
- u32 hash[MD5_DIGEST_WORDS];
u64 seq;
-
net_secret_init();
- hash[0] = (__force u32)saddr;
- hash[1] = (__force u32)daddr;
- hash[2] = ((__force u16)sport << 16) + (__force u16)dport;
- hash[3] = net_secret[15];
-
- md5_transform(hash, net_secret);
-
- seq = hash[0] | (((u64)hash[1]) << 32);
+ seq = siphash_4u32(saddr, daddr, sport, dport, net_secret);
seq += ktime_get_real_ns();
seq &= (1ull << 48) - 1;
-
return seq;
}
EXPORT_SYMBOL(secure_dccp_sequence_number);
@@ -149,26 +123,23 @@ EXPORT_SYMBOL(secure_dccp_sequence_number);
u64 secure_dccpv6_sequence_number(__be32 *saddr, __be32 *daddr,
__be16 sport, __be16 dport)
{
- u32 secret[MD5_MESSAGE_BYTES / 4];
- u32 hash[MD5_DIGEST_WORDS];
+ 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
+ };
u64 seq;
- u32 i;
-
net_secret_init();
- memcpy(hash, saddr, 16);
- for (i = 0; i < 4; i++)
- secret[i] = net_secret[i] + (__force u32)daddr[i];
- secret[4] = net_secret[4] +
- (((__force u16)sport << 16) + (__force u16)dport);
- for (i = 5; i < MD5_MESSAGE_BYTES / 4; i++)
- secret[i] = net_secret[i];
-
- md5_transform(hash, secret);
-
- seq = hash[0] | (((u64)hash[1]) << 32);
+ seq = siphash(&combined, sizeof(combined), net_secret);
seq += ktime_get_real_ns();
seq &= (1ull << 48) - 1;
-
return seq;
}
EXPORT_SYMBOL(secure_dccpv6_sequence_number);
--
2.11.0
^ permalink raw reply related
* [PATCH v5 2/4] siphash: add Nu{32,64} helpers
From: Jason A. Donenfeld @ 2016-12-15 20:30 UTC (permalink / raw)
To: Netdev, kernel-hardening, LKML, linux-crypto, David Laight,
Ted Tso, Hannes Frederic Sowa, Linus Torvalds, Eric Biggers,
Tom Herbert, George Spelvin, Vegard Nossum, ak, davem, luto
Cc: Jason A. Donenfeld
In-Reply-To: <20161215203003.31989-1-Jason@zx2c4.com>
These restore parity with the jhash interface by providing high
performance helpers for common input sizes.
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Tom Herbert <tom@herbertland.com>
---
include/linux/siphash.h | 33 ++++++++++
lib/siphash.c | 157 +++++++++++++++++++++++++++++++++++++-----------
lib/test_siphash.c | 18 ++++++
3 files changed, 172 insertions(+), 36 deletions(-)
diff --git a/include/linux/siphash.h b/include/linux/siphash.h
index 145cf5667078..6f5a08a0fc7e 100644
--- a/include/linux/siphash.h
+++ b/include/linux/siphash.h
@@ -29,4 +29,37 @@ static inline u64 siphash_unaligned(const void *data, size_t len,
u64 siphash_unaligned(const void *data, size_t len, const siphash_key_t key);
#endif
+u64 siphash_1u64(const u64 a, const siphash_key_t key);
+u64 siphash_2u64(const u64 a, const u64 b, const siphash_key_t key);
+u64 siphash_3u64(const u64 a, const u64 b, const u64 c,
+ const siphash_key_t key);
+u64 siphash_4u64(const u64 a, const u64 b, const u64 c, const u64 d,
+ const siphash_key_t key);
+
+static inline u64 siphash_2u32(const u32 a, const u32 b, const siphash_key_t key)
+{
+ return siphash_1u64((u64)b << 32 | a, key);
+}
+
+static inline u64 siphash_4u32(const u32 a, const u32 b, const u32 c, const u32 d,
+ const siphash_key_t key)
+{
+ return siphash_2u64((u64)b << 32 | a, (u64)d << 32 | c, key);
+}
+
+static inline u64 siphash_6u32(const u32 a, const u32 b, const u32 c, const u32 d,
+ const u32 e, const u32 f, const siphash_key_t key)
+{
+ return siphash_3u64((u64)b << 32 | a, (u64)d << 32 | c, (u64)f << 32 | e,
+ key);
+}
+
+static inline u64 siphash_8u32(const u32 a, const u32 b, const u32 c, const u32 d,
+ const u32 e, const u32 f, const u32 g, const u32 h,
+ const siphash_key_t key)
+{
+ return siphash_4u64((u64)b << 32 | a, (u64)d << 32 | c, (u64)f << 32 | e,
+ (u64)h << 32 | g, key);
+}
+
#endif /* _LINUX_SIPHASH_H */
diff --git a/lib/siphash.c b/lib/siphash.c
index afc13cbb1b78..970c083ab06a 100644
--- a/lib/siphash.c
+++ b/lib/siphash.c
@@ -25,6 +25,29 @@
v2 += v1; v1 = rol64(v1, 17); v1 ^= v2; v2 = rol64(v2, 32); \
} while(0)
+#define PREAMBLE(len) \
+ u64 v0 = 0x736f6d6570736575ULL; \
+ u64 v1 = 0x646f72616e646f6dULL; \
+ u64 v2 = 0x6c7967656e657261ULL; \
+ u64 v3 = 0x7465646279746573ULL; \
+ u64 b = ((u64)len) << 56; \
+ v3 ^= key[1]; \
+ v2 ^= key[0]; \
+ v1 ^= key[1]; \
+ v0 ^= key[0];
+
+#define POSTAMBLE \
+ v3 ^= b; \
+ SIPROUND; \
+ SIPROUND; \
+ v0 ^= b; \
+ v2 ^= 0xff; \
+ SIPROUND; \
+ SIPROUND; \
+ SIPROUND; \
+ SIPROUND; \
+ return (v0 ^ v1) ^ (v2 ^ v3);
+
/**
* siphash - compute 64-bit siphash PRF value
* @data: buffer to hash, must be aligned to SIPHASH_ALIGNMENT
@@ -33,18 +56,10 @@
*/
u64 siphash(const void *data, size_t len, const siphash_key_t key)
{
- u64 v0 = 0x736f6d6570736575ULL;
- u64 v1 = 0x646f72616e646f6dULL;
- u64 v2 = 0x6c7967656e657261ULL;
- u64 v3 = 0x7465646279746573ULL;
- u64 b = ((u64)len) << 56;
- u64 m;
const u8 *end = data + len - (len % sizeof(u64));
const u8 left = len & (sizeof(u64) - 1);
- v3 ^= key[1];
- v2 ^= key[0];
- v1 ^= key[1];
- v0 ^= key[0];
+ u64 m;
+ PREAMBLE(len)
for (; data != end; data += sizeof(u64)) {
m = le64_to_cpup(data);
v3 ^= m;
@@ -67,16 +82,7 @@ u64 siphash(const void *data, size_t len, const siphash_key_t key)
case 1: b |= end[0];
}
#endif
- v3 ^= b;
- SIPROUND;
- SIPROUND;
- v0 ^= b;
- v2 ^= 0xff;
- SIPROUND;
- SIPROUND;
- SIPROUND;
- SIPROUND;
- return (v0 ^ v1) ^ (v2 ^ v3);
+ POSTAMBLE
}
EXPORT_SYMBOL(siphash);
@@ -89,18 +95,10 @@ EXPORT_SYMBOL(siphash);
*/
u64 siphash_unaligned(const void *data, size_t len, const siphash_key_t key)
{
- u64 v0 = 0x736f6d6570736575ULL;
- u64 v1 = 0x646f72616e646f6dULL;
- u64 v2 = 0x6c7967656e657261ULL;
- u64 v3 = 0x7465646279746573ULL;
- u64 b = ((u64)len) << 56;
- u64 m;
const u8 *end = data + len - (len % sizeof(u64));
const u8 left = len & (sizeof(u64) - 1);
- v3 ^= key[1];
- v2 ^= key[0];
- v1 ^= key[1];
- v0 ^= key[0];
+ u64 m;
+ PREAMBLE(len)
for (; data != end; data += sizeof(u64)) {
m = get_unaligned_le64(data);
v3 ^= m;
@@ -123,16 +121,103 @@ u64 siphash_unaligned(const void *data, size_t len, const siphash_key_t key)
case 1: b |= bytes[0];
}
#endif
- v3 ^= b;
+ POSTAMBLE
+}
+EXPORT_SYMBOL(siphash_unaligned);
+#endif
+
+/**
+ * siphash_1u64 - compute 64-bit siphash PRF value of a u64
+ * @first: first u64
+ * @key: the siphash key
+ */
+u64 siphash_1u64(const u64 first, const siphash_key_t key)
+{
+ PREAMBLE(8)
+ v3 ^= first;
+ SIPROUND;
+ SIPROUND;
+ v0 ^= first;
+ POSTAMBLE
+}
+EXPORT_SYMBOL(siphash_1u64);
+
+/**
+ * siphash_2u64 - compute 64-bit siphash PRF value of 2 u64
+ * @first: first u64
+ * @second: second u64
+ * @key: the siphash key
+ */
+u64 siphash_2u64(const u64 first, const u64 second, const siphash_key_t key)
+{
+ PREAMBLE(16)
+ v3 ^= first;
SIPROUND;
SIPROUND;
- v0 ^= b;
- v2 ^= 0xff;
+ v0 ^= first;
+ v3 ^= second;
SIPROUND;
SIPROUND;
+ v0 ^= second;
+ POSTAMBLE
+}
+EXPORT_SYMBOL(siphash_2u64);
+
+/**
+ * siphash_3u64 - compute 64-bit siphash PRF value of 3 u64
+ * @first: first u64
+ * @second: second u64
+ * @third: third u64
+ * @key: the siphash key
+ */
+u64 siphash_3u64(const u64 first, const u64 second, const u64 third,
+ const siphash_key_t key)
+{
+ PREAMBLE(24)
+ v3 ^= first;
SIPROUND;
SIPROUND;
- return (v0 ^ v1) ^ (v2 ^ v3);
+ v0 ^= first;
+ v3 ^= second;
+ SIPROUND;
+ SIPROUND;
+ v0 ^= second;
+ v3 ^= third;
+ SIPROUND;
+ SIPROUND;
+ v0 ^= third;
+ POSTAMBLE
}
-EXPORT_SYMBOL(siphash_unaligned);
-#endif
+EXPORT_SYMBOL(siphash_3u64);
+
+/**
+ * siphash_4u64 - compute 64-bit siphash PRF value of 4 u64
+ * @first: first u64
+ * @second: second u64
+ * @third: third u64
+ * @forth: forth u64
+ * @key: the siphash key
+ */
+u64 siphash_4u64(const u64 first, const u64 second, const u64 third,
+ const u64 forth, const siphash_key_t key)
+{
+ PREAMBLE(32)
+ v3 ^= first;
+ SIPROUND;
+ SIPROUND;
+ v0 ^= first;
+ v3 ^= second;
+ SIPROUND;
+ SIPROUND;
+ v0 ^= second;
+ v3 ^= third;
+ SIPROUND;
+ SIPROUND;
+ v0 ^= third;
+ v3 ^= forth;
+ SIPROUND;
+ SIPROUND;
+ v0 ^= forth;
+ POSTAMBLE
+}
+EXPORT_SYMBOL(siphash_4u64);
diff --git a/lib/test_siphash.c b/lib/test_siphash.c
index 93549e4e22c5..1635189c171f 100644
--- a/lib/test_siphash.c
+++ b/lib/test_siphash.c
@@ -67,6 +67,24 @@ static int __init siphash_test_init(void)
ret = -EINVAL;
}
}
+ if (siphash_1u64(0x0706050403020100ULL, test_key) != test_vectors[8]) {
+ pr_info("self-test 1u64: FAIL\n");
+ ret = -EINVAL;
+ }
+ if (siphash_2u64(0x0706050403020100ULL, 0x0f0e0d0c0b0a0908ULL, test_key) != test_vectors[16]) {
+ pr_info("self-test 2u64: FAIL\n");
+ ret = -EINVAL;
+ }
+ if (siphash_3u64(0x0706050403020100ULL, 0x0f0e0d0c0b0a0908ULL,
+ 0x1716151413121110ULL, test_key) != test_vectors[24]) {
+ pr_info("self-test 3u64: FAIL\n");
+ ret = -EINVAL;
+ }
+ if (siphash_4u64(0x0706050403020100ULL, 0x0f0e0d0c0b0a0908ULL,
+ 0x1716151413121110ULL, 0x1f1e1d1c1b1a1918ULL, test_key) != test_vectors[32]) {
+ pr_info("self-test 4u64: FAIL\n");
+ ret = -EINVAL;
+ }
if (!ret)
pr_info("self-tests: pass\n");
return ret;
--
2.11.0
^ permalink raw reply related
* [PATCH v5 1/4] siphash: add cryptographically secure PRF
From: Jason A. Donenfeld @ 2016-12-15 20:30 UTC (permalink / raw)
To: Netdev, kernel-hardening, LKML, linux-crypto, David Laight,
Ted Tso, Hannes Frederic Sowa, Linus Torvalds, Eric Biggers,
Tom Herbert, George Spelvin, Vegard Nossum, ak, davem, luto
Cc: Jason A. Donenfeld, Jean-Philippe Aumasson, Daniel J . Bernstein
In-Reply-To: <20161215203003.31989-1-Jason@zx2c4.com>
SipHash is a 64-bit keyed hash function that is actually a
cryptographically secure PRF, like HMAC. Except SipHash is super fast,
and is meant to be used as a hashtable keyed lookup function, or as a
general PRF for short input use cases, such as sequence numbers or RNG
chaining.
For the first usage:
There are a variety of attacks known as "hashtable poisoning" in which an
attacker forms some data such that the hash of that data will be the
same, and then preceeds to fill up all entries of a hashbucket. This is
a realistic and well-known denial-of-service vector. Currently
hashtables use jhash, which is fast but not secure, and some kind of
rotating key scheme (or none at all, which isn't good). SipHash is meant
as a replacement for jhash in these cases.
There are a modicum of places in the kernel that are vulnerable to
hashtable poisoning attacks, either via userspace vectors or network
vectors, and there's not a reliable mechanism inside the kernel at the
moment to fix it. The first step toward fixing these issues is actually
getting a secure primitive into the kernel for developers to use. Then
we can, bit by bit, port things over to it as deemed appropriate.
While SipHash is extremely fast for a cryptographically secure function,
it is likely a tiny bit slower than the insecure jhash, and so replacements
will be evaluated on a case-by-case basis based on whether or not the
difference in speed is negligible and whether or not the current jhash usage
poses a real security risk.
For the second usage:
A few places in the kernel are using MD5 for creating secure sequence
numbers, port numbers, or fast random numbers. SipHash is a faster, more
fitting, and more secure replacement for MD5 in those situations.
Replacing MD5 with SipHash for these uses is obvious and straight-
forward, and so is submitted along with this patch series. There
shouldn't be much of a debate over its efficacy.
Dozens of languages are already using this internally for their hash
tables and PRFs. Some of the BSDs already use this in their kernels.
SipHash is a widely known high-speed solution to a widely known set of
problems, and it's time we catch-up.
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Jean-Philippe Aumasson <jeanphilippe.aumasson@gmail.com>
Cc: Daniel J. Bernstein <djb@cr.yp.to>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Eric Biggers <ebiggers3@gmail.com>
Cc: David Laight <David.Laight@aculab.com>
---
include/linux/siphash.h | 32 +++++++++++
lib/Kconfig.debug | 6 +--
lib/Makefile | 5 +-
lib/siphash.c | 138 ++++++++++++++++++++++++++++++++++++++++++++++++
lib/test_siphash.c | 83 +++++++++++++++++++++++++++++
5 files changed, 259 insertions(+), 5 deletions(-)
create mode 100644 include/linux/siphash.h
create mode 100644 lib/siphash.c
create mode 100644 lib/test_siphash.c
diff --git a/include/linux/siphash.h b/include/linux/siphash.h
new file mode 100644
index 000000000000..145cf5667078
--- /dev/null
+++ b/include/linux/siphash.h
@@ -0,0 +1,32 @@
+/* Copyright (C) 2016 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
+ *
+ * This file is provided under a dual BSD/GPLv2 license.
+ *
+ * SipHash: a fast short-input PRF
+ * https://131002.net/siphash/
+ *
+ * This implementation is specifically for SipHash2-4.
+ */
+
+#ifndef _LINUX_SIPHASH_H
+#define _LINUX_SIPHASH_H
+
+#include <linux/types.h>
+
+#define SIPHASH_ALIGNMENT 8
+
+typedef u64 siphash_key_t[2];
+
+u64 siphash(const void *data, size_t len, const siphash_key_t key);
+
+#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
+static inline u64 siphash_unaligned(const void *data, size_t len,
+ const siphash_key_t key)
+{
+ return siphash(data, len, key);
+}
+#else
+u64 siphash_unaligned(const void *data, size_t len, const siphash_key_t key);
+#endif
+
+#endif /* _LINUX_SIPHASH_H */
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 7446097f72bd..86254ea99b45 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1843,9 +1843,9 @@ config TEST_HASH
tristate "Perform selftest on hash functions"
default n
help
- Enable this option to test the kernel's integer (<linux/hash,h>)
- and string (<linux/stringhash.h>) hash functions on boot
- (or module load).
+ Enable this option to test the kernel's integer (<linux/hash.h>),
+ string (<linux/stringhash.h>), and siphash (<linux/siphash.h>)
+ hash functions on boot (or module load).
This is intended to help people writing architecture-specific
optimized versions. If unsure, say N.
diff --git a/lib/Makefile b/lib/Makefile
index 50144a3aeebd..71d398b04a74 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -22,7 +22,8 @@ lib-y := ctype.o string.o vsprintf.o cmdline.o \
sha1.o chacha20.o md5.o irq_regs.o argv_split.o \
flex_proportions.o ratelimit.o show_mem.o \
is_single_threaded.o plist.o decompress.o kobject_uevent.o \
- earlycpio.o seq_buf.o nmi_backtrace.o nodemask.o win_minmax.o
+ earlycpio.o seq_buf.o siphash.o \
+ nmi_backtrace.o nodemask.o win_minmax.o
lib-$(CONFIG_MMU) += ioremap.o
lib-$(CONFIG_SMP) += cpumask.o
@@ -44,7 +45,7 @@ obj-$(CONFIG_TEST_HEXDUMP) += test_hexdump.o
obj-y += kstrtox.o
obj-$(CONFIG_TEST_BPF) += test_bpf.o
obj-$(CONFIG_TEST_FIRMWARE) += test_firmware.o
-obj-$(CONFIG_TEST_HASH) += test_hash.o
+obj-$(CONFIG_TEST_HASH) += test_hash.o test_siphash.o
obj-$(CONFIG_TEST_KASAN) += test_kasan.o
obj-$(CONFIG_TEST_KSTRTOX) += test-kstrtox.o
obj-$(CONFIG_TEST_LKM) += test_module.o
diff --git a/lib/siphash.c b/lib/siphash.c
new file mode 100644
index 000000000000..afc13cbb1b78
--- /dev/null
+++ b/lib/siphash.c
@@ -0,0 +1,138 @@
+/* Copyright (C) 2016 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
+ *
+ * This file is provided under a dual BSD/GPLv2 license.
+ *
+ * SipHash: a fast short-input PRF
+ * https://131002.net/siphash/
+ *
+ * This implementation is specifically for SipHash2-4.
+ */
+
+#include <linux/siphash.h>
+#include <linux/kernel.h>
+#include <asm/unaligned.h>
+
+#if defined(CONFIG_DCACHE_WORD_ACCESS) && BITS_PER_LONG == 64
+#include <linux/dcache.h>
+#include <asm/word-at-a-time.h>
+#endif
+
+#define SIPROUND \
+ do { \
+ v0 += v1; v1 = rol64(v1, 13); v1 ^= v0; v0 = rol64(v0, 32); \
+ v2 += v3; v3 = rol64(v3, 16); v3 ^= v2; \
+ v0 += v3; v3 = rol64(v3, 21); v3 ^= v0; \
+ v2 += v1; v1 = rol64(v1, 17); v1 ^= v2; v2 = rol64(v2, 32); \
+ } while(0)
+
+/**
+ * siphash - compute 64-bit siphash PRF value
+ * @data: buffer to hash, must be aligned to SIPHASH_ALIGNMENT
+ * @size: size of @data
+ * @key: the siphash key
+ */
+u64 siphash(const void *data, size_t len, const siphash_key_t key)
+{
+ u64 v0 = 0x736f6d6570736575ULL;
+ u64 v1 = 0x646f72616e646f6dULL;
+ u64 v2 = 0x6c7967656e657261ULL;
+ u64 v3 = 0x7465646279746573ULL;
+ u64 b = ((u64)len) << 56;
+ u64 m;
+ const u8 *end = data + len - (len % sizeof(u64));
+ const u8 left = len & (sizeof(u64) - 1);
+ v3 ^= key[1];
+ v2 ^= key[0];
+ v1 ^= key[1];
+ v0 ^= key[0];
+ for (; data != end; data += sizeof(u64)) {
+ m = le64_to_cpup(data);
+ v3 ^= m;
+ SIPROUND;
+ SIPROUND;
+ v0 ^= m;
+ }
+#if defined(CONFIG_DCACHE_WORD_ACCESS) && BITS_PER_LONG == 64
+ if (left)
+ b |= le64_to_cpu((__force __le64)(load_unaligned_zeropad(data) &
+ bytemask_from_count(left)));
+#else
+ switch (left) {
+ case 7: b |= ((u64)end[6]) << 48;
+ case 6: b |= ((u64)end[5]) << 40;
+ case 5: b |= ((u64)end[4]) << 32;
+ case 4: b |= le32_to_cpup(data); break;
+ case 3: b |= ((u64)end[2]) << 16;
+ case 2: b |= le16_to_cpup(data); break;
+ case 1: b |= end[0];
+ }
+#endif
+ v3 ^= b;
+ SIPROUND;
+ SIPROUND;
+ v0 ^= b;
+ v2 ^= 0xff;
+ SIPROUND;
+ SIPROUND;
+ SIPROUND;
+ SIPROUND;
+ return (v0 ^ v1) ^ (v2 ^ v3);
+}
+EXPORT_SYMBOL(siphash);
+
+#ifndef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
+/**
+ * siphash - compute 64-bit siphash PRF value, without alignment requirements
+ * @data: buffer to hash
+ * @size: size of @data
+ * @key: the siphash key
+ */
+u64 siphash_unaligned(const void *data, size_t len, const siphash_key_t key)
+{
+ u64 v0 = 0x736f6d6570736575ULL;
+ u64 v1 = 0x646f72616e646f6dULL;
+ u64 v2 = 0x6c7967656e657261ULL;
+ u64 v3 = 0x7465646279746573ULL;
+ u64 b = ((u64)len) << 56;
+ u64 m;
+ const u8 *end = data + len - (len % sizeof(u64));
+ const u8 left = len & (sizeof(u64) - 1);
+ v3 ^= key[1];
+ v2 ^= key[0];
+ v1 ^= key[1];
+ v0 ^= key[0];
+ for (; data != end; data += sizeof(u64)) {
+ m = get_unaligned_le64(data);
+ v3 ^= m;
+ SIPROUND;
+ SIPROUND;
+ v0 ^= m;
+ }
+#if defined(CONFIG_DCACHE_WORD_ACCESS) && BITS_PER_LONG == 64
+ if (left)
+ b |= le64_to_cpu((__force __le64)(load_unaligned_zeropad(data) &
+ bytemask_from_count(left)));
+#else
+ switch (left) {
+ case 7: b |= ((u64)end[6]) << 48;
+ case 6: b |= ((u64)end[5]) << 40;
+ case 5: b |= ((u64)end[4]) << 32;
+ case 4: b |= get_unaligned_le32(end); break;
+ case 3: b |= ((u64)end[2]) << 16;
+ case 2: b |= get_unaligned_le16(end); break;
+ case 1: b |= bytes[0];
+ }
+#endif
+ v3 ^= b;
+ SIPROUND;
+ SIPROUND;
+ v0 ^= b;
+ v2 ^= 0xff;
+ SIPROUND;
+ SIPROUND;
+ SIPROUND;
+ SIPROUND;
+ return (v0 ^ v1) ^ (v2 ^ v3);
+}
+EXPORT_SYMBOL(siphash_unaligned);
+#endif
diff --git a/lib/test_siphash.c b/lib/test_siphash.c
new file mode 100644
index 000000000000..93549e4e22c5
--- /dev/null
+++ b/lib/test_siphash.c
@@ -0,0 +1,83 @@
+/* Test cases for siphash.c
+ *
+ * Copyright (C) 2016 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
+ *
+ * This file is provided under a dual BSD/GPLv2 license.
+ *
+ * SipHash: a fast short-input PRF
+ * https://131002.net/siphash/
+ *
+ * This implementation is specifically for SipHash2-4.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/siphash.h>
+#include <linux/kernel.h>
+#include <linux/string.h>
+#include <linux/errno.h>
+#include <linux/module.h>
+
+/* Test vectors taken from official reference source available at:
+ * https://131002.net/siphash/siphash24.c
+ */
+static const u64 test_vectors[64] = {
+ 0x726fdb47dd0e0e31ULL, 0x74f839c593dc67fdULL, 0x0d6c8009d9a94f5aULL,
+ 0x85676696d7fb7e2dULL, 0xcf2794e0277187b7ULL, 0x18765564cd99a68dULL,
+ 0xcbc9466e58fee3ceULL, 0xab0200f58b01d137ULL, 0x93f5f5799a932462ULL,
+ 0x9e0082df0ba9e4b0ULL, 0x7a5dbbc594ddb9f3ULL, 0xf4b32f46226bada7ULL,
+ 0x751e8fbc860ee5fbULL, 0x14ea5627c0843d90ULL, 0xf723ca908e7af2eeULL,
+ 0xa129ca6149be45e5ULL, 0x3f2acc7f57c29bdbULL, 0x699ae9f52cbe4794ULL,
+ 0x4bc1b3f0968dd39cULL, 0xbb6dc91da77961bdULL, 0xbed65cf21aa2ee98ULL,
+ 0xd0f2cbb02e3b67c7ULL, 0x93536795e3a33e88ULL, 0xa80c038ccd5ccec8ULL,
+ 0xb8ad50c6f649af94ULL, 0xbce192de8a85b8eaULL, 0x17d835b85bbb15f3ULL,
+ 0x2f2e6163076bcfadULL, 0xde4daaaca71dc9a5ULL, 0xa6a2506687956571ULL,
+ 0xad87a3535c49ef28ULL, 0x32d892fad841c342ULL, 0x7127512f72f27cceULL,
+ 0xa7f32346f95978e3ULL, 0x12e0b01abb051238ULL, 0x15e034d40fa197aeULL,
+ 0x314dffbe0815a3b4ULL, 0x027990f029623981ULL, 0xcadcd4e59ef40c4dULL,
+ 0x9abfd8766a33735cULL, 0x0e3ea96b5304a7d0ULL, 0xad0c42d6fc585992ULL,
+ 0x187306c89bc215a9ULL, 0xd4a60abcf3792b95ULL, 0xf935451de4f21df2ULL,
+ 0xa9538f0419755787ULL, 0xdb9acddff56ca510ULL, 0xd06c98cd5c0975ebULL,
+ 0xe612a3cb9ecba951ULL, 0xc766e62cfcadaf96ULL, 0xee64435a9752fe72ULL,
+ 0xa192d576b245165aULL, 0x0a8787bf8ecb74b2ULL, 0x81b3e73d20b49b6fULL,
+ 0x7fa8220ba3b2eceaULL, 0x245731c13ca42499ULL, 0xb78dbfaf3a8d83bdULL,
+ 0xea1ad565322a1a0bULL, 0x60e61c23a3795013ULL, 0x6606d7e446282b93ULL,
+ 0x6ca4ecb15c5f91e1ULL, 0x9f626da15c9625f3ULL, 0xe51b38608ef25f57ULL,
+ 0x958a324ceb064572ULL
+};
+static const siphash_key_t test_key =
+ { 0x0706050403020100ULL , 0x0f0e0d0c0b0a0908ULL };
+
+static int __init siphash_test_init(void)
+{
+ u8 in[64] __aligned(SIPHASH_ALIGNMENT);
+ u8 in_unaligned[65];
+ u8 i;
+ int ret = 0;
+
+ for (i = 0; i < 64; ++i) {
+ in[i] = i;
+ in_unaligned[i + 1] = i;
+ if (siphash(in, i, test_key) != test_vectors[i]) {
+ pr_info("self-test aligned %u: FAIL\n", i + 1);
+ ret = -EINVAL;
+ }
+ if (siphash_unaligned(in_unaligned + 1, i, test_key) != test_vectors[i]) {
+ pr_info("self-test unaligned %u: FAIL\n", i + 1);
+ ret = -EINVAL;
+ }
+ }
+ if (!ret)
+ pr_info("self-tests: pass\n");
+ return ret;
+}
+
+static void __exit siphash_test_exit(void)
+{
+}
+
+module_init(siphash_test_init);
+module_exit(siphash_test_exit);
+
+MODULE_AUTHOR("Jason A. Donenfeld <Jason@zx2c4.com>");
+MODULE_LICENSE("Dual BSD/GPL");
--
2.11.0
^ permalink raw reply related
* [PATCH v5 0/4] The SipHash Patchset
From: Jason A. Donenfeld @ 2016-12-15 20:29 UTC (permalink / raw)
To: Netdev, kernel-hardening, LKML, linux-crypto, David Laight,
Ted Tso, Hannes Frederic Sowa, Linus Torvalds, Eric Biggers,
Tom Herbert, George Spelvin, Vegard Nossum, ak, davem, luto
Cc: Jason A. Donenfeld
Hey folks,
I think we're approaching the end of the review for this patchset and we're
getting somewhat close to being ready for it being queued up. At this point,
I've incorporated all of the extremely helpful and instructive suggestions
from the list.
For this v5, we now accept u64[2] as the key, so that alignment is taken
care of naturally. For other alignment issues, we have both the fast aligned
version and the unaligned version, depending on what's necessary. We've
worked out the issues for struct padding. The functions now take a void
pointer to avoid ugly casting, which also helps us shed the inline helper
functions which were not very pretty. The replacements of MD5 have been
benchmarked and show a big increase in speed. We've even come up with a
better naming scheme for dword/qword. All and all it's shaping up nicely.
So, if this series looks good to you, please send along your Reviewed-by,
so we can begin to get this completed. If there are still lingering issues,
let me know and I'll incorporated them into a v6 if necessary.
Thanks,
Jason
Jason A. Donenfeld (4):
siphash: add cryptographically secure PRF
siphash: add Nu{32,64} helpers
secure_seq: use SipHash in place of MD5
random: use SipHash in place of MD5
drivers/char/random.c | 32 +++----
include/linux/siphash.h | 65 ++++++++++++++
lib/Kconfig.debug | 6 +-
lib/Makefile | 5 +-
lib/siphash.c | 223 ++++++++++++++++++++++++++++++++++++++++++++++++
lib/test_siphash.c | 101 ++++++++++++++++++++++
net/core/secure_seq.c | 133 +++++++++++------------------
7 files changed, 460 insertions(+), 105 deletions(-)
create mode 100644 include/linux/siphash.h
create mode 100644 lib/siphash.c
create mode 100644 lib/test_siphash.c
--
2.11.0
^ permalink raw reply
* Re: [PATCH v3 3/3] random: use siphash24 instead of md5 for get_random_int/long
From: Jason A. Donenfeld @ 2016-12-15 18:51 UTC (permalink / raw)
To: David Laight
Cc: Netdev, kernel-hardening@lists.openwall.com, LKML,
linux-crypto@vger.kernel.org, Jean-Philippe Aumasson, Ted Tso
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB02401A1@AcuExch.aculab.com>
Hi David,
On Thu, Dec 15, 2016 at 11:14 AM, David Laight <David.Laight@aculab.com> wrote:
> From: Behalf Of Jason A. Donenfeld
>> Sent: 14 December 2016 18:46
> ...
>> + ret = *chaining = siphash24((u8 *)&combined, offsetof(typeof(combined), end),
>
> If you make the first argument 'const void *' you won't need the cast
> on every call.
>
> I'd also suggest making the key u64[2].
I'll do both. Thanks for the suggestion.
Jason
^ permalink raw reply
* Re: [PATCH v2 1/4] siphash: add cryptographically secure hashtable function
From: Jason A. Donenfeld @ 2016-12-15 18:50 UTC (permalink / raw)
To: Hannes Frederic Sowa, David Laight
Cc: Netdev, kernel-hardening@lists.openwall.com,
Jean-Philippe Aumasson, LKML, Linux Crypto Mailing List,
Daniel J . Bernstein, Linus Torvalds, Eric Biggers
In-Reply-To: <924ef794-eae0-2a6b-508b-069718339edc@stressinduktion.org>
Hi David & Hannes,
This conversation is veering off course. I think this doesn't really
matter at all. Gcc converts u64 into essentially a pair of u32 on
32-bit platforms, so the alignment requirements for 32-bit is at a
maximum 32 bits. On 64-bit platforms the alignment requirements are
related at a maximum to the biggest register size, so 64-bit
alignment. For this reason, no matter the behavior of __aligned(8),
we're okay. Likewise, even without __aligned(8), if gcc aligns structs
by their biggest member, then we get 4 byte alignment on 32-bit and 8
byte alignment on 64-bit, which is fine. There's no 32-bit platform
that will trap on a 64-bit unaligned access because there's no such
thing as a 64-bit access there. In short, we're fine.
(The reason in6_addr aligns itself to 4 bytes on 64-bit platforms is
that it's defined as being u32 blah[4]. If we added a u64 blah[2],
we'd get 8 byte alignment, but that's not in the header. Feel free to
start a new thread about this issue if you feel this ought to be added
for whatever reason.)
One optimization that's been suggested on this list is that instead of
u8 key[16] and requiring the alignment attribute, I should just use
u64 key[2]. This seems reasonable to me, and it will also save the
endian conversion call. These keys generally aren't transmitted over a
network, so I don't think a byte-wise encoding is particularly
important.
The other suggestion I've seen is that I make the functions take a
const void * instead of a const u8 * for the data, in order to save
ugly casts. I'll do this too.
Meanwhile Linus has condemned our 4dwords/2qwords naming, and I'll
need to think of something different. The best I can think of right
now is siphash_4_u32/siphash_2_u64, but I don't find it especially
pretty. Open to suggestions.
Regards,
Jason
^ permalink raw reply
* Re: [Qemu-devel] [PATCH v7 1/1] crypto: add virtio-crypto driver
From: Michael S. Tsirkin @ 2016-12-15 16:42 UTC (permalink / raw)
To: Gonglei (Arei)
Cc: virtio-dev@lists.oasis-open.org, Huangweidong (C),
herbert@gondor.apana.org.au, Hanweidong (Randy),
qemu-devel@nongnu.org, Claudio Fontana,
linux-kernel@vger.kernel.org, Wanzongshun (Vincent),
virtualization@lists.linux-foundation.org, Xuquan (Quan Xu),
linux-crypto@vger.kernel.org, stefanha@redhat.com,
Zhoujian (jay, Euler), Luonengjun, longpeng, davem@davemloft.net,
Wubin (H)
In-Reply-To: <33183CC9F5247A488A2544077AF19020DA16320F@DGGEMA505-MBX.china.huawei.com>
On Thu, Dec 15, 2016 at 01:08:51AM +0000, Gonglei (Arei) wrote:
>
>
>
>
> Regards,
> -Gonglei
>
>
> > -----Original Message-----
> > From: Zeng, Xin [mailto:xin.zeng@intel.com]
> > Sent: Thursday, December 15, 2016 8:59 AM
> > To: Gonglei (Arei); Halil Pasic; linux-kernel@vger.kernel.org;
> > qemu-devel@nongnu.org; virtio-dev@lists.oasis-open.org;
> > virtualization@lists.linux-foundation.org; linux-crypto@vger.kernel.org
> > Cc: Huangweidong (C); Claudio Fontana; mst@redhat.com; Luonengjun;
> > Hanweidong (Randy); Xuquan (Quan Xu); Wanzongshun (Vincent);
> > stefanha@redhat.com; Zhoujian (jay, Euler); cornelia.huck@de.ibm.com;
> > longpeng; arei.gonglei@hotmail.com; davem@davemloft.net; Wubin (H);
> > herbert@gondor.apana.org.au
> > Subject: RE: [Qemu-devel] [PATCH v7 1/1] crypto: add virtio-crypto driver
> >
> > On Thursday, December 15, 2016 8:45 AM, Gonglei (Arei) Wrote:
> > < > > diff --git a/drivers/crypto/virtio/virtio_crypto_core.c
> > < > b/drivers/crypto/virtio/virtio_crypto_core.c
> > < > > new file mode 100644
> > < > > index 0000000..c0854a1
> > < > > --- /dev/null
> > < > > +++ b/drivers/crypto/virtio/virtio_crypto_core.c
> > < > > @@ -0,0 +1,474 @@
> > < > [..]
> > < > > +
> > < > > +static void virtcrypto_dataq_callback(struct virtqueue *vq)
> > < > > +{
> > < > > + struct virtio_crypto *vcrypto = vq->vdev->priv;
> > < > > + struct virtio_crypto_request *vc_req;
> > < > > + unsigned long flags;
> > < > > + unsigned int len;
> > < > > + struct ablkcipher_request *ablk_req;
> > < > > + int error;
> > < > > +
> > < > > + spin_lock_irqsave(&vcrypto->lock, flags);
> > < >
> > < > Would it make sense to use a per virtqueue lock
> > < > like in virtio_blk for example instead of locking on the whole
> > < > device? OK, it seems you use only one dataqueue, so it
> > < > may not be that relevant.
> > < >
> > < Currently yes, both the backend device (cryptodev-backend-builtin)
> > < and the frontend driver use one dataqueue.
> > <
> >
> > I think it makes sense to use per virtqueue lock here though it only uses one
> > queue so far,
> > but in the spec we already have multi queues support.
> >
> Yes, I agree. Will do that in V8 soon.
> Hope to catch up with Michael's pull request for 4.10.
>
> Regards,
> -Gonglei
I merged v7, this change will have to wait. Sorry.
^ permalink raw reply
* Crypto Fixes for 4.10
From: Herbert Xu @ 2016-12-15 16:07 UTC (permalink / raw)
To: Linus Torvalds, David S. Miller, Linux Kernel Mailing List,
Linux Crypto Mailing List
In-Reply-To: <20161119102748.GA4277@gondor.apana.org.au>
Hi Linus:
This push fixes the following issues:
- A crash regression in the new skcipher walker.
- Incorrect return value in public_key_verify_signature.
- Fix for in-place signing in the sign-file utility.
Please pull from
git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6.git linus
Alex Yashchenko (1):
sign-file: Fix inplace signing when src and dst names are both specified
Ard Biesheuvel (1):
crypto: skcipher - fix crash in virtual walk
Pan Bian (1):
crypto: asymmetric_keys - set error code on failure
crypto/asymmetric_keys/public_key.c | 1 +
crypto/skcipher.c | 4 +++-
scripts/sign-file.c | 2 +-
3 files changed, 5 insertions(+), 2 deletions(-)
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH v2 1/4] siphash: add cryptographically secure hashtable function
From: Hannes Frederic Sowa @ 2016-12-15 15:53 UTC (permalink / raw)
To: David Laight, Jason A. Donenfeld
Cc: Netdev, kernel-hardening@lists.openwall.com,
Jean-Philippe Aumasson, LKML, Linux Crypto Mailing List,
Daniel J . Bernstein, Linus Torvalds, Eric Biggers
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB0240529@AcuExch.aculab.com>
On 15.12.2016 16:41, David Laight wrote:
> Try (retyped):
>
> echo 'struct { long a; long long b; } s; int bar { return sizeof s; }' >foo.c
> gcc [-m32] -O2 -S foo.c; cat foo.s
>
> And look at what is generated.
I used __alignof__(unsigned long long) with -m32.
>> Right now ipv6 addresses have an alignment of 4. So we couldn't even
>> naturally pass them to siphash but would need to copy them around, which
>> I feel like a source of bugs.
>
> That is more of a problem on systems that don't support misaligned accesses.
> Reading the 64bit values with two explicit 32bit reads would work.
> I think you can get gcc to do that by adding an aligned(4) attribute to the
> structure member.
Yes, and that is actually my fear, because we support those
architectures. I can't comment on that as I don't understand enough of this.
If someone finds a way to cause misaligned reads on a small box this
seems (maybe depending on sysctls they get fixed up or panic) to be a
much bigger issue than having a hash DoS.
Thanks,
Hannes
^ permalink raw reply
* RE: [PATCH v2 1/4] siphash: add cryptographically secure hashtable function
From: David Laight @ 2016-12-15 15:41 UTC (permalink / raw)
To: 'Hannes Frederic Sowa', Jason A. Donenfeld
Cc: Netdev, kernel-hardening@lists.openwall.com,
Jean-Philippe Aumasson, LKML, Linux Crypto Mailing List,
Daniel J . Bernstein, Linus Torvalds, Eric Biggers
In-Reply-To: <f6813084-6f46-0b22-e9c3-97b2e266f8f6@stressinduktion.org>
From: Hannes Frederic Sowa
> Sent: 15 December 2016 14:57
> On 15.12.2016 14:56, David Laight wrote:
> > From: Hannes Frederic Sowa
> >> Sent: 15 December 2016 12:50
> >> On 15.12.2016 13:28, David Laight wrote:
> >>> From: Hannes Frederic Sowa
> >>>> Sent: 15 December 2016 12:23
> >>> ...
> >>>> Hmm? Even the Intel ABI expects alignment of unsigned long long to be 8
> >>>> bytes on 32 bit. Do you question that?
> >>>
> >>> Yes.
> >>>
> >>> The linux ABI for x86 (32 bit) only requires 32bit alignment for u64 (etc).
> >>
> >> Hmm, u64 on 32 bit is unsigned long long and not unsigned long. Thus I
> >> am actually not sure if the ABI would say anything about that (sorry
> >> also for my wrong statement above).
> >>
> >> Alignment requirement of unsigned long long on gcc with -m32 actually
> >> seem to be 8.
> >
> > It depends on the architecture.
> > For x86 it is definitely 4.
>
> May I ask for a reference?
Ask anyone who has had to do compatibility layers to support 32bit
binaries on 64bit systems.
> I couldn't see unsigned long long being
> mentioned in the ia32 abi spec that I found. I agree that those accesses
> might be synthetically assembled by gcc and for me the alignment of 4
> would have seemed natural. But my gcc at least in 32 bit mode disagrees
> with that.
Try (retyped):
echo 'struct { long a; long long b; } s; int bar { return sizeof s; }' >foo.c
gcc [-m32] -O2 -S foo.c; cat foo.s
And look at what is generated.
> Right now ipv6 addresses have an alignment of 4. So we couldn't even
> naturally pass them to siphash but would need to copy them around, which
> I feel like a source of bugs.
That is more of a problem on systems that don't support misaligned accesses.
Reading the 64bit values with two explicit 32bit reads would work.
I think you can get gcc to do that by adding an aligned(4) attribute to the
structure member.
David
^ permalink raw reply
* Re: [PATCH v2 1/4] siphash: add cryptographically secure hashtable function
From: Hannes Frederic Sowa @ 2016-12-15 14:56 UTC (permalink / raw)
To: David Laight, Jason A. Donenfeld
Cc: Netdev, kernel-hardening@lists.openwall.com,
Jean-Philippe Aumasson, LKML, Linux Crypto Mailing List,
Daniel J . Bernstein, Linus Torvalds, Eric Biggers
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB0240437@AcuExch.aculab.com>
On 15.12.2016 14:56, David Laight wrote:
> From: Hannes Frederic Sowa
>> Sent: 15 December 2016 12:50
>> On 15.12.2016 13:28, David Laight wrote:
>>> From: Hannes Frederic Sowa
>>>> Sent: 15 December 2016 12:23
>>> ...
>>>> Hmm? Even the Intel ABI expects alignment of unsigned long long to be 8
>>>> bytes on 32 bit. Do you question that?
>>>
>>> Yes.
>>>
>>> The linux ABI for x86 (32 bit) only requires 32bit alignment for u64 (etc).
>>
>> Hmm, u64 on 32 bit is unsigned long long and not unsigned long. Thus I
>> am actually not sure if the ABI would say anything about that (sorry
>> also for my wrong statement above).
>>
>> Alignment requirement of unsigned long long on gcc with -m32 actually
>> seem to be 8.
>
> It depends on the architecture.
> For x86 it is definitely 4.
May I ask for a reference? I couldn't see unsigned long long being
mentioned in the ia32 abi spec that I found. I agree that those accesses
might be synthetically assembled by gcc and for me the alignment of 4
would have seemed natural. But my gcc at least in 32 bit mode disagrees
with that.
> It might be 8 for sparc, ppc and/or alpha.
This is something to find out...
Right now ipv6 addresses have an alignment of 4. So we couldn't even
naturally pass them to siphash but would need to copy them around, which
I feel like a source of bugs.
Bye,
Hannes
^ permalink raw reply
* RE: [PATCH v2 1/4] siphash: add cryptographically secure hashtable function
From: David Laight @ 2016-12-15 13:56 UTC (permalink / raw)
To: 'Hannes Frederic Sowa', Jason A. Donenfeld
Cc: Netdev, kernel-hardening@lists.openwall.com,
Jean-Philippe Aumasson, LKML, Linux Crypto Mailing List,
Daniel J . Bernstein, Linus Torvalds, Eric Biggers
In-Reply-To: <0f3c3694-c00b-aae2-5b08-25bc64bf6372@stressinduktion.org>
From: Hannes Frederic Sowa
> Sent: 15 December 2016 12:50
> On 15.12.2016 13:28, David Laight wrote:
> > From: Hannes Frederic Sowa
> >> Sent: 15 December 2016 12:23
> > ...
> >> Hmm? Even the Intel ABI expects alignment of unsigned long long to be 8
> >> bytes on 32 bit. Do you question that?
> >
> > Yes.
> >
> > The linux ABI for x86 (32 bit) only requires 32bit alignment for u64 (etc).
>
> Hmm, u64 on 32 bit is unsigned long long and not unsigned long. Thus I
> am actually not sure if the ABI would say anything about that (sorry
> also for my wrong statement above).
>
> Alignment requirement of unsigned long long on gcc with -m32 actually
> seem to be 8.
It depends on the architecture.
For x86 it is definitely 4.
It might be 8 for sparc, ppc and/or alpha.
David
^ permalink raw reply
* [PATCH -resend with CC] crypto: algif_hash, avoid zero-sized array
From: Jiri Slaby @ 2016-12-15 13:31 UTC (permalink / raw)
To: herbert; +Cc: linux-crypto, linux-kernel, Jiri Slaby, David S. Miller
With this reproducer:
struct sockaddr_alg alg = {
.salg_family = 0x26,
.salg_type = "hash",
.salg_feat = 0xf,
.salg_mask = 0x5,
.salg_name = "digest_null",
};
int sock, sock2;
sock = socket(AF_ALG, SOCK_SEQPACKET, 0);
bind(sock, (struct sockaddr *)&alg, sizeof(alg));
sock2 = accept(sock, NULL, NULL);
setsockopt(sock, SOL_ALG, ALG_SET_KEY, "\x9b\xca", 2);
accept(sock2, NULL, NULL);
==== 8< ======== 8< ======== 8< ======== 8< ====
one can immediatelly see an UBSAN warning:
UBSAN: Undefined behaviour in crypto/algif_hash.c:187:7
variable length array bound value 0 <= 0
CPU: 0 PID: 15949 Comm: syz-executor Tainted: G E 4.4.30-0-default #1
...
Call Trace:
...
[<ffffffff81d598fd>] ? __ubsan_handle_vla_bound_not_positive+0x13d/0x188
[<ffffffff81d597c0>] ? __ubsan_handle_out_of_bounds+0x1bc/0x1bc
[<ffffffffa0e2204d>] ? hash_accept+0x5bd/0x7d0 [algif_hash]
[<ffffffffa0e2293f>] ? hash_accept_nokey+0x3f/0x51 [algif_hash]
[<ffffffffa0e206b0>] ? hash_accept_parent_nokey+0x4a0/0x4a0 [algif_hash]
[<ffffffff8235c42b>] ? SyS_accept+0x2b/0x40
It is a correct warning, as hash state is propagated to accept as zero,
but creating a zero-length variable array is not allowed in C.
Fix this as proposed by Herbert -- do "?: 1" on that site. No sizeof or
similar happens in the code there, so we just allocate one byte even
though we do not use the array.
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net> (maintainer:CRYPTO API)
Reported-by: Sasha Levin <sasha.levin@oracle.com>
---
crypto/algif_hash.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
index d19b09cdf284..54fc90e8339c 100644
--- a/crypto/algif_hash.c
+++ b/crypto/algif_hash.c
@@ -245,7 +245,7 @@ static int hash_accept(struct socket *sock, struct socket *newsock, int flags)
struct alg_sock *ask = alg_sk(sk);
struct hash_ctx *ctx = ask->private;
struct ahash_request *req = &ctx->req;
- char state[crypto_ahash_statesize(crypto_ahash_reqtfm(req))];
+ char state[crypto_ahash_statesize(crypto_ahash_reqtfm(req)) ? : 1];
struct sock *sk2;
struct alg_sock *ask2;
struct hash_ctx *ctx2;
--
2.11.0
^ permalink raw reply related
* Re: [PATCH] crypto: skcipher - fix crash in virtual walk
From: Milan Broz @ 2016-12-15 13:27 UTC (permalink / raw)
To: Herbert Xu, Ard Biesheuvel; +Cc: linux-crypto
In-Reply-To: <20161214103905.GB11960@gondor.apana.org.au>
On 12/14/2016 11:39 AM, Herbert Xu wrote:
> On Tue, Dec 13, 2016 at 01:34:02PM +0000, Ard Biesheuvel wrote:
>> The new skcipher walk API may crash in the following way. (Interestingly,
>> the tcrypt boot time tests seem unaffected, while an explicit test using
>> the module triggers it)
>>
>> Unable to handle kernel NULL pointer dereference at virtual address 00000000
>> ...
>> [<ffff000008431d84>] __memcpy+0x84/0x180
>> [<ffff0000083ec0d0>] skcipher_walk_done+0x328/0x340
>> [<ffff0000080c5c04>] ctr_encrypt+0x84/0x100
>> [<ffff000008406d60>] simd_skcipher_encrypt+0x88/0x98
>> [<ffff0000083fa05c>] crypto_rfc3686_crypt+0x8c/0x98
>> [<ffff0000009b0900>] test_skcipher_speed+0x518/0x820 [tcrypt]
>> [<ffff0000009b31c0>] do_test+0x1408/0x3b70 [tcrypt]
>> [<ffff0000009bd050>] tcrypt_mod_init+0x50/0x1000 [tcrypt]
>> [<ffff0000080838f4>] do_one_initcall+0x44/0x138
>> [<ffff0000081aee60>] do_init_module+0x68/0x1e0
>> [<ffff0000081524d0>] load_module+0x1fd0/0x2458
>> [<ffff000008152c38>] SyS_finit_module+0xe0/0xf0
>> [<ffff0000080836f0>] el0_svc_naked+0x24/0x28
>>
>> This is due to the fact that skcipher_done_slow() may be entered with
>> walk->buffer unset. Since skcipher_walk_done() already deals with the
>> case where walk->buffer == walk->page, it appears to be the intention
>> that walk->buffer point to walk->page after skcipher_next_slow(), so
>> ensure that is the case.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> Patch applied. Thanks.
Fixed problem here as well...
Tested-by: Milan Broz <gmazyland@gmail.com>
Without patch, just running cryptsetup tests (make check) on 32bit machine
I get kernel crash in LRW mode.
For me, the problem can be triggered from the userspace crypto API (no root needed!)
dd if=/dev/zero of=test bs=1M count=32
echo blah | /sbin/cryptsetup luksFormat test -c aes-lrw-null -q --use-urandom
and I get
Dec 15 13:00:50 kernel: NET: Registered protocol family 38
Dec 15 13:00:50 kernel: BUG: unable to handle kernel NULL pointer dereference at (null)
Dec 15 13:00:50 kernel: IP: memcpy+0xf/0x20
Dec 15 13:00:50 kernel: *pde = 00000000
Dec 15 13:00:50 kernel:
Dec 15 13:00:50 kernel: Oops: 0000 [#1] PREEMPT SMP
Dec 15 13:00:50 kernel: Modules linked in: lrw algif_skcipher af_alg loop rpcsec_gss_krb5 dm_mod crc32_pclmul crc32c_intel pcbc aesni_intel aes_i586 crypto_simd cryptd ata_piix
Dec 15 13:00:50 kernel: CPU: 0 PID: 1667 Comm: cryptsetup Tainted: G W 4.9.0+ #122
Dec 15 13:00:50 kernel: Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 07/02/2015
Dec 15 13:00:50 kernel: task: f5cc66c0 task.stack: f45e6000
Dec 15 13:00:50 kernel: EIP: memcpy+0xf/0x20
Dec 15 13:00:50 kernel: EFLAGS: 00010202 CPU: 0
Dec 15 13:00:50 kernel: EAX: fffbaffc EBX: 00000004 ECX: 00000001 EDX: 00000000
Dec 15 13:00:50 kernel: ESI: 00000000 EDI: fffbaffc EBP: f45e7d48 ESP: f45e7d3c
Dec 15 13:00:50 kernel: DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
Dec 15 13:00:50 kernel: CR0: 80050033 CR2: 00000000 CR3: 35eba000 CR4: 001406d0
Dec 15 13:00:50 kernel: Call Trace:
Dec 15 13:00:50 kernel: scatterwalk_copychunks+0x75/0xd0
Dec 15 13:00:50 kernel: skcipher_walk_done+0x2b1/0x300
Dec 15 13:00:50 kernel: pre_crypt+0x230/0x350 [lrw]
Dec 15 13:00:50 kernel: ? __kmalloc+0x222/0x270
Dec 15 13:00:50 kernel: do_decrypt+0x27/0xb0 [lrw]
Dec 15 13:00:50 kernel: decrypt+0x19/0x20 [lrw]
Dec 15 13:00:50 kernel: skcipher_recvmsg+0x2d3/0x6c0 [algif_skcipher]
Dec 15 13:00:50 kernel: ? trace_hardirqs_on_caller+0xd6/0x200
Dec 15 13:00:50 kernel: ? release_sock+0x63/0x90
Dec 15 13:00:50 kernel: ? trace_hardirqs_on+0xb/0x10
Dec 15 13:00:50 kernel: ? __local_bh_enable_ip+0x5c/0xd0
Dec 15 13:00:50 kernel: ? _raw_spin_unlock_bh+0x2a/0x30
Dec 15 13:00:50 kernel: skcipher_recvmsg_nokey+0x26/0x38 [algif_skcipher]
Dec 15 13:00:50 kernel: sock_read_iter+0x77/0xb0
Dec 15 13:00:50 kernel: __vfs_read+0xaa/0x120
Dec 15 13:00:50 kernel: vfs_read+0x72/0x100
Dec 15 13:00:50 kernel: SyS_read+0x3d/0xa0
Dec 15 13:00:50 kernel: do_int80_syscall_32+0x40/0x110
Dec 15 13:00:50 kernel: entry_INT80_32+0x2f/0x2f
Dec 15 13:00:50 kernel: EIP: 0xb77cc9f2
Dec 15 13:00:50 kernel: EFLAGS: 00000246 CPU: 0
Dec 15 13:00:50 kernel: EAX: ffffffda EBX: 00000006 ECX: bff01f4c EDX: 00000200
Dec 15 13:00:50 kernel: ESI: 00000030 EDI: fffffffb EBP: bff01e78 ESP: bff01dd4
Dec 15 13:00:50 kernel: DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 007b
Dec 15 13:00:50 kernel: Code: fc ff ff 8b 43 58 2b 43 50 88 43 4e 5b 5d c3 90 90 90 90 90 90 90 90 90 90 90 90 90 55 89 e5 57 89 c7 56 89 d6 53 89 cb c1 e9 02 <f3> a5 89 d9 83 e1 03 74 02 f3 a4 5b 5e 5f 5d c3 90 55 89 e5 57
Dec 15 13:00:50 kernel: EIP: memcpy+0xf/0x20 SS:ESP: 0068:f45e7d3c
Dec 15 13:00:50 kernel: CR2: 0000000000000000
Dec 15 13:00:50 kernel: ---[ end trace 6d5fbbd95de42602 ]---
Dec 15 13:00:50 kernel: note: cryptsetup[1667] exited with preempt_count 1
^ permalink raw reply
* Re: [PATCH v2 1/4] siphash: add cryptographically secure hashtable function
From: Hannes Frederic Sowa @ 2016-12-15 12:50 UTC (permalink / raw)
To: David Laight, Jason A. Donenfeld
Cc: Netdev, kernel-hardening@lists.openwall.com,
Jean-Philippe Aumasson, LKML, Linux Crypto Mailing List,
Daniel J . Bernstein, Linus Torvalds, Eric Biggers
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB02402C0@AcuExch.aculab.com>
On 15.12.2016 13:28, David Laight wrote:
> From: Hannes Frederic Sowa
>> Sent: 15 December 2016 12:23
> ...
>> Hmm? Even the Intel ABI expects alignment of unsigned long long to be 8
>> bytes on 32 bit. Do you question that?
>
> Yes.
>
> The linux ABI for x86 (32 bit) only requires 32bit alignment for u64 (etc).
Hmm, u64 on 32 bit is unsigned long long and not unsigned long. Thus I
am actually not sure if the ABI would say anything about that (sorry
also for my wrong statement above).
Alignment requirement of unsigned long long on gcc with -m32 actually
seem to be 8.
^ permalink raw reply
* Re: [PATCH v2 0/3] Add Support for Cavium Cryptographic Accelerarion Unit
From: George Cherian @ 2016-12-15 12:42 UTC (permalink / raw)
To: George Cherian, herbert, davem; +Cc: linux-kernel, linux-crypto
In-Reply-To: <1481637801-1076-1-git-send-email-george.cherian@cavium.com>
Hi,
I got few review comments for this series from David Daney.
I am reworking on this series and will sent v3 once it is done.
So,kindly ignore this series
Regards,
-George
On 12/13/2016 07:33 PM, George Cherian wrote:
>
> This series adds the support for Cavium Cryptographic Accelerarion Unit (CPT)
> CPT is available in Cavium's Octeon-Tx SoC series.
>
> The series was tested with ecryptfs and dm-crypt for in kernel cryptographic
> offload operations.
>
> Changes v1 -> v2
> -- Addressed a crash issue when more gather components are passed.
> -- Redo the cptvf request manager.
> - Get rid of the un necessary buffer copies.
> -- s/uint*_t/u*
> -- Remove unwanted Macro definitions
> -- Remove the redundant ROUNDUP* macros and use kernel function
> -- Select proper config option in Kconfig file.
> -- Removed some of the unwanted header file inclusions
> -- Miscellaneous Cleanup
>
> George Cherian (3):
> drivers: crypto: Add Support for Octeon-tx CPT Engine
> drivers: crypto: Add the Virtual Function driver for CPT
> drivers: crypto: Enable CPT options crypto for build
>
> drivers/crypto/Kconfig | 1 +
> drivers/crypto/Makefile | 1 +
> drivers/crypto/cavium/cpt/Kconfig | 16 +
> drivers/crypto/cavium/cpt/Makefile | 3 +
> drivers/crypto/cavium/cpt/cpt_common.h | 166 +++++
> drivers/crypto/cavium/cpt/cpt_hw_types.h | 736 ++++++++++++++++++++
> drivers/crypto/cavium/cpt/cptpf.h | 69 ++
> drivers/crypto/cavium/cpt/cptpf_main.c | 733 ++++++++++++++++++++
> drivers/crypto/cavium/cpt/cptpf_mbox.c | 163 +++++
> drivers/crypto/cavium/cpt/cptvf.h | 145 ++++
> drivers/crypto/cavium/cpt/cptvf_algs.c | 424 ++++++++++++
> drivers/crypto/cavium/cpt/cptvf_algs.h | 110 +++
> drivers/crypto/cavium/cpt/cptvf_main.c | 971 +++++++++++++++++++++++++++
> drivers/crypto/cavium/cpt/cptvf_mbox.c | 205 ++++++
> drivers/crypto/cavium/cpt/cptvf_reqmanager.c | 581 ++++++++++++++++
> drivers/crypto/cavium/cpt/request_manager.h | 147 ++++
> 16 files changed, 4471 insertions(+)
> create mode 100644 drivers/crypto/cavium/cpt/Kconfig
> create mode 100644 drivers/crypto/cavium/cpt/Makefile
> create mode 100644 drivers/crypto/cavium/cpt/cpt_common.h
> create mode 100644 drivers/crypto/cavium/cpt/cpt_hw_types.h
> create mode 100644 drivers/crypto/cavium/cpt/cptpf.h
> create mode 100644 drivers/crypto/cavium/cpt/cptpf_main.c
> create mode 100644 drivers/crypto/cavium/cpt/cptpf_mbox.c
> create mode 100644 drivers/crypto/cavium/cpt/cptvf.h
> create mode 100644 drivers/crypto/cavium/cpt/cptvf_algs.c
> create mode 100644 drivers/crypto/cavium/cpt/cptvf_algs.h
> create mode 100644 drivers/crypto/cavium/cpt/cptvf_main.c
> create mode 100644 drivers/crypto/cavium/cpt/cptvf_mbox.c
> create mode 100644 drivers/crypto/cavium/cpt/cptvf_reqmanager.c
> create mode 100644 drivers/crypto/cavium/cpt/request_manager.h
>
^ permalink raw reply
* RE: [PATCH v2 1/4] siphash: add cryptographically secure hashtable function
From: David Laight @ 2016-12-15 12:28 UTC (permalink / raw)
To: 'Hannes Frederic Sowa', Jason A. Donenfeld
Cc: Netdev, kernel-hardening@lists.openwall.com,
Jean-Philippe Aumasson, LKML, Linux Crypto Mailing List,
Daniel J . Bernstein, Linus Torvalds, Eric Biggers
In-Reply-To: <707472e1-b385-836d-c4c6-791c1dcc0776@stressinduktion.org>
From: Hannes Frederic Sowa
> Sent: 15 December 2016 12:23
...
> Hmm? Even the Intel ABI expects alignment of unsigned long long to be 8
> bytes on 32 bit. Do you question that?
Yes.
The linux ABI for x86 (32 bit) only requires 32bit alignment for u64 (etc).
David
^ permalink raw reply
* Re: [PATCH v2 1/4] siphash: add cryptographically secure hashtable function
From: Hannes Frederic Sowa @ 2016-12-15 12:23 UTC (permalink / raw)
To: David Laight, Jason A. Donenfeld
Cc: Netdev, kernel-hardening@lists.openwall.com,
Jean-Philippe Aumasson, LKML, Linux Crypto Mailing List,
Daniel J . Bernstein, Linus Torvalds, Eric Biggers
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB02401ED@AcuExch.aculab.com>
On 15.12.2016 12:04, David Laight wrote:
> From: Hannes Frederic Sowa
>> Sent: 14 December 2016 22:03
>> On 14.12.2016 13:46, Jason A. Donenfeld wrote:
>>> Hi David,
>>>
>>> On Wed, Dec 14, 2016 at 10:56 AM, David Laight <David.Laight@aculab.com> wrote:
>>>> ...
>>>>> +u64 siphash24(const u8 *data, size_t len, const u8 key[SIPHASH24_KEY_LEN])
>>>> ...
>>>>> + u64 k0 = get_unaligned_le64(key);
>>>>> + u64 k1 = get_unaligned_le64(key + sizeof(u64));
>>>> ...
>>>>> + m = get_unaligned_le64(data);
>>>>
>>>> All these unaligned accesses are going to get expensive on architectures
>>>> like sparc64.
>>>
>>> Yes, the unaligned accesses aren't pretty. Since in pretty much all
>>> use cases thus far, the data can easily be made aligned, perhaps it
>>> makes sense to create siphash24() and siphash24_unaligned(). Any
>>> thoughts on doing something like that?
>>
>> I fear that the alignment requirement will be a source of bugs on 32 bit
>> machines, where you cannot even simply take a well aligned struct on a
>> stack and put it into the normal siphash(aligned) function without
>> adding alignment annotations everywhere. Even blocks returned from
>> kmalloc on 32 bit are not aligned to 64 bit.
>
> Are you doing anything that will require 64bit alignment on 32bit systems?
> It is unlikely that the kernel can use any simd registers that have wider
> alignment requirements.
>
> You also really don't want to request on-stack items have large alignments.
> While gcc can generate code to do it, it isn't pretty.
Hmm? Even the Intel ABI expects alignment of unsigned long long to be 8
bytes on 32 bit. Do you question that?
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox