* [PATCH v2 2/4] siphash: add convenience functions for jhash converts
From: Jason A. Donenfeld @ 2016-12-14 3:59 UTC (permalink / raw)
To: Netdev, kernel-hardening, LKML, linux-crypto; +Cc: Jason A. Donenfeld
In-Reply-To: <20161214035927.30004-1-Jason@zx2c4.com>
Many jhash users currently rely on the Nwords functions. In order to
make transitions to siphash fit something people already know about, we
provide analog functions here. This also winds up being nice for the
networking stack, where hashing 32-bit fields is common.
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
Changes from v1->v2:
- None in this patch, but see elsewhere in series.
include/linux/siphash.h | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/include/linux/siphash.h b/include/linux/siphash.h
index 6623b3090645..1391054c4c29 100644
--- a/include/linux/siphash.h
+++ b/include/linux/siphash.h
@@ -17,4 +17,37 @@ enum siphash_lengths {
u64 siphash24(const u8 *data, size_t len, const u8 key[SIPHASH24_KEY_LEN]);
+static inline u64 siphash24_1word(const u32 a, const u8 key[SIPHASH24_KEY_LEN])
+{
+ return siphash24((u8 *)&a, sizeof(a), key);
+}
+
+static inline u64 siphash24_2words(const u32 a, const u32 b, const u8 key[SIPHASH24_KEY_LEN])
+{
+ const struct {
+ u32 a;
+ u32 b;
+ } __packed combined = {
+ .a = a,
+ .b = b
+ };
+
+ return siphash24((const u8 *)&combined, sizeof(combined), key);
+}
+
+static inline u64 siphash24_3words(const u32 a, const u32 b, const u32 c, const u8 key[SIPHASH24_KEY_LEN])
+{
+ const struct {
+ u32 a;
+ u32 b;
+ u32 c;
+ } __packed combined = {
+ .a = a,
+ .b = b,
+ .c = c
+ };
+
+ return siphash24((const u8 *)&combined, sizeof(combined), key);
+}
+
#endif /* _LINUX_SIPHASH_H */
--
2.11.0
^ permalink raw reply related
* [PATCH 4/3] random: use siphash24 instead of md5 for get_random_int/long
From: Jason A. Donenfeld @ 2016-12-14 3:10 UTC (permalink / raw)
To: Netdev, David Miller, Linus Torvalds,
kernel-hardening@lists.openwall.com, LKML, George Spelvin,
Scott Bauer, Andi Kleen, Andy Lutomirski, Greg KH, Eric Biggers,
linux-crypto, Ted Tso
Cc: Jason A. Donenfeld, Jean-Philippe Aumasson
In-Reply-To: <20161214001656.19388-1-Jason@zx2c4.com>
This duplicates the current algorithm for get_random_int/long, but uses
siphash24 instead. This comes with several benefits. It's certainly
faster and more cryptographically secure than MD5. This patch also
hashes the pid, entropy, and timestamp as fixed width fields, 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 siphash24, 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.
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Jean-Philippe Aumasson <jeanphilippe.aumasson@gmail.com>
---
drivers/char/random.c | 50 +++++++++++++++++++++++++++++++-------------------
1 file changed, 31 insertions(+), 19 deletions(-)
diff --git a/drivers/char/random.c b/drivers/char/random.c
index d6876d506220..25f96f074da5 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 u8 random_int_secret[SIPHASH24_KEY_LEN];
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,25 @@ static DEFINE_PER_CPU(__u32 [MD5_DIGEST_WORDS], get_random_int_hash)
*/
unsigned int get_random_int(void)
{
- __u32 *hash;
+ uint64_t *chaining;
unsigned int ret;
+ struct {
+ uint64_t chaining;
+ unsigned long ts;
+ unsigned long entropy;
+ pid_t pid;
+ } __packed combined;
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);
+ combined.chaining = *chaining;
+ combined.ts = jiffies;
+ combined.entropy = random_get_entropy();
+ combined.pid = current->pid;
+ ret = *chaining = siphash24((u8 *)&combined, sizeof(combined), random_int_secret);
+ put_cpu_var(chaining);
return ret;
}
EXPORT_SYMBOL(get_random_int);
@@ -2083,19 +2089,25 @@ EXPORT_SYMBOL(get_random_int);
*/
unsigned long get_random_long(void)
{
- __u32 *hash;
+ uint64_t *chaining;
unsigned long ret;
+ struct {
+ uint64_t chaining;
+ unsigned long ts;
+ unsigned long entropy;
+ pid_t pid;
+ } __packed combined;
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);
+ combined.chaining = *chaining;
+ combined.ts = jiffies;
+ combined.entropy = random_get_entropy();
+ combined.pid = current->pid;
+ ret = *chaining = siphash24((u8 *)&combined, sizeof(combined), random_int_secret);
+ put_cpu_var(chaining);
return ret;
}
EXPORT_SYMBOL(get_random_long);
--
2.11.0
^ permalink raw reply related
* [PATCH v2] wusbcore: Fix one more crypto-on-the-stack bug
From: Andy Lutomirski @ 2016-12-14 2:50 UTC (permalink / raw)
To: linux-kernel, linux-usb, gregkh
Cc: Eric Biggers, linux-crypto, Herbert Xu, Stephan Mueller,
Andy Lutomirski
The driver put a constant buffer of all zeros on the stack and
pointed a scatterlist entry at it. This doesn't work with virtual
stacks. Use ZERO_PAGE instead.
Cc: stable@vger.kernel.org # 4.9 only
Reported-by: Eric Biggers <ebiggers3@gmail.com>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
drivers/usb/wusbcore/crypto.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/usb/wusbcore/crypto.c b/drivers/usb/wusbcore/crypto.c
index 79451f7ef1b7..062c205f0046 100644
--- a/drivers/usb/wusbcore/crypto.c
+++ b/drivers/usb/wusbcore/crypto.c
@@ -216,7 +216,6 @@ static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc,
struct scatterlist sg[4], sg_dst;
void *dst_buf;
size_t dst_size;
- const u8 bzero[16] = { 0 };
u8 iv[crypto_skcipher_ivsize(tfm_cbc)];
size_t zero_padding;
@@ -261,7 +260,7 @@ static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc,
sg_set_buf(&sg[1], &scratch->b1, sizeof(scratch->b1));
sg_set_buf(&sg[2], b, blen);
/* 0 if well behaved :) */
- sg_set_buf(&sg[3], bzero, zero_padding);
+ sg_set_page(&sg[3], ZERO_PAGE(0), zero_padding, 0);
sg_init_one(&sg_dst, dst_buf, dst_size);
skcipher_request_set_tfm(req, tfm_cbc);
--
2.9.3
^ permalink raw reply related
* Re: [PATCH v2] keys/encrypted: Fix two crypto-on-the-stack bugs
From: Andy Lutomirski @ 2016-12-14 2:53 UTC (permalink / raw)
To: Andy Lutomirski
Cc: linux-kernel@vger.kernel.org, USB list, David Howells, keyrings,
Eric Biggers, linux-crypto, Herbert Xu, Stephan Mueller
In-Reply-To: <627e948e37314c13a67c90917386c814c56b8e20.1481683609.git.luto@kernel.org>
On Tue, Dec 13, 2016 at 6:48 PM, Andy Lutomirski <luto@kernel.org> wrote:
> The driver put a constant buffer of all zeros on the stack and
> pointed a scatterlist entry at it in two places. This doesn't work
> with virtual stacks. Use ZERO_PAGE instead.
Wait a second...
> - sg_set_buf(&sg_out[1], pad, sizeof pad);
> + sg_set_buf(&sg_out[1], empty_zero_page, 16);
My fix here is obviously bogus (I meant to use ZERO_PAGE(0)), but what
exactly is the code trying to do? The old code makes no sense. It's
setting the *output* buffer to zeroed padding.
--Andy
^ permalink raw reply
* [PATCH v2] keys/encrypted: Fix two crypto-on-the-stack bugs
From: Andy Lutomirski @ 2016-12-14 2:48 UTC (permalink / raw)
To: linux-kernel, linux-usb, dhowells, keyrings
Cc: Eric Biggers, linux-crypto, Herbert Xu, Stephan Mueller,
Andy Lutomirski
The driver put a constant buffer of all zeros on the stack and
pointed a scatterlist entry at it in two places. This doesn't work
with virtual stacks. Use ZERO_PAGE instead.
Cc: stable@vger.kernel.org # 4.9 only
Reported-by: Eric Biggers <ebiggers3@gmail.com>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
security/keys/encrypted-keys/encrypted.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
index 17a06105ccb6..e963160ed26a 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -481,7 +481,6 @@ static int derived_key_encrypt(struct encrypted_key_payload *epayload,
unsigned int encrypted_datalen;
u8 iv[AES_BLOCK_SIZE];
unsigned int padlen;
- char pad[16];
int ret;
encrypted_datalen = roundup(epayload->decrypted_datalen, blksize);
@@ -493,11 +492,10 @@ static int derived_key_encrypt(struct encrypted_key_payload *epayload,
goto out;
dump_decrypted_data(epayload);
- memset(pad, 0, sizeof pad);
sg_init_table(sg_in, 2);
sg_set_buf(&sg_in[0], epayload->decrypted_data,
epayload->decrypted_datalen);
- sg_set_buf(&sg_in[1], pad, padlen);
+ sg_set_page(&sg_in[1], ZERO_PAGE(0), padlen, 0);
sg_init_table(sg_out, 1);
sg_set_buf(sg_out, epayload->encrypted_data, encrypted_datalen);
@@ -584,7 +582,6 @@ static int derived_key_decrypt(struct encrypted_key_payload *epayload,
struct skcipher_request *req;
unsigned int encrypted_datalen;
u8 iv[AES_BLOCK_SIZE];
- char pad[16];
int ret;
encrypted_datalen = roundup(epayload->decrypted_datalen, blksize);
@@ -594,13 +591,12 @@ static int derived_key_decrypt(struct encrypted_key_payload *epayload,
goto out;
dump_encrypted_data(epayload, encrypted_datalen);
- memset(pad, 0, sizeof pad);
sg_init_table(sg_in, 1);
sg_init_table(sg_out, 2);
sg_set_buf(sg_in, epayload->encrypted_data, encrypted_datalen);
sg_set_buf(&sg_out[0], epayload->decrypted_data,
epayload->decrypted_datalen);
- sg_set_buf(&sg_out[1], pad, sizeof pad);
+ sg_set_buf(&sg_out[1], empty_zero_page, 16);
memcpy(iv, epayload->iv, sizeof(iv));
skcipher_request_set_crypt(req, sg_in, sg_out, encrypted_datalen, iv);
--
2.9.3
^ permalink raw reply related
* Re: [PATCH v2 0/8] Conversion crypto API documentation to Sphinx
From: Jonathan Corbet @ 2016-12-13 23:40 UTC (permalink / raw)
To: Stephan Müller; +Cc: Herbert Xu, linux-crypto, linux-doc
In-Reply-To: <3555336.Wm4L5bpP9y@positron.chronox.de>
On Tue, 13 Dec 2016 23:06:14 +0100
Stephan Müller <smueller@chronox.de> wrote:
> I am sorry, then I may have misunderstood you.
>
> I would think that Herbert agreed that you push the entire patch set through
> your tree, including the documentation changes to the crypto header files.
>
> I just did a patch on the parts that touch the crypto header files. All apply
> cleanly except patch 07/08 which has a 5 line hunk due to other changes, Is
> that hunk ok for you, it would be great if you could take it. If you do not
> like that hunk, we could push it through the cryptodev tree once rc-1 is out.
Clearly there was a bit of misunderstanding going around :)
I've applied the patch set to docs-next. Oddly I had no trouble with #7,
but #2 was badly mailer-mangled and took some work.
Anyway, it should make its way into linux-next; if there's no fireworks,
I'll endeavor to send it Linusward in time to avoid interfering with his
kitchen duties.
Sorry for the confusion,
jon
^ permalink raw reply
* Re: [PATCH v3] siphash: add cryptographically secure hashtable function
From: Jason A. Donenfeld @ 2016-12-13 23:36 UTC (permalink / raw)
To: Linus Torvalds
Cc: Andi Kleen, kernel-hardening@lists.openwall.com, LKML,
Linux Crypto Mailing List, George Spelvin, Scott Bauer,
Andy Lutomirski, Greg KH, Eric Biggers, Jean-Philippe Aumasson,
Daniel J . Bernstein
In-Reply-To: <CA+55aFyBsU_sxUuuNBMFUQonWOtfoW9AMk=vn=KLTKrkXVv+MA@mail.gmail.com>
Hi Linus,
On Tue, Dec 13, 2016 at 8:25 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> Yeah,. the TCP sequence number md5_transform() cases are likely the
> best example of something where siphash might be good. That tends to
> be really just a couple words of data (the address and port info) plus
> the net_secret[] hash. I think they currently simply just fill in the
> fixed-sized 64-byte md5-round area.
>
> I wonder it's worth it to have a special spihash version that does
> that same "fixed 64-byte area" thing.
What happens in MD5 the hash function is that it first initializes its
initial 128-bit hash to a magic constant, and then reads 64 bytes at a
time from the input and calls md5_transform on that, which each time
manipulates that 128-bit value from its starting value. At the end of
the input, some special padding is applied for small final blocks,
some finalization, and then the resultant hash is whatever that
128-bit value is at the end of the process.
What the tcp stack does with secure_tcp_sequence_number function in
net/core/secure_seq.c, and a variety of other places, is to just
supply that 128-bit initial value not with the magic constant, but
instead with saddr||daddr||sport||dport||net_secret[15] and then calls
md5_transform on the 64-byte long term secret random value
(net_secret). From the resultant 128-bit value, they take the first
32-bits. In addition to being rather heavy weight, this strikes me as
cryptographically a bit dubious too. But that's where your "fixed
64-byte area" notion comes from.
Siphash makes things a lot more simple than that. Since siphash is a
PRF and not a mere hash function, it takes an explicit secret key
parameter, which would be net_secret, some input data, which would be
saddr||daddr||sport||dport, and then spits out a 64-bit number, 32
bits of which would be used as the sequence number.
seq_num = seq_scale(siphash24(saddr||daddr||sport||dport, net_secret));
A lot simpler, faster, and actually secure.
> But please talk to the netwotrking people. Maybe that's the proper way
> to get this merged?
I had hoped to do it the lazy way, and just have it just wind up in
lib/. But I suppose you and Greg are of course right, and I should
submit this with a real usage. So I'll do that, and resubmit in
another thread as a series to LKML and netdev.
Thanks for your feedback!
Jason
^ permalink raw reply
* [PATCH v4] siphash: add cryptographically secure hashtable function
From: Jason A. Donenfeld @ 2016-12-13 22:48 UTC (permalink / raw)
To: Linus Torvalds, kernel-hardening@lists.openwall.com, LKML,
Linux Crypto Mailing List, George Spelvin, Scott Bauer,
Andi Kleen, Andy Lutomirski, Greg KH, Eric Biggers
Cc: Jason A. Donenfeld, Jean-Philippe Aumasson, Daniel J . Bernstein
In-Reply-To: <20161213083948.GA8994@zzz>
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.
SipHash isn't just some new trendy hash function. It's been around for a
while, and there really isn't anything that comes remotely close to
being useful in the way SipHash is. With that said, why do we need this?
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.
Linux developers already seem to be aware that this is an issue, and
various places that use hash tables in, say, a network context, use a
non-cryptographically secure function (usually jhash) and then try to
twiddle with the key on a time basis (or in many cases just do nothing
and hope that nobody notices). While this is an admirable attempt at
solving the problem, it doesn't actually fix it. SipHash fixes it.
(It fixes it in such a sound way that you could even build a stream
cipher out of SipHash that would resist the modern cryptanalysis.)
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.
Dozens of languages are already using this internally for their hash
tables. Some of the BSDs already use this in their kernels. SipHash is
a widely known high-speed solution to a widely known problem, 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>
---
Changes from v3->v4:
- load_unaligned_zeropad is only called when left is non zero
include/linux/siphash.h | 20 +++++++++++++
lib/Kconfig.debug | 6 ++--
lib/Makefile | 5 ++--
lib/siphash.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++++
lib/test_siphash.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 176 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..6623b3090645
--- /dev/null
+++ b/include/linux/siphash.h
@@ -0,0 +1,20 @@
+/* Copyright (C) 2016 Jason A. Donenfeld <Jason@zx2c4.com>
+ *
+ * This file is provided under a dual BSD/GPLv2 license.
+ *
+ * SipHash: a fast short-input PRF
+ * https://131002.net/siphash/
+ */
+
+#ifndef _LINUX_SIPHASH_H
+#define _LINUX_SIPHASH_H
+
+#include <linux/types.h>
+
+enum siphash_lengths {
+ SIPHASH24_KEY_LEN = 16
+};
+
+u64 siphash24(const u8 *data, size_t len, const u8 key[SIPHASH24_KEY_LEN]);
+
+#endif /* _LINUX_SIPHASH_H */
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index a6c8db1d62f6..2a1797704b41 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1823,9 +1823,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..7b55ad3a7fe9
--- /dev/null
+++ b/lib/siphash.c
@@ -0,0 +1,76 @@
+/* Copyright (C) 2015-2016 Jason A. Donenfeld <Jason@zx2c4.com>
+ * Copyright (C) 2012-2014 Jean-Philippe Aumasson <jeanphilippe.aumasson@gmail.com>
+ * Copyright (C) 2012-2014 Daniel J. Bernstein <djb@cr.yp.to>
+ *
+ * This file is provided under a dual BSD/GPLv2 license.
+ *
+ * SipHash: a fast short-input PRF
+ * https://131002.net/siphash/
+ */
+
+#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)
+
+u64 siphash24(const u8 *data, size_t len, const u8 key[SIPHASH24_KEY_LEN])
+{
+ u64 v0 = 0x736f6d6570736575ULL;
+ u64 v1 = 0x646f72616e646f6dULL;
+ u64 v2 = 0x6c7967656e657261ULL;
+ u64 v3 = 0x7465646279746573ULL;
+ u64 b = ((u64)len) << 56;
+ u64 k0 = get_unaligned_le64(key);
+ u64 k1 = get_unaligned_le64(key + sizeof(u64));
+ u64 m;
+ const u8 *end = data + len - (len % sizeof(u64));
+ const u8 left = len & (sizeof(u64) - 1);
+ v3 ^= k1;
+ v2 ^= k0;
+ v1 ^= k1;
+ v0 ^= k0;
+ 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(load_unaligned_zeropad(data) & bytemask_from_count(left));
+#else
+ switch (left) {
+ case 7: b |= ((u64)data[6]) << 48;
+ case 6: b |= ((u64)data[5]) << 40;
+ case 5: b |= ((u64)data[4]) << 32;
+ case 4: b |= get_unaligned_le32(data); break;
+ case 3: b |= ((u64)data[2]) << 16;
+ case 2: b |= get_unaligned_le16(data); break;
+ case 1: b |= data[0];
+ }
+#endif
+ v3 ^= b;
+ SIPROUND;
+ SIPROUND;
+ v0 ^= b;
+ v2 ^= 0xff;
+ SIPROUND;
+ SIPROUND;
+ SIPROUND;
+ SIPROUND;
+ return (v0 ^ v1) ^ (v2 ^ v3);
+}
+EXPORT_SYMBOL(siphash24);
diff --git a/lib/test_siphash.c b/lib/test_siphash.c
new file mode 100644
index 000000000000..336298aaa33b
--- /dev/null
+++ b/lib/test_siphash.c
@@ -0,0 +1,74 @@
+/* Test cases for siphash.c
+ *
+ * Copyright (C) 2015-2016 Jason A. Donenfeld <Jason@zx2c4.com>
+ *
+ * This file is provided under a dual BSD/GPLv2 license.
+ *
+ * SipHash: a fast short-input PRF
+ * https://131002.net/siphash/
+ */
+
+#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 int __init siphash_test_init(void)
+{
+ u8 in[64], k[16], i;
+ int ret = 0;
+
+ for (i = 0; i < 16; ++i)
+ k[i] = i;
+ for (i = 0; i < 64; ++i) {
+ in[i] = i;
+ if (siphash24(in, i, k) != test_vectors[i]) {
+ pr_info("self-test %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
* Re: [PATCH v3] siphash: add cryptographically secure hashtable function
From: Jason A. Donenfeld @ 2016-12-13 22:43 UTC (permalink / raw)
To: Eric Biggers
Cc: Linus Torvalds, kernel-hardening@lists.openwall.com, LKML,
Linux Crypto Mailing List, George Spelvin, Scott Bauer,
Andi Kleen, Andy Lutomirski, Greg KH, Jean-Philippe Aumasson,
Daniel J . Bernstein
In-Reply-To: <20161213083948.GA8994@zzz>
Hi Eric,
On Tue, Dec 13, 2016 at 9:39 AM, Eric Biggers <ebiggers3@gmail.com> wrote:
> Hmm, I don't think you can really do load_unaligned_zeropad() without first
> checking for 'left != 0'. The fixup section for load_unaligned_zeropad()
> assumes that rounding the pointer down to a word boundary will produce an
> address from which an 'unsigned long' can be loaded. But if 'left = 0' and we
> happen to be on a page boundary with the next page unmapped, then this will not
> be true and the second load will still fault.
Excellent point. I haven't been able to trigger this in my
experiments, but it doesn't look like there's much to prevent this
from happening. I'll submit a v4 with this as fixed, since there
hasn't been any other code quality issues.
Jason
^ permalink raw reply
* Re: [PATCH v2 0/8] Conversion crypto API documentation to Sphinx
From: Stephan Müller @ 2016-12-13 22:06 UTC (permalink / raw)
To: Jonathan Corbet; +Cc: Herbert Xu, linux-crypto, linux-doc
In-Reply-To: <20161213145059.5204d498@lwn.net>
Am Dienstag, 13. Dezember 2016, 14:50:59 CET schrieb Jonathan Corbet:
Hi Jonathan,
> On Tue, 13 Dec 2016 22:25:07 +0100
>
> Stephan Müller <smueller@chronox.de> wrote:
> > Considering that a large batch of documentation updates just landed in
> > Linus' tree, I am wondering why the crypto API documentation update is
> > not among it.
> Well, I'd asked if I could run the documentation-specific parts through
> docs-next. I guess I was waiting for a version of the patch set with just
> that, rather than intermixed with a bunch of crypto-side changes. Were
> you expecting me to take the whole set?
>
> I can still try to do that, I guess; let me know if I should try to set
> the whole set upward.
>
I am sorry, then I may have misunderstood you.
I would think that Herbert agreed that you push the entire patch set through
your tree, including the documentation changes to the crypto header files.
I just did a patch on the parts that touch the crypto header files. All apply
cleanly except patch 07/08 which has a 5 line hunk due to other changes, Is
that hunk ok for you, it would be great if you could take it. If you do not
like that hunk, we could push it through the cryptodev tree once rc-1 is out.
Thanks a lot.
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH v2 0/8] Conversion crypto API documentation to Sphinx
From: Jonathan Corbet @ 2016-12-13 21:50 UTC (permalink / raw)
To: Stephan Müller; +Cc: Herbert Xu, linux-crypto, linux-doc
In-Reply-To: <2168354.PkToFvufzV@positron.chronox.de>
On Tue, 13 Dec 2016 22:25:07 +0100
Stephan Müller <smueller@chronox.de> wrote:
> Considering that a large batch of documentation updates just landed in Linus'
> tree, I am wondering why the crypto API documentation update is not among it.
Well, I'd asked if I could run the documentation-specific parts through
docs-next. I guess I was waiting for a version of the patch set with just
that, rather than intermixed with a bunch of crypto-side changes. Were
you expecting me to take the whole set?
I can still try to do that, I guess; let me know if I should try to set
the whole set upward.
jon
^ permalink raw reply
* Re: [PATCH v2 0/8] Conversion crypto API documentation to Sphinx
From: Stephan Müller @ 2016-12-13 21:25 UTC (permalink / raw)
To: Jonathan Corbet; +Cc: Herbert Xu, linux-crypto, linux-doc
In-Reply-To: <20161025023549.GA17875@gondor.apana.org.au>
Am Dienstag, 25. Oktober 2016, 10:35:49 CET schrieb Herbert Xu:
Hi Jonathan,
> > > > the attached patch set converts the existing crypto API documentation
> > > > from DocBook to Sphinx.
> > >
> > > This looks generally good to me - thanks for doing it!
> > >
> > > Is there any chance of running the Documentation/ parts through the docs
> > > tree? Documentation/index.rst has become a bit of a conflict point
> > > otherwise...
> >
> > Unless Herbert objects, I would not see any reason why we should not push
> > it through the docs tree.
>
> No objections from me.
Considering that a large batch of documentation updates just landed in Linus'
tree, I am wondering why the crypto API documentation update is not among it.
Thanks
Stephan
^ permalink raw reply
* [PATCH] crypto: AF_ALG - fix memory management of aio with multiple iocbs
From: Stephan Müller @ 2016-12-13 20:42 UTC (permalink / raw)
To: herbert; +Cc: linux-crypto
Hi Herbert,
I am sorry to interrupt your merge window, but may I ask to consider
this patch for the current development cycle as well as for stable
back to v4.1 where the algif_skcipher AIO support was added?
It fixes the two bug reports which I reported back in September
that allow crashing the kernel from user space as an unprivileged
user. I think that this patch now fixes the real issue and not just
papers things over.
The fix can be validated using the following invocation from [1].
test/kcapi -d 2 -x 9 -e -c "cbc(aes)" -k
8d7dd9b0170ce0b5f2f8e1aa768e01e91da8bfc67fd486d081b28254c99eb423 -i
7fbc02ebf5b93322329df9bfccb635af -p 48981da18e4bb9ef7e2e3162d16b1910
Without the patch, the kernel crashes. With the patch, the kernel works.
The test duplicates the plaintext for supplying two IOCBs, expecting the
two identical blocks of ciphertext. When changing the test such that
both input data blocks are different, the resulting cipher text blocks
are different, as expected.
[1] http://www.chronox.de/libkcapi.html
---8<---
When submitting multiple IOCBs to be processed with one AIO invocation,
the initially supplied input data is processed with with each AIO
operation. For example, a simplified AIO operation may look like the
following:
1. sendmsg(32 bytes)
2. io_submit which defines 2 IOCBs (i.e. 2 operations providing
16 bytes buffer each to invoke an ecb(aes) operation)
The io_submit call is processed by the skcipher_recvmsg_async AF_ALG
handler. io_submit invokes skcipher_recvmsg_async once for each IOCB.
skcipher_recvmsg_async processes the ecb(aes) operation request, taking
the first 16 bytes from the input. When finishing the
skcipher_recvmsg_async operation, the page holding the 32 bytes of input
data from sendmsg cannot be released yet, but the scatter/gather list
pointing into the page needs to be advanced to point to the
second 16 bytes. Only when all data is used up, the page is released.
Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
crypto/algif_skcipher.c | 35 ++++++++++++++++++++++++++++++-----
1 file changed, 30 insertions(+), 5 deletions(-)
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 1e38aaa..68bde92 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -72,7 +72,8 @@ struct skcipher_async_req {
#define MAX_SGL_ENTS ((4096 - sizeof(struct skcipher_sg_list)) / \
sizeof(struct scatterlist) - 1)
-static void skcipher_free_async_sgls(struct skcipher_async_req *sreq)
+static void skcipher_free_async_sgls(struct skcipher_async_req *sreq,
+ unsigned int len)
{
struct skcipher_async_rsgl *rsgl, *tmp;
struct scatterlist *sgl;
@@ -86,8 +87,31 @@ static void skcipher_free_async_sgls(struct skcipher_async_req *sreq)
}
sgl = sreq->tsg;
n = sg_nents(sgl);
- for_each_sg(sgl, sg, n, i)
- put_page(sg_page(sg));
+ for_each_sg(sgl, sg, n, i) {
+ struct page *page = sg_page(sg);
+
+ if (!page)
+ continue;
+
+ /*
+ * The async operation may have processed only a subset of
+ * the data that was initially received from the caller.
+ * Thus, we only can release the data that a cipher operation
+ * processed.
+ */
+ if (len < sg->length) {
+ /* ensure that empty SGLs are not referenced any more */
+ sreq->tsg = sg;
+
+ /* advance the buffers to the unprocessed data */
+ sg->length -= len;
+ sg->offset += len;
+ return;
+ }
+
+ len -= sg->length;
+ put_page(page);
+ }
kfree(sreq->tsg);
}
@@ -95,10 +119,11 @@ static void skcipher_free_async_sgls(struct skcipher_async_req *sreq)
static void skcipher_async_cb(struct crypto_async_request *req, int err)
{
struct skcipher_async_req *sreq = req->data;
+ struct skcipher_request *sk_req = &sreq->req;
struct kiocb *iocb = sreq->iocb;
atomic_dec(sreq->inflight);
- skcipher_free_async_sgls(sreq);
+ skcipher_free_async_sgls(sreq, err ? 0 : sk_req->cryptlen);
kzfree(sreq);
iocb->ki_complete(iocb, err, err);
}
@@ -623,7 +648,7 @@ static int skcipher_recvmsg_async(struct socket *sock, struct msghdr *msg,
goto unlock;
}
free:
- skcipher_free_async_sgls(sreq);
+ skcipher_free_async_sgls(sreq, err ? 0 : len);
unlock:
skcipher_wmem_wakeup(sk);
release_sock(sk);
--
2.9.3
^ permalink raw reply related
* Re: [PATCH v2 2/2] crypto: mediatek - add DT bindings documentation
From: Rob Herring @ 2016-12-13 20:06 UTC (permalink / raw)
To: Ryder Lee
Cc: Herbert Xu, David S. Miller, Matthias Brugger, devicetree,
linux-mediatek, linux-kernel, linux-crypto, linux-arm-kernel,
Sean Wang, Roy Luo
In-Reply-To: <1481592676-2248-3-git-send-email-ryder.lee@mediatek.com>
On Tue, Dec 13, 2016 at 09:31:16AM +0800, Ryder Lee wrote:
> Add DT bindings documentation for the crypto driver
>
> Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
> ---
> .../devicetree/bindings/crypto/mediatek-crypto.txt | 32 ++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/crypto/mediatek-crypto.txt
>
> diff --git a/Documentation/devicetree/bindings/crypto/mediatek-crypto.txt b/Documentation/devicetree/bindings/crypto/mediatek-crypto.txt
> new file mode 100644
> index 0000000..47a786e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/crypto/mediatek-crypto.txt
> @@ -0,0 +1,32 @@
> +MediaTek cryptographic accelerators
> +
> +Required properties:
> +- compatible: Should be "mediatek,eip97-crypto"
> +- reg: Address and length of the register set for the device
> +- interrupts: Should contain the five crypto engines interrupts in numeric
> + order. These are global system and four descriptor rings.
> +- clocks: the clock used by the core
> +- clock-names: the names of the clock listed in the clocks property. These are
> + "ethif", "cryp"
> +- power-domains: Must contain a reference to the PM domain.
> +
> +
> +Optional properties:
> +- interrupt-parent: Should be the phandle for the interrupt controller
> + that services interrupts for this device
This is not optional. It's perhaps inherited from the parent. You can
drop it as it's implied by interrupts property.
Rob
^ permalink raw reply
* Re: [PATCH] keys/encrypted: Fix two crypto-on-the-stack bugs
From: David Howells @ 2016-12-13 20:02 UTC (permalink / raw)
To: Andy Lutomirski
Cc: dhowells, David Laight, Joerg Roedel, David Woodhouse,
Linus Torvalds, Ingo Molnar, Andy Lutomirski,
linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
keyrings@vger.kernel.org, Eric Biggers,
linux-crypto@vger.kernel.org, Herbert Xu, Stephan Mueller
In-Reply-To: <CALCETrVbEhxRWFmgrePeLriQU5J6ZZaN15tTAYja1hJwYFrRpg@mail.gmail.com>
Andy Lutomirski <luto@amacapital.net> wrote:
> I don't know whether you're right, but that sounds a bit silly to me.
> This is a *tiny* amount of memory.
Assuming a 1MiB kernel image in 4K pages, that gets you back a couple of pages
I think - useful if you've only got a few MiB of RAM.
David
^ permalink raw reply
* Re: [PATCH v3] siphash: add cryptographically secure hashtable function
From: Linus Torvalds @ 2016-12-13 19:26 UTC (permalink / raw)
To: Eric Biggers
Cc: Jason A. Donenfeld, kernel-hardening@lists.openwall.com, LKML,
Linux Crypto Mailing List, George Spelvin, Scott Bauer,
Andi Kleen, Andy Lutomirski, Greg KH, Jean-Philippe Aumasson,
Daniel J . Bernstein
In-Reply-To: <20161213083948.GA8994@zzz>
On Tue, Dec 13, 2016 at 12:39 AM, Eric Biggers <ebiggers3@gmail.com> wrote:
>
> Hmm, I don't think you can really do load_unaligned_zeropad() without first
> checking for 'left != 0'.
Right you are. If the allocation is at the end of a page, the 0-size
case would be entirely outside the page and there's no fixup.
Of course, that never happens in normal code, but DEBUG_PAGE_ALLOC can
trigger it.
Linus
^ permalink raw reply
* Re: [PATCH v3] siphash: add cryptographically secure hashtable function
From: Linus Torvalds @ 2016-12-13 19:25 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: Andi Kleen, kernel-hardening@lists.openwall.com, LKML,
Linux Crypto Mailing List, George Spelvin, Scott Bauer,
Andy Lutomirski, Greg KH, Eric Biggers, Jean-Philippe Aumasson,
Daniel J . Bernstein
In-Reply-To: <CAHmME9qk+Z8CdhTFQnTkwT8S-n56tzi77-uAV3dNigcGMQx7uQ@mail.gmail.com>
On Mon, Dec 12, 2016 at 3:04 PM, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> Indeed this would be a great first candidate. There are lots of places
> where MD5 (!!) is pulled in for this sort of thing, when SipHash could
> be a faster and leaner replacement (and arguably more secure than
> rusty MD5).
Yeah,. the TCP sequence number md5_transform() cases are likely the
best example of something where siphash might be good. That tends to
be really just a couple words of data (the address and port info) plus
the net_secret[] hash. I think they currently simply just fill in the
fixed-sized 64-byte md5-round area.
I wonder it's worth it to have a special spihash version that does
that same "fixed 64-byte area" thing.
But please talk to the netwotrking people. Maybe that's the proper way
to get this merged?
Linus
^ permalink raw reply
* Re: [PATCH] crypto: aesni-intel - RFC4106 can zero copy when !PageHighMem
From: Dave Watson @ 2016-12-13 19:07 UTC (permalink / raw)
To: Ilya Lesokhin; +Cc: linux-crypto, tadeusz.struk, herbert, tls-fpga-sw-dev
In-Reply-To: <1481639526-71743-1-git-send-email-ilyal@mellanox.com>
On 12/13/16 04:32 PM, Ilya Lesokhin wrote:
> --- a/arch/x86/crypto/aesni-intel_glue.c
> +++ b/arch/x86/crypto/aesni-intel_glue.c
> @@ -903,9 +903,11 @@ static int helper_rfc4106_encrypt(struct aead_request *req)
> *((__be32 *)(iv+12)) = counter;
>
> if (sg_is_last(req->src) &&
> - req->src->offset + req->src->length <= PAGE_SIZE &&
> + (!PageHighMem(sg_page(req->src)) ||
> + req->src->offset + req->src->length <= PAGE_SIZE) &&
> sg_is_last(req->dst) &&
> - req->dst->offset + req->dst->length <= PAGE_SIZE) {
> + (!PageHighMem(sg_page(req->dst)) ||
> + req->dst->offset + req->dst->length <= PAGE_SIZE)) {
> one_entry_in_sg = 1;
> scatterwalk_start(&src_sg_walk, req->src);
> assoc = scatterwalk_map(&src_sg_walk);
I was also experimenting with a similar patch that loosened up the
restrictions here, checking for highmem. Note that you can go even
further and check the AAD, data, and TAG all separately, the current
aesni crypto routines take them as separate buffers. (This might fix
the RFC5288 patch AAD size issue?)
Long term it would be nice to improve the asm routines instead to
support scatter / gather IO and any AAD len, as the newer intel
routines do:
https://github.com/01org/isa-l_crypto/tree/master/aes
^ permalink raw reply
* Re: Remaining crypto API regressions with CONFIG_VMAP_STACK
From: Andy Lutomirski @ 2016-12-13 17:06 UTC (permalink / raw)
To: Herbert Xu
Cc: Eric Biggers, linux-crypto, linux-kernel@vger.kernel.org,
linux-mm@kvack.org, kernel-hardening@lists.openwall.com,
Andrew Lutomirski, Stephan Mueller
In-Reply-To: <20161213033928.GB5601@gondor.apana.org.au>
On Mon, Dec 12, 2016 at 7:39 PM, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Mon, Dec 12, 2016 at 10:34:10AM -0800, Andy Lutomirski wrote:
>>
>> Here's my status.
>>
>> > drivers/crypto/bfin_crc.c:351
>> > drivers/crypto/qce/sha.c:299
>> > drivers/crypto/sahara.c:973,988
>> > drivers/crypto/talitos.c:1910
>> > drivers/crypto/qce/sha.c:325
>>
>> I have a patch to make these depend on !VMAP_STACK.
>
> Why? They're all marked as ASYNC AFAIK.
>
>> I have a patch to convert this to, drumroll please:
>>
>> priv->tx_tfm_mic = crypto_alloc_shash("michael_mic", 0,
>> CRYPTO_ALG_ASYNC);
>>
>> Herbert, I'm at a loss as what a "shash" that's "ASYNC" even means.
>
> Having 0 as type and CRYPTO_ALG_ASYNC as mask in general means
> that we're requesting a sync algorithm (i.e., ASYNC bit off).
>
> However, it is completely unnecessary for shash as they can never
> be async. So this could be changed to just ("michael_mic", 0, 0).
I'm confused by a bunch of this.
1. Is it really the case that crypto_alloc_xyz(..., CRYPTO_ALG_ASYNC)
means to allocate a *synchronous* transform? That's not what I
expected.
2. What guarantees that an async request is never allocated on the
stack? If it's just convention, could an assertion be added
somewhere?
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] orinoco: Use shash instead of ahash for MIC calculations
From: Kalle Valo @ 2016-12-13 17:03 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Andy Lutomirski, linux-kernel@vger.kernel.org, USB list,
Linux Wireless List, Eric Biggers,
linux-crypto-u79uwXL29TY76Z2rM5mHXA, Herbert Xu, Stephan Mueller
In-Reply-To: <CALCETrXxQ9FxuqV5A1rkj2SpeFfd89njDP9h5VBuNx387ieKdQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> writes:
> On Tue, Dec 13, 2016 at 3:35 AM, Kalle Valo <kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
>> Andy Lutomirski <luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> writes:
>>
>>> Eric Biggers pointed out that the orinoco driver pointed scatterlists
>>> at the stack.
>>>
>>> Fix it by switching from ahash to shash. The result should be
>>> simpler, faster, and more correct.
>>>
>>> Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org # 4.9 only
>>> Reported-by: Eric Biggers <ebiggers3-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> Signed-off-by: Andy Lutomirski <luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>
>> "more correct"? Does this fix a real user visible bug or what? And why
>> just stable 4.9, does this maybe have something to do with
>> CONFIG_VMAP_STACK?
>
> Whoops, I had that text in some other patches but forgot to put it in
> this one. It'll blow up with CONFIG_VMAP_STACK=y if a debug option
> like CONFIG_DEBUG_VIRTUAL=y is set. It may work by accident if
> debugging is off.
Makes sense now, thanks. I'll add that to the commit log and queue this
to 4.10.
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH] keys/encrypted: Fix two crypto-on-the-stack bugs
From: Andy Lutomirski @ 2016-12-13 17:02 UTC (permalink / raw)
To: David Howells
Cc: David Laight, Joerg Roedel, David Woodhouse, Linus Torvalds,
Ingo Molnar, Andy Lutomirski, linux-kernel@vger.kernel.org,
linux-usb@vger.kernel.org, keyrings@vger.kernel.org, Eric Biggers,
linux-crypto@vger.kernel.org, Herbert Xu, Stephan Mueller
In-Reply-To: <2661.1481647538@warthog.procyon.org.uk>
On Tue, Dec 13, 2016 at 8:45 AM, David Howells <dhowells@redhat.com> wrote:
> Andy Lutomirski <luto@amacapital.net> wrote:
>
>> After all, rodata is ordinary memory, is backed by struct page, etc.
>
> Is that actually true? I thought some arches excluded the kernel image from
> the page struct array to make the array consume less memory.
I don't know whether you're right, but that sounds a bit silly to me.
This is a *tiny* amount of memory.
But there's yet another snag. Alpha doesn't have empty_zero_page --
it only has ZERO_PAGE. I could do page_address(ZERO_PAGE(0))...
--Andy
^ permalink raw reply
* Re: [PATCH] keys/encrypted: Fix two crypto-on-the-stack bugs
From: David Howells @ 2016-12-13 16:45 UTC (permalink / raw)
To: Andy Lutomirski
Cc: dhowells, David Laight, Joerg Roedel, David Woodhouse,
Linus Torvalds, Ingo Molnar, Andy Lutomirski,
linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
keyrings@vger.kernel.org, Eric Biggers,
linux-crypto@vger.kernel.org, Herbert Xu, Stephan Mueller
In-Reply-To: <CALCETrWsTKq0NOpwiJtB50OU7w99-m82NhPG_Uxs2Fqbpz0LLA@mail.gmail.com>
Andy Lutomirski <luto@amacapital.net> wrote:
> After all, rodata is ordinary memory, is backed by struct page, etc.
Is that actually true? I thought some arches excluded the kernel image from
the page struct array to make the array consume less memory.
David
^ permalink raw reply
* Re: [PATCH] orinoco: Use shash instead of ahash for MIC calculations
From: Andy Lutomirski @ 2016-12-13 16:41 UTC (permalink / raw)
To: Kalle Valo
Cc: Andy Lutomirski, linux-kernel@vger.kernel.org, USB list,
Linux Wireless List, Eric Biggers, linux-crypto, Herbert Xu,
Stephan Mueller
In-Reply-To: <87mvg0kqno.fsf@purkki.adurom.net>
On Tue, Dec 13, 2016 at 3:35 AM, Kalle Valo <kvalo@codeaurora.org> wrote:
> Andy Lutomirski <luto@kernel.org> writes:
>
>> Eric Biggers pointed out that the orinoco driver pointed scatterlists
>> at the stack.
>>
>> Fix it by switching from ahash to shash. The result should be
>> simpler, faster, and more correct.
>>
>> Cc: stable@vger.kernel.org # 4.9 only
>> Reported-by: Eric Biggers <ebiggers3@gmail.com>
>> Signed-off-by: Andy Lutomirski <luto@kernel.org>
>
> "more correct"? Does this fix a real user visible bug or what? And why
> just stable 4.9, does this maybe have something to do with
> CONFIG_VMAP_STACK?
Whoops, I had that text in some other patches but forgot to put it in
this one. It'll blow up with CONFIG_VMAP_STACK=y if a debug option
like CONFIG_DEBUG_VIRTUAL=y is set. It may work by accident if
debugging is off.
--Andy
^ permalink raw reply
* Re: [PATCH] keys/encrypted: Fix two crypto-on-the-stack bugs
From: Andy Lutomirski @ 2016-12-13 16:40 UTC (permalink / raw)
To: David Laight, Joerg Roedel, David Woodhouse, Linus Torvalds,
Ingo Molnar
Cc: Andy Lutomirski, linux-kernel@vger.kernel.org,
linux-usb@vger.kernel.org, dhowells@redhat.com,
keyrings@vger.kernel.org, Eric Biggers,
linux-crypto@vger.kernel.org, Herbert Xu, Stephan Mueller
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB023CA99@AcuExch.aculab.com>
[add some people who might know]
On Tue, Dec 13, 2016 at 4:20 AM, David Laight <David.Laight@aculab.com> wrote:
> From: Andy Lutomirski
>> Sent: 12 December 2016 20:53
>> The driver put a constant buffer of all zeros on the stack and
>> pointed a scatterlist entry at it in two places. This doesn't work
>> with virtual stacks. Use a static 16-byte buffer of zeros instead.
> ...
>
> I didn't think you could dma from static data either.
According to lib/dma-debug.c, you can't dma to or from kernel text or
rodata, but you can dma to or from kernel bss or data. So
empty_zero_page should be okay, because it's not rodata right now.
But I think this is rather silly. Joerg, Linus, etc: would it be okay
to change lib/dma-debug.c to allow DMA *from* rodata? After all,
rodata is ordinary memory, is backed by struct page, etc. And DMA
from the zero page had better be okay because I think it happens if
you mmap some zeros, don't write to them, and then direct I/O them to
a device. Then I could also move empty_zero_page to rodata.
--Andy
^ permalink raw reply
* Re: [PATCH v6 2/2] crypto: add virtio-crypto driver
From: Halil Pasic @ 2016-12-13 16:11 UTC (permalink / raw)
To: Gonglei (Arei)
Cc: virtio-dev@lists.oasis-open.org, Xuquan (Quan Xu),
Huangweidong (C), Herbert Xu, Michael S. Tsirkin, Claudio Fontana,
Hanweidong (Randy), Luonengjun, qemu-devel@nongnu.org,
Wanzongshun (Vincent), linux-kernel@vger.kernel.org,
linux-crypto@vger.kernel.org, stefanha@redhat.com,
Zhoujian (jay, Euler), longpeng,
virtualization@lists.linux-foundation.org, davem@davemloft.net,
"Wubin \(H\)" <
In-Reply-To: <20161212234941-mutt-send-email-mst@kernel.org>
On 12/12/2016 11:05 PM, Michael S. Tsirkin wrote:
> On Mon, Dec 12, 2016 at 06:54:07PM +0800, Herbert Xu wrote:
>> On Mon, Dec 12, 2016 at 06:25:12AM +0000, Gonglei (Arei) wrote:
>>> Hi, Michael & Herbert
>>>
>>> Because the virtio-crypto device emulation had been in QEMU 2.8,
>>> would you please merge the virtio-crypto driver for 4.10 if no other
>>> comments? If so, Miachel pls ack and/or review the patch, then
>>> Herbert will take it (I asked him last week). Thank you!
>>>
>>> Ps: Note on 4.10 merge window timing from Linus
>>> https://lkml.org/lkml/2016/12/7/506
>>>
>>> Dec 23rd is the deadline for 4.10 merge window.
>>
>> Sorry but it's too late for 4.10. It needed to have been in my
>> tree before the merge window opened to make it for this cycle.
>>
>> Cheers,
>
>
> Objections to me merging this? I'm preparing my tree right now.
Got this when testing the most recent version on s390x
[ 20.391074] test 0 (128 bit key, 16 byte blocks): [ 20.391078] BUG: using smp_processor_id() in preemptible [00000000] code: insmod/97
[ 20.391082] caller is virtio_crypto_ablkcipher_setkey+0x44/0x198
[ 20.391085] CPU: 0 PID: 97 Comm: insmod Not tainted 4.9.0-02683-gb62a1ab #46
[ 20.391088] Hardware name: IBM 2964 NC9 704 (KVM)
[ 20.391405] Stack:
[ 20.391407] 000000000c0eb6d0 000000000c0eb760 0000000000000003 0000000000000000
[ 20.391414] 000000000c0eb800 000000000c0eb778 000000000c0eb778 0000000000000020
[ 20.391420] 0000000000000000 000000000000000a 0000000000000020 000003ff0000000a
[ 20.391426] 000003ff0000000c 000000000c0eb7c8 0000000000000000 0000000000000000
[ 20.391432] 0700000000c173c8 00000000001126ba 000000000c0eb760 000000000c0eb7b8
[ 20.391439] Call Trace:
[ 20.391442] ([<000000000011259e>] show_trace+0x8e/0xe0)
[ 20.391446] [<0000000000112670>] show_stack+0x80/0xd8
[ 20.391449] [<0000000000753ab6>] dump_stack+0x96/0xd8
[ 20.391453] [<00000000007872e6>] check_preemption_disabled+0xfe/0x128
[ 20.391456] [<0000000000839cc4>] virtio_crypto_ablkcipher_setkey+0x44/0x198
[ 20.391459] [<0000000000705a40>] skcipher_setkey_ablkcipher+0x50/0x70
[ 20.391476] [<000003ff80002a48>] test_skcipher_speed+0x328/0xb98 [tcrypt]
[ 20.391492] [<000003ff800063dc>] do_test+0x1c24/0x28e0 [tcrypt]
[ 20.391509] [<000003ff8001006a>] tcrypt_mod_init+0x6a/0x1000 [tcrypt]
[ 20.391512] [<00000000001002cc>] do_one_initcall+0xb4/0x148
[ 20.391515] [<0000000000298632>] do_init_module+0x7a/0x228
[ 20.391519] [<00000000001fd380>] load_module+0x2428/0x2de0
[ 20.391522] [<00000000001fde8a>] SyS_init_module+0x152/0x160
[ 20.391526] [<00000000009f1306>] system_call+0xd6/0x270
[ 20.391528] no locks held by insmod/97.
Gonglei, any idea? Did not look into it myself yet.
Halil
>
>> --
>> 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
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