2019-03-31 20:05:59

by Eric Biggers

[permalink] [raw]
Subject: [RFC/RFT PATCH 00/18] crypto: fuzz algorithms against their generic implementation

Hello,

In the crypto API, all implementations of each algorithm are supposed to
produce the same results. However, testing of this is currently limited
to the list of test vectors hardcoded for each algorithm. Although
after recent improvements the self-tests do much more with each test
vector, hardcoded test vectors can never cover all cases.

This series improves the situation by making the self-tests
automatically generate random test vectors using the corresponding
generic implementation, then run them against the algorithm under test.
This detects bugs where the implementations don't match.

These new fuzz tests are behind CONFIG_CRYPTO_MANAGER_EXTRA_TESTS.

As usual, the series begins with fixes for the bugs found, roughly in
order of decreasing importance. Please consider sending the Poly1305
fix to Linus soon.

Patches 12-17 make the testmgr improvements.

Patch 18 makes all templates and generic implementations be registered
earlier so that they're available when optimized implementations are
being tested, when both are built-in. Note that even after this, for
many algorithms it's still possible to make the generic implementation
unset or modular. Thus a missing generic implementation just causes the
comparison tests to be skipped with a warning; they aren't failed.

Also as usual, I've only tested all generic, x86, arm, and arm64
software algorithms. I encourage people to run the tests on drivers and
other architectures, as they will find more bugs.

This series can also be found in git at:

URL: https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux.git
Branch: cryptofuzz-vs-generic

Eric Biggers (18):
crypto: x86/poly1305 - fix overflow during partial reduction
crypto: crct10dif-generic - fix use via crypto_shash_digest()
crypto: x86/crct10dif-pcl - fix use via crypto_shash_digest()
crypto: skcipher - restore default skcipher_walk::iv on error
crypto: skcipher - don't WARN on unprocessed data after slow walk step
crypto: chacha20poly1305 - set cra_name correctly
crypto: gcm - fix incompatibility between "gcm" and "gcm_base"
crypto: ccm - fix incompatibility between "ccm" and "ccm_base"
crypto: streebog - fix unaligned memory accesses
crypto: cts - don't support empty messages
crypto: arm64/cbcmac - handle empty messages in same way as template
crypto: testmgr - expand ability to test for errors
crypto: testmgr - identify test vectors by name rather than number
crypto: testmgr - add helpers for fuzzing against generic
implementation
crypto: testmgr - fuzz hashes against their generic implementation
crypto: testmgr - fuzz skciphers against their generic implementation
crypto: testmgr - fuzz AEADs against their generic implementation
crypto: run initcalls for generic implementations earlier

arch/arm64/crypto/aes-glue.c | 2 +-
arch/x86/crypto/crct10dif-pclmul_glue.c | 13 +-
arch/x86/crypto/poly1305-avx2-x86_64.S | 14 +-
arch/x86/crypto/poly1305-sse2-x86_64.S | 22 +-
crypto/842.c | 2 +-
crypto/Makefile | 7 +-
crypto/adiantum.c | 2 +-
crypto/aegis128.c | 2 +-
crypto/aegis128l.c | 2 +-
crypto/aegis256.c | 2 +-
crypto/aes_generic.c | 2 +-
crypto/ansi_cprng.c | 2 +-
crypto/anubis.c | 2 +-
crypto/arc4.c | 2 +-
crypto/authenc.c | 2 +-
crypto/authencesn.c | 2 +-
crypto/blowfish_generic.c | 2 +-
crypto/camellia_generic.c | 2 +-
crypto/cast5_generic.c | 2 +-
crypto/cast6_generic.c | 2 +-
crypto/cbc.c | 2 +-
crypto/ccm.c | 40 +-
crypto/cfb.c | 2 +-
crypto/chacha20poly1305.c | 6 +-
crypto/chacha_generic.c | 2 +-
crypto/cmac.c | 2 +-
crypto/crc32_generic.c | 2 +-
crypto/crc32c_generic.c | 2 +-
crypto/crct10dif_generic.c | 13 +-
crypto/crypto_null.c | 2 +-
crypto/ctr.c | 2 +-
crypto/cts.c | 20 +-
crypto/deflate.c | 2 +-
crypto/des_generic.c | 2 +-
crypto/dh.c | 2 +-
crypto/drbg.c | 2 +-
crypto/ecb.c | 2 +-
crypto/ecdh.c | 2 +-
crypto/echainiv.c | 2 +-
crypto/fcrypt.c | 2 +-
crypto/fips.c | 2 +-
crypto/gcm.c | 32 +-
crypto/ghash-generic.c | 2 +-
crypto/hmac.c | 2 +-
crypto/jitterentropy-kcapi.c | 2 +-
crypto/keywrap.c | 2 +-
crypto/khazad.c | 2 +-
crypto/lrw.c | 2 +-
crypto/lz4.c | 2 +-
crypto/lz4hc.c | 2 +-
crypto/lzo-rle.c | 2 +-
crypto/lzo.c | 2 +-
crypto/md4.c | 2 +-
crypto/md5.c | 2 +-
crypto/michael_mic.c | 2 +-
crypto/morus1280.c | 2 +-
crypto/morus640.c | 2 +-
crypto/nhpoly1305.c | 2 +-
crypto/ofb.c | 2 +-
crypto/pcbc.c | 2 +-
crypto/poly1305_generic.c | 2 +-
crypto/rmd128.c | 2 +-
crypto/rmd160.c | 2 +-
crypto/rmd256.c | 2 +-
crypto/rmd320.c | 2 +-
crypto/rsa.c | 2 +-
crypto/salsa20_generic.c | 2 +-
crypto/seed.c | 2 +-
crypto/seqiv.c | 2 +-
crypto/serpent_generic.c | 2 +-
crypto/sha1_generic.c | 2 +-
crypto/sha256_generic.c | 2 +-
crypto/sha3_generic.c | 2 +-
crypto/sha512_generic.c | 2 +-
crypto/skcipher.c | 21 +-
crypto/sm3_generic.c | 2 +-
crypto/sm4_generic.c | 2 +-
crypto/streebog_generic.c | 27 +-
crypto/tcrypt.c | 2 +-
crypto/tea.c | 2 +-
crypto/testmgr.c | 978 +++++++++++++++++++++---
crypto/testmgr.h | 66 +-
crypto/tgr192.c | 2 +-
crypto/twofish_generic.c | 2 +-
crypto/vmac.c | 2 +-
crypto/wp512.c | 2 +-
crypto/xcbc.c | 2 +-
crypto/xts.c | 2 +-
crypto/zstd.c | 2 +-
include/crypto/streebog.h | 5 +-
90 files changed, 1115 insertions(+), 301 deletions(-)

--
2.21.0



2019-03-31 20:05:58

by Eric Biggers

[permalink] [raw]
Subject: [RFC/RFT PATCH 03/18] crypto: x86/crct10dif-pcl - fix use via crypto_shash_digest()

From: Eric Biggers <[email protected]>

The ->digest() method of crct10dif-pclmul reads the current CRC value
from the shash_desc context. But this value is uninitialized, causing
crypto_shash_digest() to compute the wrong result. Fix it.

Probably this wasn't noticed before because lib/crc-t10dif.c only uses
crypto_shash_update(), not crypto_shash_digest(). Likewise,
crypto_shash_digest() is not yet tested by the crypto self-tests because
those only test the ahash API which only uses shash init/update/final.

Fixes: 0b95a7f85718 ("crypto: crct10dif - Glue code to cast accelerated CRCT10DIF assembly as a crypto transform")
Cc: <[email protected]> # v3.11+
Cc: Tim Chen <[email protected]>
Signed-off-by: Eric Biggers <[email protected]>
---
arch/x86/crypto/crct10dif-pclmul_glue.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/arch/x86/crypto/crct10dif-pclmul_glue.c b/arch/x86/crypto/crct10dif-pclmul_glue.c
index 64f17d2d8b67a..3c81e15b0873f 100644
--- a/arch/x86/crypto/crct10dif-pclmul_glue.c
+++ b/arch/x86/crypto/crct10dif-pclmul_glue.c
@@ -71,15 +71,14 @@ static int chksum_final(struct shash_desc *desc, u8 *out)
return 0;
}

-static int __chksum_finup(__u16 *crcp, const u8 *data, unsigned int len,
- u8 *out)
+static int __chksum_finup(__u16 crc, const u8 *data, unsigned int len, u8 *out)
{
if (len >= 16 && crypto_simd_usable()) {
kernel_fpu_begin();
- *(__u16 *)out = crc_t10dif_pcl(*crcp, data, len);
+ *(__u16 *)out = crc_t10dif_pcl(crc, data, len);
kernel_fpu_end();
} else
- *(__u16 *)out = crc_t10dif_generic(*crcp, data, len);
+ *(__u16 *)out = crc_t10dif_generic(crc, data, len);
return 0;
}

@@ -88,15 +87,13 @@ static int chksum_finup(struct shash_desc *desc, const u8 *data,
{
struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);

- return __chksum_finup(&ctx->crc, data, len, out);
+ return __chksum_finup(ctx->crc, data, len, out);
}

static int chksum_digest(struct shash_desc *desc, const u8 *data,
unsigned int length, u8 *out)
{
- struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
-
- return __chksum_finup(&ctx->crc, data, length, out);
+ return __chksum_finup(0, data, length, out);
}

static struct shash_alg alg = {
--
2.21.0


2019-03-31 20:05:59

by Eric Biggers

[permalink] [raw]
Subject: [RFC/RFT PATCH 02/18] crypto: crct10dif-generic - fix use via crypto_shash_digest()

From: Eric Biggers <[email protected]>

The ->digest() method of crct10dif-generic reads the current CRC value
from the shash_desc context. But this value is uninitialized, causing
crypto_shash_digest() to compute the wrong result. Fix it.

Probably this wasn't noticed before because lib/crc-t10dif.c only uses
crypto_shash_update(), not crypto_shash_digest(). Likewise,
crypto_shash_digest() is not yet tested by the crypto self-tests because
those only test the ahash API which only uses shash init/update/final.

This bug was detected by my patches that improve testmgr to fuzz
algorithms against their generic implementation.

Fixes: 2d31e518a428 ("crypto: crct10dif - Wrap crc_t10dif function all to use crypto transform framework")
Cc: <[email protected]> # v3.11+
Cc: Tim Chen <[email protected]>
Signed-off-by: Eric Biggers <[email protected]>
---
crypto/crct10dif_generic.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/crypto/crct10dif_generic.c b/crypto/crct10dif_generic.c
index 8e94e29dc6fc8..d08048ae55527 100644
--- a/crypto/crct10dif_generic.c
+++ b/crypto/crct10dif_generic.c
@@ -65,10 +65,9 @@ static int chksum_final(struct shash_desc *desc, u8 *out)
return 0;
}

-static int __chksum_finup(__u16 *crcp, const u8 *data, unsigned int len,
- u8 *out)
+static int __chksum_finup(__u16 crc, const u8 *data, unsigned int len, u8 *out)
{
- *(__u16 *)out = crc_t10dif_generic(*crcp, data, len);
+ *(__u16 *)out = crc_t10dif_generic(crc, data, len);
return 0;
}

@@ -77,15 +76,13 @@ static int chksum_finup(struct shash_desc *desc, const u8 *data,
{
struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);

- return __chksum_finup(&ctx->crc, data, len, out);
+ return __chksum_finup(ctx->crc, data, len, out);
}

static int chksum_digest(struct shash_desc *desc, const u8 *data,
unsigned int length, u8 *out)
{
- struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
-
- return __chksum_finup(&ctx->crc, data, length, out);
+ return __chksum_finup(0, data, length, out);
}

static struct shash_alg alg = {
--
2.21.0


2019-03-31 20:05:59

by Eric Biggers

[permalink] [raw]
Subject: [RFC/RFT PATCH 05/18] crypto: skcipher - don't WARN on unprocessed data after slow walk step

From: Eric Biggers <[email protected]>

skcipher_walk_done() assumes it's a bug if, after the "slow" path is
executed where the next chunk of data is processed via a bounce buffer,
the algorithm says it didn't process all bytes. Thus it WARNs on this.

However, this can happen legitimately when the message needs to be
evenly divisible into "blocks" but isn't, and the algorithm has a
'walksize' greater than the block size. For example, ecb-aes-neonbs
sets 'walksize' to 128 bytes and only supports messages evenly divisible
into 16-byte blocks. If, say, 17 message bytes remain but they straddle
scatterlist elements, the skcipher_walk code will take the "slow" path
and pass the algorithm all 17 bytes in the bounce buffer. But the
algorithm will only be able to process 16 bytes, triggering the WARN.

Fix this by just removing the WARN_ON(). Returning -EINVAL, as the code
already does, is the right behavior.

This bug was detected by my patches that improve testmgr to fuzz
algorithms against their generic implementation.

Fixes: b286d8b1a690 ("crypto: skcipher - Add skcipher walk interface")
Cc: <[email protected]> # v4.10+
Signed-off-by: Eric Biggers <[email protected]>
---
crypto/skcipher.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/crypto/skcipher.c b/crypto/skcipher.c
index 0f639f018047d..85836be4f274f 100644
--- a/crypto/skcipher.c
+++ b/crypto/skcipher.c
@@ -131,8 +131,13 @@ int skcipher_walk_done(struct skcipher_walk *walk, int err)
memcpy(walk->dst.virt.addr, walk->page, n);
skcipher_unmap_dst(walk);
} else if (unlikely(walk->flags & SKCIPHER_WALK_SLOW)) {
- if (WARN_ON(err)) {
- /* unexpected case; didn't process all bytes */
+ if (err) {
+ /*
+ * Didn't process all bytes. Either the algorithm is
+ * broken, or this was the last step and it turned out
+ * the message wasn't evenly divisible into blocks but
+ * the algorithm requires it.
+ */
err = -EINVAL;
goto finish;
}
--
2.21.0


2019-03-31 20:06:01

by Eric Biggers

[permalink] [raw]
Subject: [RFC/RFT PATCH 04/18] crypto: skcipher - restore default skcipher_walk::iv on error

From: Eric Biggers <[email protected]>

When the user-provided IV buffer is not aligned to the algorithm's
alignmask, skcipher_walk_virt() allocates an aligned buffer and copies
the IV into it. However, skcipher_walk_virt() can fail after that
point, and in this case the buffer will be freed.

This causes a use-after-free read in callers that read from walk->iv
unconditionally, e.g. the LRW template. For example, this can be
reproduced by trying to encrypt fewer than 16 bytes using "lrw(aes)".

Fix it by making skcipher_walk_first() reset walk->iv to walk->oiv if
skcipher_walk_next() fails.

This addresses a similar problem as commit 2b4f27c36bcd ("crypto:
skcipher - set walk.iv for zero-length inputs"), but that didn't
consider the case where skcipher_walk_virt() fails after copying the IV.

This bug was detected by my patches that improve testmgr to fuzz
algorithms against their generic implementation, when the extra
self-tests were run on a KASAN-enabled kernel.

Fixes: b286d8b1a690 ("crypto: skcipher - Add skcipher walk interface")
Cc: <[email protected]> # v4.10+
Signed-off-by: Eric Biggers <[email protected]>
---
crypto/skcipher.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/crypto/skcipher.c b/crypto/skcipher.c
index bcf13d95f54ac..0f639f018047d 100644
--- a/crypto/skcipher.c
+++ b/crypto/skcipher.c
@@ -426,19 +426,27 @@ static int skcipher_copy_iv(struct skcipher_walk *walk)

static int skcipher_walk_first(struct skcipher_walk *walk)
{
+ int err;
+
if (WARN_ON_ONCE(in_irq()))
return -EDEADLK;

walk->buffer = NULL;
if (unlikely(((unsigned long)walk->iv & walk->alignmask))) {
- int err = skcipher_copy_iv(walk);
+ err = skcipher_copy_iv(walk);
if (err)
return err;
}

walk->page = NULL;

- return skcipher_walk_next(walk);
+ err = skcipher_walk_next(walk);
+
+ /* On error, don't leave 'walk->iv' pointing to freed buffer. */
+ if (unlikely(err))
+ walk->iv = walk->oiv;
+
+ return err;
}

static int skcipher_walk_skcipher(struct skcipher_walk *walk,
--
2.21.0


2019-03-31 20:06:01

by Eric Biggers

[permalink] [raw]
Subject: [RFC/RFT PATCH 01/18] crypto: x86/poly1305 - fix overflow during partial reduction

From: Eric Biggers <[email protected]>

The x86_64 implementation of Poly1305 produces the wrong result on some
inputs because poly1305_4block_avx2() incorrectly assumes that when
partially reducing the accumulator, the bits carried from limb 'd4' to
limb 'h0' fit in a 32-bit integer. This is true for poly1305-generic
which processes only one block at a time. However, it's not true for
the AVX2 implementation, which processes 4 blocks at a time and
therefore can produce intermediate limbs about 4x larger.

Fix it by making the relevant calculations use 64-bit arithmetic rather
than 32-bit. Note that most of the carries already used 64-bit
arithmetic, but the d4 -> h0 carry was different for some reason.

To be safe I also made the same change to the corresponding SSE2 code,
though that only operates on 1 or 2 blocks at a time. I don't think
it's really needed for poly1305_block_sse2(), but it doesn't hurt
because it's already x86_64 code. It *might* be needed for
poly1305_2block_sse2(), but overflows aren't easy to reproduce there.

This bug was originally detected by my patches that improve testmgr to
fuzz algorithms against their generic implementation. But also add a
test vector which reproduces it directly (in the AVX2 case).

Fixes: b1ccc8f4b631 ("crypto: poly1305 - Add a four block AVX2 variant for x86_64")
Fixes: c70f4abef07a ("crypto: poly1305 - Add a SSE2 SIMD variant for x86_64")
Cc: <[email protected]> # v4.3+
Cc: Martin Willi <[email protected]>
Cc: Jason A. Donenfeld <[email protected]>
Signed-off-by: Eric Biggers <[email protected]>
---
arch/x86/crypto/poly1305-avx2-x86_64.S | 14 +++++---
arch/x86/crypto/poly1305-sse2-x86_64.S | 22 ++++++++-----
crypto/testmgr.h | 44 +++++++++++++++++++++++++-
3 files changed, 67 insertions(+), 13 deletions(-)

diff --git a/arch/x86/crypto/poly1305-avx2-x86_64.S b/arch/x86/crypto/poly1305-avx2-x86_64.S
index 3b6e70d085da8..8457cdd47f751 100644
--- a/arch/x86/crypto/poly1305-avx2-x86_64.S
+++ b/arch/x86/crypto/poly1305-avx2-x86_64.S
@@ -323,6 +323,12 @@ ENTRY(poly1305_4block_avx2)
vpaddq t2,t1,t1
vmovq t1x,d4

+ # Now do a partial reduction mod (2^130)-5, carrying h0 -> h1 -> h2 ->
+ # h3 -> h4 -> h0 -> h1 to get h0,h2,h3,h4 < 2^26 and h1 < 2^26 + a small
+ # amount. Careful: we must not assume the carry bits 'd0 >> 26',
+ # 'd1 >> 26', 'd2 >> 26', 'd3 >> 26', and '(d4 >> 26) * 5' fit in 32-bit
+ # integers. It's true in a single-block implementation, but not here.
+
# d1 += d0 >> 26
mov d0,%rax
shr $26,%rax
@@ -361,16 +367,16 @@ ENTRY(poly1305_4block_avx2)
# h0 += (d4 >> 26) * 5
mov d4,%rax
shr $26,%rax
- lea (%eax,%eax,4),%eax
- add %eax,%ebx
+ lea (%rax,%rax,4),%rax
+ add %rax,%rbx
# h4 = d4 & 0x3ffffff
mov d4,%rax
and $0x3ffffff,%eax
mov %eax,h4

# h1 += h0 >> 26
- mov %ebx,%eax
- shr $26,%eax
+ mov %rbx,%rax
+ shr $26,%rax
add %eax,h1
# h0 = h0 & 0x3ffffff
andl $0x3ffffff,%ebx
diff --git a/arch/x86/crypto/poly1305-sse2-x86_64.S b/arch/x86/crypto/poly1305-sse2-x86_64.S
index e6add74d78a59..6f0be7a869641 100644
--- a/arch/x86/crypto/poly1305-sse2-x86_64.S
+++ b/arch/x86/crypto/poly1305-sse2-x86_64.S
@@ -253,16 +253,16 @@ ENTRY(poly1305_block_sse2)
# h0 += (d4 >> 26) * 5
mov d4,%rax
shr $26,%rax
- lea (%eax,%eax,4),%eax
- add %eax,%ebx
+ lea (%rax,%rax,4),%rax
+ add %rax,%rbx
# h4 = d4 & 0x3ffffff
mov d4,%rax
and $0x3ffffff,%eax
mov %eax,h4

# h1 += h0 >> 26
- mov %ebx,%eax
- shr $26,%eax
+ mov %rbx,%rax
+ shr $26,%rax
add %eax,h1
# h0 = h0 & 0x3ffffff
andl $0x3ffffff,%ebx
@@ -524,6 +524,12 @@ ENTRY(poly1305_2block_sse2)
paddq t2,t1
movq t1,d4

+ # Now do a partial reduction mod (2^130)-5, carrying h0 -> h1 -> h2 ->
+ # h3 -> h4 -> h0 -> h1 to get h0,h2,h3,h4 < 2^26 and h1 < 2^26 + a small
+ # amount. Careful: we must not assume the carry bits 'd0 >> 26',
+ # 'd1 >> 26', 'd2 >> 26', 'd3 >> 26', and '(d4 >> 26) * 5' fit in 32-bit
+ # integers. It's true in a single-block implementation, but not here.
+
# d1 += d0 >> 26
mov d0,%rax
shr $26,%rax
@@ -562,16 +568,16 @@ ENTRY(poly1305_2block_sse2)
# h0 += (d4 >> 26) * 5
mov d4,%rax
shr $26,%rax
- lea (%eax,%eax,4),%eax
- add %eax,%ebx
+ lea (%rax,%rax,4),%rax
+ add %rax,%rbx
# h4 = d4 & 0x3ffffff
mov d4,%rax
and $0x3ffffff,%eax
mov %eax,h4

# h1 += h0 >> 26
- mov %ebx,%eax
- shr $26,%eax
+ mov %rbx,%rax
+ shr $26,%rax
add %eax,h1
# h0 = h0 & 0x3ffffff
andl $0x3ffffff,%ebx
diff --git a/crypto/testmgr.h b/crypto/testmgr.h
index f267633cf13ac..d18a37629f053 100644
--- a/crypto/testmgr.h
+++ b/crypto/testmgr.h
@@ -5634,7 +5634,49 @@ static const struct hash_testvec poly1305_tv_template[] = {
.psize = 80,
.digest = "\x13\x00\x00\x00\x00\x00\x00\x00"
"\x00\x00\x00\x00\x00\x00\x00\x00",
- },
+ }, { /* Regression test for overflow in AVX2 implementation */
+ .plaintext = "\xff\xff\xff\xff\xff\xff\xff\xff"
+ "\xff\xff\xff\xff\xff\xff\xff\xff"
+ "\xff\xff\xff\xff\xff\xff\xff\xff"
+ "\xff\xff\xff\xff\xff\xff\xff\xff"
+ "\xff\xff\xff\xff\xff\xff\xff\xff"
+ "\xff\xff\xff\xff\xff\xff\xff\xff"
+ "\xff\xff\xff\xff\xff\xff\xff\xff"
+ "\xff\xff\xff\xff\xff\xff\xff\xff"
+ "\xff\xff\xff\xff\xff\xff\xff\xff"
+ "\xff\xff\xff\xff\xff\xff\xff\xff"
+ "\xff\xff\xff\xff\xff\xff\xff\xff"
+ "\xff\xff\xff\xff\xff\xff\xff\xff"
+ "\xff\xff\xff\xff\xff\xff\xff\xff"
+ "\xff\xff\xff\xff\xff\xff\xff\xff"
+ "\xff\xff\xff\xff\xff\xff\xff\xff"
+ "\xff\xff\xff\xff\xff\xff\xff\xff"
+ "\xff\xff\xff\xff\xff\xff\xff\xff"
+ "\xff\xff\xff\xff\xff\xff\xff\xff"
+ "\xff\xff\xff\xff\xff\xff\xff\xff"
+ "\xff\xff\xff\xff\xff\xff\xff\xff"
+ "\xff\xff\xff\xff\xff\xff\xff\xff"
+ "\xff\xff\xff\xff\xff\xff\xff\xff"
+ "\xff\xff\xff\xff\xff\xff\xff\xff"
+ "\xff\xff\xff\xff\xff\xff\xff\xff"
+ "\xff\xff\xff\xff\xff\xff\xff\xff"
+ "\xff\xff\xff\xff\xff\xff\xff\xff"
+ "\xff\xff\xff\xff\xff\xff\xff\xff"
+ "\xff\xff\xff\xff\xff\xff\xff\xff"
+ "\xff\xff\xff\xff\xff\xff\xff\xff"
+ "\xff\xff\xff\xff\xff\xff\xff\xff"
+ "\xff\xff\xff\xff\xff\xff\xff\xff"
+ "\xff\xff\xff\xff\xff\xff\xff\xff"
+ "\xff\xff\xff\xff\xff\xff\xff\xff"
+ "\xff\xff\xff\xff\xff\xff\xff\xff"
+ "\xff\xff\xff\xff\xff\xff\xff\xff"
+ "\xff\xff\xff\xff\xff\xff\xff\xff"
+ "\xff\xff\xff\xff\xff\xff\xff\xff"
+ "\xff\xff\xff\xff",
+ .psize = 300,
+ .digest = "\xfb\x5e\x96\xd8\x61\xd5\xc7\xc8"
+ "\x78\xe5\x87\xcc\x2d\x5a\x22\xe1",
+ }
};

/* NHPoly1305 test vectors from https://github.com/google/adiantum */
--
2.21.0


2019-03-31 20:06:01

by Eric Biggers

[permalink] [raw]
Subject: [RFC/RFT PATCH 07/18] crypto: gcm - fix incompatibility between "gcm" and "gcm_base"

From: Eric Biggers <[email protected]>

GCM instances can be created by either the "gcm" template, which only
allows choosing the block cipher, e.g. "gcm(aes)"; or by "gcm_base",
which allows choosing the ctr and ghash implementations, e.g.
"gcm_base(ctr(aes-generic),ghash-generic)".

However, a "gcm_base" instance prevents a "gcm" instance from being
registered using the same implementations. Nor will the instance be
found by lookups of "gcm". This can be used as a denial of service.
Moreover, "gcm_base" instances are never tested by the crypto
self-tests, even if there are compatible "gcm" tests.

The root cause of these problems is that instances of the two templates
use different cra_names. Therefore, fix these problems by making
"gcm_base" instances set the same cra_name as "gcm" instances, e.g.
"gcm(aes)" instead of "gcm_base(ctr(aes-generic),ghash-generic)".

This requires extracting the block cipher name from the name of the ctr
algorithm, which means starting to require that the stream cipher really
be ctr and not something else. But it would be pretty bizarre if anyone
was actually relying on being able to use a non-ctr stream cipher here.

Fixes: d00aa19b507b ("[CRYPTO] gcm: Allow block cipher parameter")
Cc: [email protected]
Signed-off-by: Eric Biggers <[email protected]>
---
crypto/gcm.c | 30 ++++++++----------------------
1 file changed, 8 insertions(+), 22 deletions(-)

diff --git a/crypto/gcm.c b/crypto/gcm.c
index e1a11f529d257..12ff5ed569272 100644
--- a/crypto/gcm.c
+++ b/crypto/gcm.c
@@ -597,7 +597,6 @@ static void crypto_gcm_free(struct aead_instance *inst)

static int crypto_gcm_create_common(struct crypto_template *tmpl,
struct rtattr **tb,
- const char *full_name,
const char *ctr_name,
const char *ghash_name)
{
@@ -650,24 +649,23 @@ static int crypto_gcm_create_common(struct crypto_template *tmpl,

ctr = crypto_spawn_skcipher_alg(&ctx->ctr);

- /* We only support 16-byte blocks. */
+ /* Must be CTR mode with 16-byte blocks. */
err = -EINVAL;
- if (crypto_skcipher_alg_ivsize(ctr) != 16)
+ if (strncmp(ctr->base.cra_name, "ctr(", 4) != 0 ||
+ crypto_skcipher_alg_ivsize(ctr) != 16)
goto out_put_ctr;

- /* Not a stream cipher? */
- if (ctr->base.cra_blocksize != 1)
+ err = -ENAMETOOLONG;
+ if (snprintf(inst->alg.base.cra_name, CRYPTO_MAX_ALG_NAME,
+ "gcm(%s", ctr->base.cra_name + 4) >= CRYPTO_MAX_ALG_NAME)
goto out_put_ctr;

- err = -ENAMETOOLONG;
if (snprintf(inst->alg.base.cra_driver_name, CRYPTO_MAX_ALG_NAME,
"gcm_base(%s,%s)", ctr->base.cra_driver_name,
ghash_alg->cra_driver_name) >=
CRYPTO_MAX_ALG_NAME)
goto out_put_ctr;

- memcpy(inst->alg.base.cra_name, full_name, CRYPTO_MAX_ALG_NAME);
-
inst->alg.base.cra_flags = (ghash->base.cra_flags |
ctr->base.cra_flags) & CRYPTO_ALG_ASYNC;
inst->alg.base.cra_priority = (ghash->base.cra_priority +
@@ -709,7 +707,6 @@ static int crypto_gcm_create(struct crypto_template *tmpl, struct rtattr **tb)
{
const char *cipher_name;
char ctr_name[CRYPTO_MAX_ALG_NAME];
- char full_name[CRYPTO_MAX_ALG_NAME];

cipher_name = crypto_attr_alg_name(tb[1]);
if (IS_ERR(cipher_name))
@@ -719,12 +716,7 @@ static int crypto_gcm_create(struct crypto_template *tmpl, struct rtattr **tb)
CRYPTO_MAX_ALG_NAME)
return -ENAMETOOLONG;

- if (snprintf(full_name, CRYPTO_MAX_ALG_NAME, "gcm(%s)", cipher_name) >=
- CRYPTO_MAX_ALG_NAME)
- return -ENAMETOOLONG;
-
- return crypto_gcm_create_common(tmpl, tb, full_name,
- ctr_name, "ghash");
+ return crypto_gcm_create_common(tmpl, tb, ctr_name, "ghash");
}

static int crypto_gcm_base_create(struct crypto_template *tmpl,
@@ -732,7 +724,6 @@ static int crypto_gcm_base_create(struct crypto_template *tmpl,
{
const char *ctr_name;
const char *ghash_name;
- char full_name[CRYPTO_MAX_ALG_NAME];

ctr_name = crypto_attr_alg_name(tb[1]);
if (IS_ERR(ctr_name))
@@ -742,12 +733,7 @@ static int crypto_gcm_base_create(struct crypto_template *tmpl,
if (IS_ERR(ghash_name))
return PTR_ERR(ghash_name);

- if (snprintf(full_name, CRYPTO_MAX_ALG_NAME, "gcm_base(%s,%s)",
- ctr_name, ghash_name) >= CRYPTO_MAX_ALG_NAME)
- return -ENAMETOOLONG;
-
- return crypto_gcm_create_common(tmpl, tb, full_name,
- ctr_name, ghash_name);
+ return crypto_gcm_create_common(tmpl, tb, ctr_name, ghash_name);
}

static int crypto_rfc4106_setkey(struct crypto_aead *parent, const u8 *key,
--
2.21.0


2019-03-31 20:06:03

by Eric Biggers

[permalink] [raw]
Subject: [RFC/RFT PATCH 16/18] crypto: testmgr - fuzz skciphers against their generic implementation

From: Eric Biggers <[email protected]>

When the extra crypto self-tests are enabled, test each skcipher
algorithm against its generic implementation when one is available.
This involves: checking the algorithm properties for consistency, then
randomly generating test vectors using the generic implementation and
running them against the implementation under test. Both bad and good
inputs are tested.

This has already detected bugs in the skcipher_walk API, including one
that caused a use-after free read in the lrw template and other places.
It also detected an inconsistency in the cts implementations.

Signed-off-by: Eric Biggers <[email protected]>
---
crypto/testmgr.c | 197 +++++++++++++++++++++++++++++++++++++++++++++++
crypto/testmgr.h | 2 +-
2 files changed, 198 insertions(+), 1 deletion(-)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 6f5e85b3d17db..3fe178e32b0fe 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -2113,6 +2113,186 @@ static int test_skcipher_vec(const char *driver, int enc,
return 0;
}

+#ifdef CONFIG_CRYPTO_MANAGER_EXTRA_TESTS
+/*
+ * Generate a symmetric cipher test vector from the given implementation.
+ * Assumes the buffers in 'vec' were already allocated.
+ */
+static void generate_random_cipher_testvec(struct skcipher_request *req,
+ struct cipher_testvec *vec,
+ unsigned int maxdatasize,
+ char *name, size_t max_namelen)
+{
+ struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
+ const unsigned int maxkeysize = tfm->keysize;
+ const unsigned int ivsize = crypto_skcipher_ivsize(tfm);
+ struct scatterlist src, dst;
+ u8 iv[MAX_IVLEN];
+ DECLARE_CRYPTO_WAIT(wait);
+
+ /* Key: length in [0, maxkeysize], but usually choose maxkeysize */
+ vec->klen = maxkeysize;
+ if (prandom_u32() % 4 == 0)
+ vec->klen = prandom_u32() % (maxkeysize + 1);
+ generate_random_bytes((u8 *)vec->key, vec->klen);
+ vec->setkey_error = crypto_skcipher_setkey(tfm, vec->key, vec->klen);
+
+ /* IV */
+ generate_random_bytes((u8 *)vec->iv, ivsize);
+
+ /* Plaintext */
+ vec->len = generate_random_length(maxdatasize);
+ generate_random_bytes((u8 *)vec->ptext, vec->len);
+
+ /* If the key couldn't be set, no need to continue to encrypt. */
+ if (vec->setkey_error)
+ goto done;
+
+ /* Ciphertext */
+ sg_init_one(&src, vec->ptext, vec->len);
+ sg_init_one(&dst, vec->ctext, vec->len);
+ memcpy(iv, vec->iv, ivsize);
+ skcipher_request_set_callback(req, 0, crypto_req_done, &wait);
+ skcipher_request_set_crypt(req, &src, &dst, vec->len, iv);
+ vec->crypt_error = crypto_wait_req(crypto_skcipher_encrypt(req), &wait);
+done:
+ snprintf(name, max_namelen, "\"random: len=%u klen=%u\"",
+ vec->len, vec->klen);
+}
+
+/*
+ * Test the skcipher algorithm represented by @req against the corresponding
+ * generic implementation, if one is available.
+ */
+static int test_skcipher_vs_generic_impl(const char *driver,
+ const char *generic_driver,
+ struct skcipher_request *req,
+ struct cipher_test_sglists *tsgls)
+{
+ struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
+ const unsigned int ivsize = crypto_skcipher_ivsize(tfm);
+ const unsigned int blocksize = crypto_skcipher_blocksize(tfm);
+ const unsigned int maxdatasize = (2 * PAGE_SIZE) - TESTMGR_POISON_LEN;
+ const char *algname = tfm->base.__crt_alg->cra_name;
+ char _generic_driver[CRYPTO_MAX_ALG_NAME];
+ struct crypto_skcipher *generic_tfm = NULL;
+ struct skcipher_request *generic_req = NULL;
+ unsigned int i;
+ struct cipher_testvec vec = { 0 };
+ char vec_name[64];
+ struct testvec_config cfg;
+ char cfgname[TESTVEC_CONFIG_NAMELEN];
+ int err;
+
+ if (noextratests)
+ return 0;
+
+ /* Keywrap isn't supported here yet as it handles its IV differently. */
+ if (strncmp(algname, "kw(", 3) == 0)
+ return 0;
+
+ if (!generic_driver) { /* Use default naming convention? */
+ err = build_generic_driver_name(algname, _generic_driver);
+ if (err)
+ return err;
+ generic_driver = _generic_driver;
+ }
+
+ if (strcmp(generic_driver, driver) == 0) /* Already the generic impl? */
+ return 0;
+
+ generic_tfm = crypto_alloc_skcipher(generic_driver, 0, 0);
+ if (IS_ERR(generic_tfm)) {
+ err = PTR_ERR(generic_tfm);
+ if (err == -ENOENT) {
+ pr_warn("alg: skcipher: skipping comparison tests for %s because %s is unavailable\n",
+ driver, generic_driver);
+ return 0;
+ }
+ pr_err("alg: skcipher: error allocating %s (generic impl of %s): %d\n",
+ generic_driver, algname, err);
+ return err;
+ }
+
+ generic_req = skcipher_request_alloc(generic_tfm, GFP_KERNEL);
+ if (!generic_req) {
+ err = -ENOMEM;
+ goto out;
+ }
+
+ /* Check the algorithm properties for consistency. */
+
+ if (tfm->keysize != generic_tfm->keysize) {
+ pr_err("alg: skcipher: max keysize for %s (%u) doesn't match generic impl (%u)\n",
+ driver, tfm->keysize, generic_tfm->keysize);
+ err = -EINVAL;
+ goto out;
+ }
+
+ if (ivsize != crypto_skcipher_ivsize(generic_tfm)) {
+ pr_err("alg: skcipher: ivsize for %s (%u) doesn't match generic impl (%u)\n",
+ driver, ivsize, crypto_skcipher_ivsize(generic_tfm));
+ err = -EINVAL;
+ goto out;
+ }
+
+ if (blocksize != crypto_skcipher_blocksize(generic_tfm)) {
+ pr_err("alg: skcipher: blocksize for %s (%u) doesn't match generic impl (%u)\n",
+ driver, blocksize,
+ crypto_skcipher_blocksize(generic_tfm));
+ err = -EINVAL;
+ goto out;
+ }
+
+ /*
+ * Now generate test vectors using the generic implementation, and test
+ * the other implementation against them.
+ */
+
+ vec.key = kmalloc(tfm->keysize, GFP_KERNEL);
+ vec.iv = kmalloc(ivsize, GFP_KERNEL);
+ vec.ptext = kmalloc(maxdatasize, GFP_KERNEL);
+ vec.ctext = kmalloc(maxdatasize, GFP_KERNEL);
+ if (!vec.key || !vec.iv || !vec.ptext || !vec.ctext) {
+ err = -ENOMEM;
+ goto out;
+ }
+
+ for (i = 0; i < fuzz_iterations * 8; i++) {
+ generate_random_cipher_testvec(generic_req, &vec, maxdatasize,
+ vec_name, sizeof(vec_name));
+ generate_random_testvec_config(&cfg, cfgname, sizeof(cfgname));
+
+ err = test_skcipher_vec_cfg(driver, ENCRYPT, &vec, vec_name,
+ &cfg, req, tsgls);
+ if (err)
+ goto out;
+ err = test_skcipher_vec_cfg(driver, DECRYPT, &vec, vec_name,
+ &cfg, req, tsgls);
+ if (err)
+ goto out;
+ cond_resched();
+ }
+ err = 0;
+out:
+ kfree(vec.key);
+ kfree(vec.iv);
+ kfree(vec.ptext);
+ kfree(vec.ctext);
+ crypto_free_skcipher(generic_tfm);
+ skcipher_request_free(generic_req);
+ return err;
+}
+#else /* !CONFIG_CRYPTO_MANAGER_EXTRA_TESTS */
+static int test_skcipher_vs_generic_impl(const char *driver,
+ const char *generic_driver,
+ struct skcipher_request *req,
+ struct cipher_test_sglists *tsgls)
+{
+ return 0;
+}
+#endif /* !CONFIG_CRYPTO_MANAGER_EXTRA_TESTS */
+
static int test_skcipher(const char *driver, int enc,
const struct cipher_test_suite *suite,
struct skcipher_request *req,
@@ -2172,6 +2352,11 @@ static int alg_test_skcipher(const struct alg_test_desc *desc,
goto out;

err = test_skcipher(driver, DECRYPT, suite, req, tsgls);
+ if (err)
+ goto out;
+
+ err = test_skcipher_vs_generic_impl(driver, desc->generic_driver, req,
+ tsgls);
out:
free_cipher_test_sglists(tsgls);
skcipher_request_free(req);
@@ -3127,12 +3312,14 @@ static int alg_test_null(const struct alg_test_desc *desc,
static const struct alg_test_desc alg_test_descs[] = {
{
.alg = "adiantum(xchacha12,aes)",
+ .generic_driver = "adiantum(xchacha12-generic,aes-generic,nhpoly1305-generic)",
.test = alg_test_skcipher,
.suite = {
.cipher = __VECS(adiantum_xchacha12_aes_tv_template)
},
}, {
.alg = "adiantum(xchacha20,aes)",
+ .generic_driver = "adiantum(xchacha20-generic,aes-generic,nhpoly1305-generic)",
.test = alg_test_skcipher,
.suite = {
.cipher = __VECS(adiantum_xchacha20_aes_tv_template)
@@ -3905,30 +4092,35 @@ static const struct alg_test_desc alg_test_descs[] = {
}
}, {
.alg = "lrw(aes)",
+ .generic_driver = "lrw(ecb(aes-generic))",
.test = alg_test_skcipher,
.suite = {
.cipher = __VECS(aes_lrw_tv_template)
}
}, {
.alg = "lrw(camellia)",
+ .generic_driver = "lrw(ecb(camellia-generic))",
.test = alg_test_skcipher,
.suite = {
.cipher = __VECS(camellia_lrw_tv_template)
}
}, {
.alg = "lrw(cast6)",
+ .generic_driver = "lrw(ecb(cast6-generic))",
.test = alg_test_skcipher,
.suite = {
.cipher = __VECS(cast6_lrw_tv_template)
}
}, {
.alg = "lrw(serpent)",
+ .generic_driver = "lrw(ecb(serpent-generic))",
.test = alg_test_skcipher,
.suite = {
.cipher = __VECS(serpent_lrw_tv_template)
}
}, {
.alg = "lrw(twofish)",
+ .generic_driver = "lrw(ecb(twofish-generic))",
.test = alg_test_skcipher,
.suite = {
.cipher = __VECS(tf_lrw_tv_template)
@@ -4263,6 +4455,7 @@ static const struct alg_test_desc alg_test_descs[] = {
},
}, {
.alg = "xts(aes)",
+ .generic_driver = "xts(ecb(aes-generic))",
.test = alg_test_skcipher,
.fips_allowed = 1,
.suite = {
@@ -4270,12 +4463,14 @@ static const struct alg_test_desc alg_test_descs[] = {
}
}, {
.alg = "xts(camellia)",
+ .generic_driver = "xts(ecb(camellia-generic))",
.test = alg_test_skcipher,
.suite = {
.cipher = __VECS(camellia_xts_tv_template)
}
}, {
.alg = "xts(cast6)",
+ .generic_driver = "xts(ecb(cast6-generic))",
.test = alg_test_skcipher,
.suite = {
.cipher = __VECS(cast6_xts_tv_template)
@@ -4289,12 +4484,14 @@ static const struct alg_test_desc alg_test_descs[] = {
.fips_allowed = 1,
}, {
.alg = "xts(serpent)",
+ .generic_driver = "xts(ecb(serpent-generic))",
.test = alg_test_skcipher,
.suite = {
.cipher = __VECS(serpent_xts_tv_template)
}
}, {
.alg = "xts(twofish)",
+ .generic_driver = "xts(ecb(twofish-generic))",
.test = alg_test_skcipher,
.suite = {
.cipher = __VECS(tf_xts_tv_template)
diff --git a/crypto/testmgr.h b/crypto/testmgr.h
index beb9bd79fe1fa..e03f4f72723b1 100644
--- a/crypto/testmgr.h
+++ b/crypto/testmgr.h
@@ -71,7 +71,7 @@ struct cipher_testvec {
const char *ptext;
const char *ctext;
unsigned char wk; /* weak key flag */
- unsigned char klen;
+ unsigned short klen;
unsigned short len;
bool fips_skip;
bool generates_iv;
--
2.21.0


2019-03-31 20:06:02

by Eric Biggers

[permalink] [raw]
Subject: [RFC/RFT PATCH 15/18] crypto: testmgr - fuzz hashes against their generic implementation

From: Eric Biggers <[email protected]>

When the extra crypto self-tests are enabled, test each hash algorithm
against its generic implementation when one is available. This
involves: checking the algorithm properties for consistency, then
randomly generating test vectors using the generic implementation and
running them against the implementation under test. Both bad and good
inputs are tested.

This has already detected a bug in the x86 implementation of poly1305,
bugs in crct10dif, and an inconsistency in cbcmac.

Signed-off-by: Eric Biggers <[email protected]>
---
crypto/testmgr.c | 174 +++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 170 insertions(+), 4 deletions(-)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index edbb5001cad58..6f5e85b3d17db 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -1286,9 +1286,169 @@ static int test_hash_vec(const char *driver, const struct hash_testvec *vec,
return 0;
}

+#ifdef CONFIG_CRYPTO_MANAGER_EXTRA_TESTS
+/*
+ * Generate a hash test vector from the given implementation.
+ * Assumes the buffers in 'vec' were already allocated.
+ */
+static void generate_random_hash_testvec(struct crypto_shash *tfm,
+ struct hash_testvec *vec,
+ unsigned int maxkeysize,
+ unsigned int maxdatasize,
+ char *name, size_t max_namelen)
+{
+ SHASH_DESC_ON_STACK(desc, tfm);
+
+ /* Data */
+ vec->psize = generate_random_length(maxdatasize);
+ generate_random_bytes((u8 *)vec->plaintext, vec->psize);
+
+ /*
+ * Key: length in range [1, maxkeysize], but usually choose maxkeysize.
+ * If algorithm is unkeyed, then maxkeysize == 0 and set ksize = 0.
+ */
+ vec->setkey_error = 0;
+ vec->ksize = 0;
+ if (maxkeysize) {
+ vec->ksize = maxkeysize;
+ if (prandom_u32() % 4 == 0)
+ vec->ksize = 1 + (prandom_u32() % maxkeysize);
+ generate_random_bytes((u8 *)vec->key, vec->ksize);
+
+ vec->setkey_error = crypto_shash_setkey(tfm, vec->key,
+ vec->ksize);
+ /* If the key couldn't be set, no need to continue to digest. */
+ if (vec->setkey_error)
+ goto done;
+ }
+
+ /* Digest */
+ desc->tfm = tfm;
+ desc->flags = 0;
+ vec->digest_error = crypto_shash_digest(desc, vec->plaintext,
+ vec->psize, (u8 *)vec->digest);
+done:
+ snprintf(name, max_namelen, "\"random: psize=%u ksize=%u\"",
+ vec->psize, vec->ksize);
+}
+
+/*
+ * Test the hash algorithm represented by @req against the corresponding generic
+ * implementation, if one is available.
+ */
+static int test_hash_vs_generic_impl(const char *driver,
+ const char *generic_driver,
+ unsigned int maxkeysize,
+ struct ahash_request *req,
+ struct test_sglist *tsgl,
+ u8 *hashstate)
+{
+ struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
+ const unsigned int digestsize = crypto_ahash_digestsize(tfm);
+ const unsigned int blocksize = crypto_ahash_blocksize(tfm);
+ const unsigned int maxdatasize = (2 * PAGE_SIZE) - TESTMGR_POISON_LEN;
+ const char *algname = tfm->base.__crt_alg->cra_name;
+ char _generic_driver[CRYPTO_MAX_ALG_NAME];
+ struct crypto_shash *generic_tfm = NULL;
+ unsigned int i;
+ struct hash_testvec vec = { 0 };
+ char vec_name[64];
+ struct testvec_config cfg;
+ char cfgname[TESTVEC_CONFIG_NAMELEN];
+ int err;
+
+ if (noextratests)
+ return 0;
+
+ if (!generic_driver) { /* Use default naming convention? */
+ err = build_generic_driver_name(algname, _generic_driver);
+ if (err)
+ return err;
+ generic_driver = _generic_driver;
+ }
+
+ if (strcmp(generic_driver, driver) == 0) /* Already the generic impl? */
+ return 0;
+
+ generic_tfm = crypto_alloc_shash(generic_driver, 0, 0);
+ if (IS_ERR(generic_tfm)) {
+ err = PTR_ERR(generic_tfm);
+ if (err == -ENOENT) {
+ pr_warn("alg: hash: skipping comparison tests for %s because %s is unavailable\n",
+ driver, generic_driver);
+ return 0;
+ }
+ pr_err("alg: hash: error allocating %s (generic impl of %s): %d\n",
+ generic_driver, algname, err);
+ return err;
+ }
+
+ /* Check the algorithm properties for consistency. */
+
+ if (digestsize != crypto_shash_digestsize(generic_tfm)) {
+ pr_err("alg: hash: digestsize for %s (%u) doesn't match generic impl (%u)\n",
+ driver, digestsize,
+ crypto_shash_digestsize(generic_tfm));
+ err = -EINVAL;
+ goto out;
+ }
+
+ if (blocksize != crypto_shash_blocksize(generic_tfm)) {
+ pr_err("alg: hash: blocksize for %s (%u) doesn't match generic impl (%u)\n",
+ driver, blocksize, crypto_shash_blocksize(generic_tfm));
+ err = -EINVAL;
+ goto out;
+ }
+
+ /*
+ * Now generate test vectors using the generic implementation, and test
+ * the other implementation against them.
+ */
+
+ vec.key = kmalloc(maxkeysize, GFP_KERNEL);
+ vec.plaintext = kmalloc(maxdatasize, GFP_KERNEL);
+ vec.digest = kmalloc(digestsize, GFP_KERNEL);
+ if (!vec.key || !vec.plaintext || !vec.digest) {
+ err = -ENOMEM;
+ goto out;
+ }
+
+ for (i = 0; i < fuzz_iterations * 8; i++) {
+ generate_random_hash_testvec(generic_tfm, &vec,
+ maxkeysize, maxdatasize,
+ vec_name, sizeof(vec_name));
+ generate_random_testvec_config(&cfg, cfgname, sizeof(cfgname));
+
+ err = test_hash_vec_cfg(driver, &vec, vec_name, &cfg,
+ req, tsgl, hashstate);
+ if (err)
+ goto out;
+ cond_resched();
+ }
+ err = 0;
+out:
+ kfree(vec.key);
+ kfree(vec.plaintext);
+ kfree(vec.digest);
+ crypto_free_shash(generic_tfm);
+ return err;
+}
+#else /* !CONFIG_CRYPTO_MANAGER_EXTRA_TESTS */
+static int test_hash_vs_generic_impl(const char *driver,
+ const char *generic_driver,
+ unsigned int maxkeysize,
+ struct ahash_request *req,
+ struct test_sglist *tsgl,
+ u8 *hashstate)
+{
+ return 0;
+}
+#endif /* !CONFIG_CRYPTO_MANAGER_EXTRA_TESTS */
+
static int __alg_test_hash(const struct hash_testvec *vecs,
unsigned int num_vecs, const char *driver,
- u32 type, u32 mask)
+ u32 type, u32 mask,
+ const char *generic_driver, unsigned int maxkeysize)
{
struct crypto_ahash *tfm;
struct ahash_request *req = NULL;
@@ -1336,7 +1496,8 @@ static int __alg_test_hash(const struct hash_testvec *vecs,
if (err)
goto out;
}
- err = 0;
+ err = test_hash_vs_generic_impl(driver, generic_driver, maxkeysize, req,
+ tsgl, hashstate);
out:
kfree(hashstate);
if (tsgl) {
@@ -1354,6 +1515,7 @@ static int alg_test_hash(const struct alg_test_desc *desc, const char *driver,
const struct hash_testvec *template = desc->suite.hash.vecs;
unsigned int tcount = desc->suite.hash.count;
unsigned int nr_unkeyed, nr_keyed;
+ unsigned int maxkeysize = 0;
int err;

/*
@@ -1372,16 +1534,20 @@ static int alg_test_hash(const struct alg_test_desc *desc, const char *driver,
"unkeyed ones must come first\n", desc->alg);
return -EINVAL;
}
+ maxkeysize = max_t(unsigned int, maxkeysize,
+ template[nr_unkeyed + nr_keyed].ksize);
}

err = 0;
if (nr_unkeyed) {
- err = __alg_test_hash(template, nr_unkeyed, driver, type, mask);
+ err = __alg_test_hash(template, nr_unkeyed, driver, type, mask,
+ desc->generic_driver, maxkeysize);
template += nr_unkeyed;
}

if (!err && nr_keyed)
- err = __alg_test_hash(template, nr_keyed, driver, type, mask);
+ err = __alg_test_hash(template, nr_keyed, driver, type, mask,
+ desc->generic_driver, maxkeysize);

return err;
}
--
2.21.0


2019-03-31 20:06:03

by Eric Biggers

[permalink] [raw]
Subject: [RFC/RFT PATCH 14/18] crypto: testmgr - add helpers for fuzzing against generic implementation

From: Eric Biggers <[email protected]>

Add some helper functions in preparation for fuzz testing algorithms
against their generic implementation.

Signed-off-by: Eric Biggers <[email protected]>
---
crypto/testmgr.c | 128 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 128 insertions(+)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index fcdb891b9b357..edbb5001cad58 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -125,6 +125,7 @@ struct kpp_test_suite {

struct alg_test_desc {
const char *alg;
+ const char *generic_driver;
int (*test)(const struct alg_test_desc *desc, const char *driver,
u32 type, u32 mask);
int fips_allowed; /* set if alg is allowed in fips mode */
@@ -742,6 +743,91 @@ static int build_cipher_test_sglists(struct cipher_test_sglists *tsgls,
}

#ifdef CONFIG_CRYPTO_MANAGER_EXTRA_TESTS
+
+/* Generate a random length in range [0, max_len], but prefer smaller values */
+static unsigned int generate_random_length(unsigned int max_len)
+{
+ unsigned int len = prandom_u32() % (max_len + 1);
+
+ switch (prandom_u32() % 4) {
+ case 0:
+ return len % 64;
+ case 1:
+ return len % 256;
+ case 2:
+ return len % 1024;
+ default:
+ return len;
+ }
+}
+
+/* Sometimes make some random changes to the given data buffer */
+static void mutate_buffer(u8 *buf, size_t count)
+{
+ size_t num_flips;
+ size_t i;
+ size_t pos;
+
+ /* Sometimes flip some bits */
+ if (prandom_u32() % 4 == 0) {
+ num_flips = min_t(size_t, 1 << (prandom_u32() % 8), count * 8);
+ for (i = 0; i < num_flips; i++) {
+ pos = prandom_u32() % (count * 8);
+ buf[pos / 8] ^= 1 << (pos % 8);
+ }
+ }
+
+ /* Sometimes flip some bytes */
+ if (prandom_u32() % 4 == 0) {
+ num_flips = min_t(size_t, 1 << (prandom_u32() % 8), count);
+ for (i = 0; i < num_flips; i++)
+ buf[prandom_u32() % count] ^= 0xff;
+ }
+}
+
+/* Randomly generate 'count' bytes, but sometimes make them "interesting" */
+static void generate_random_bytes(u8 *buf, size_t count)
+{
+ u8 b;
+ int increment;
+ size_t i;
+
+ if (count == 0)
+ return;
+
+ switch (prandom_u32() % 8) { /* Choose a generation strategy */
+ case 0:
+ case 1:
+ /* All the same byte, plus optional mutations */
+ switch (prandom_u32() % 4) {
+ case 0:
+ b = 0x00;
+ break;
+ case 1:
+ b = 0xff;
+ break;
+ default:
+ b = (u8)prandom_u32();
+ break;
+ }
+ memset(buf, b, count);
+ mutate_buffer(buf, count);
+ break;
+ case 2:
+ /* Ascending or descending bytes, plus optional mutations */
+ increment = (int)(u8)prandom_u32() - 128;
+ b = (u8)prandom_u32();
+ for (i = 0; i < count; i++, b += increment)
+ buf[i] = b;
+ mutate_buffer(buf, count);
+ break;
+ default:
+ /* Fully random bytes */
+ for (i = 0; i < count; i++)
+ buf[i] = (u8)prandom_u32();
+ }
+}
+
static char *generate_random_sgl_divisions(struct test_sg_division *divs,
size_t max_divs, char *p, char *end,
bool gen_flushes, u32 req_flags)
@@ -896,6 +982,48 @@ static void crypto_reenable_simd_for_test(void)
__this_cpu_write(crypto_simd_disabled_for_test, false);
preempt_enable();
}
+
+/*
+ * Given an algorithm name, build the name of the generic implementation of that
+ * algorithm, assuming the usual naming convention. Specifically, this appends
+ * "-generic" to every part of the name that is not a template name. Examples:
+ *
+ * aes => aes-generic
+ * cbc(aes) => cbc(aes-generic)
+ * cts(cbc(aes)) => cts(cbc(aes-generic))
+ * rfc7539(chacha20,poly1305) => rfc7539(chacha20-generic,poly1305-generic)
+ *
+ * Return: 0 on success, or -ENAMETOOLONG if the generic name would be too long
+ */
+static int build_generic_driver_name(const char *algname,
+ char driver_name[CRYPTO_MAX_ALG_NAME])
+{
+ const char *in = algname;
+ char *out = driver_name;
+ size_t len = strlen(algname);
+
+ if (len >= CRYPTO_MAX_ALG_NAME)
+ goto too_long;
+ do {
+ const char *in_saved = in;
+
+ while (*in && *in != '(' && *in != ')' && *in != ',')
+ *out++ = *in++;
+ if (*in != '(' && in > in_saved) {
+ len += 8;
+ if (len >= CRYPTO_MAX_ALG_NAME)
+ goto too_long;
+ memcpy(out, "-generic", 8);
+ out += 8;
+ }
+ } while ((*out++ = *in++) != '\0');
+ return 0;
+
+too_long:
+ pr_err("alg: generic driver name for \"%s\" would be too long\n",
+ algname);
+ return -ENAMETOOLONG;
+}
#else /* !CONFIG_CRYPTO_MANAGER_EXTRA_TESTS */
static void crypto_disable_simd_for_test(void)
{
--
2.21.0


2019-03-31 20:06:03

by Eric Biggers

[permalink] [raw]
Subject: [RFC/RFT PATCH 13/18] crypto: testmgr - identify test vectors by name rather than number

From: Eric Biggers <[email protected]>

In preparation for fuzz testing algorithms against their generic
implementation, make error messages in testmgr identify test vectors by
name rather than index. Built-in test vectors are simply "named" by
their index in testmgr.h, as before. But (in later patches) generated
test vectors will be given more descriptive names to help developers
debug problems detected with them.

Signed-off-by: Eric Biggers <[email protected]>
---
crypto/testmgr.c | 183 +++++++++++++++++++++++++----------------------
1 file changed, 96 insertions(+), 87 deletions(-)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index f81043c76cf55..fcdb891b9b357 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -925,17 +925,17 @@ static int do_ahash_op(int (*op)(struct ahash_request *req),

static int check_nonfinal_hash_op(const char *op, int err,
u8 *result, unsigned int digestsize,
- const char *driver, unsigned int vec_num,
+ const char *driver, const char *vec_name,
const struct testvec_config *cfg)
{
if (err) {
- pr_err("alg: hash: %s %s() failed with err %d on test vector %u, cfg=\"%s\"\n",
- driver, op, err, vec_num, cfg->name);
+ pr_err("alg: hash: %s %s() failed with err %d on test vector %s, cfg=\"%s\"\n",
+ driver, op, err, vec_name, cfg->name);
return err;
}
if (!testmgr_is_poison(result, digestsize)) {
- pr_err("alg: hash: %s %s() used result buffer on test vector %u, cfg=\"%s\"\n",
- driver, op, vec_num, cfg->name);
+ pr_err("alg: hash: %s %s() used result buffer on test vector %s, cfg=\"%s\"\n",
+ driver, op, vec_name, cfg->name);
return -EINVAL;
}
return 0;
@@ -943,7 +943,7 @@ static int check_nonfinal_hash_op(const char *op, int err,

static int test_hash_vec_cfg(const char *driver,
const struct hash_testvec *vec,
- unsigned int vec_num,
+ const char *vec_name,
const struct testvec_config *cfg,
struct ahash_request *req,
struct test_sglist *tsgl,
@@ -970,14 +970,14 @@ static int test_hash_vec_cfg(const char *driver,
if (err) {
if (err == vec->setkey_error)
return 0;
- pr_err("alg: hash: %s setkey failed with err %d on test vector %u; flags=%#x\n",
- driver, err, vec_num,
+ pr_err("alg: hash: %s setkey failed with err %d on test vector %s; flags=%#x\n",
+ driver, err, vec_name,
crypto_ahash_get_flags(tfm));
return err;
}
if (vec->setkey_error) {
- pr_err("alg: hash: %s setkey unexpectedly succeeded on test vector %u\n",
- driver, vec_num);
+ pr_err("alg: hash: %s setkey unexpectedly succeeded on test vector %s\n",
+ driver, vec_name);
return -EINVAL;
}
}
@@ -989,8 +989,8 @@ static int test_hash_vec_cfg(const char *driver,
err = build_test_sglist(tsgl, cfg->src_divs, alignmask, vec->psize,
&input, divs);
if (err) {
- pr_err("alg: hash: %s: error preparing scatterlist for test vector %u, cfg=\"%s\"\n",
- driver, vec_num, cfg->name);
+ pr_err("alg: hash: %s: error preparing scatterlist for test vector %s, cfg=\"%s\"\n",
+ driver, vec_name, cfg->name);
return err;
}

@@ -1009,13 +1009,13 @@ static int test_hash_vec_cfg(const char *driver,
if (err) {
if (err == vec->digest_error)
return 0;
- pr_err("alg: hash: %s digest() failed with err %d on test vector %u, cfg=\"%s\"\n",
- driver, err, vec_num, cfg->name);
+ pr_err("alg: hash: %s digest() failed with err %d on test vector %s, cfg=\"%s\"\n",
+ driver, err, vec_name, cfg->name);
return err;
}
if (vec->digest_error) {
- pr_err("alg: hash: %s digest() unexpectedly succeeded on test vector %u, cfg=\"%s\"\n",
- driver, vec_num, cfg->name);
+ pr_err("alg: hash: %s digest() unexpectedly succeeded on test vector %s, cfg=\"%s\"\n",
+ driver, vec_name, cfg->name);
return -EINVAL;
}
goto result_ready;
@@ -1027,7 +1027,7 @@ static int test_hash_vec_cfg(const char *driver,
ahash_request_set_crypt(req, NULL, result, 0);
err = do_ahash_op(crypto_ahash_init, req, &wait, cfg->nosimd);
err = check_nonfinal_hash_op("init", err, result, digestsize,
- driver, vec_num, cfg);
+ driver, vec_name, cfg);
if (err)
return err;

@@ -1045,7 +1045,7 @@ static int test_hash_vec_cfg(const char *driver,
divs[i]->nosimd);
err = check_nonfinal_hash_op("update", err,
result, digestsize,
- driver, vec_num, cfg);
+ driver, vec_name, cfg);
if (err)
return err;
pending_sgl = NULL;
@@ -1058,13 +1058,13 @@ static int test_hash_vec_cfg(const char *driver,
err = crypto_ahash_export(req, hashstate);
err = check_nonfinal_hash_op("export", err,
result, digestsize,
- driver, vec_num, cfg);
+ driver, vec_name, cfg);
if (err)
return err;
if (!testmgr_is_poison(hashstate + statesize,
TESTMGR_POISON_LEN)) {
- pr_err("alg: hash: %s export() overran state buffer on test vector %u, cfg=\"%s\"\n",
- driver, vec_num, cfg->name);
+ pr_err("alg: hash: %s export() overran state buffer on test vector %s, cfg=\"%s\"\n",
+ driver, vec_name, cfg->name);
return -EOVERFLOW;
}

@@ -1072,7 +1072,7 @@ static int test_hash_vec_cfg(const char *driver,
err = crypto_ahash_import(req, hashstate);
err = check_nonfinal_hash_op("import", err,
result, digestsize,
- driver, vec_num, cfg);
+ driver, vec_name, cfg);
if (err)
return err;
}
@@ -1087,21 +1087,21 @@ static int test_hash_vec_cfg(const char *driver,
/* finish with update() and final() */
err = do_ahash_op(crypto_ahash_update, req, &wait, cfg->nosimd);
err = check_nonfinal_hash_op("update", err, result, digestsize,
- driver, vec_num, cfg);
+ driver, vec_name, cfg);
if (err)
return err;
err = do_ahash_op(crypto_ahash_final, req, &wait, cfg->nosimd);
if (err) {
- pr_err("alg: hash: %s final() failed with err %d on test vector %u, cfg=\"%s\"\n",
- driver, err, vec_num, cfg->name);
+ pr_err("alg: hash: %s final() failed with err %d on test vector %s, cfg=\"%s\"\n",
+ driver, err, vec_name, cfg->name);
return err;
}
} else {
/* finish with finup() */
err = do_ahash_op(crypto_ahash_finup, req, &wait, cfg->nosimd);
if (err) {
- pr_err("alg: hash: %s finup() failed with err %d on test vector %u, cfg=\"%s\"\n",
- driver, err, vec_num, cfg->name);
+ pr_err("alg: hash: %s finup() failed with err %d on test vector %s, cfg=\"%s\"\n",
+ driver, err, vec_name, cfg->name);
return err;
}
}
@@ -1109,13 +1109,13 @@ static int test_hash_vec_cfg(const char *driver,
result_ready:
/* Check that the algorithm produced the correct digest */
if (memcmp(result, vec->digest, digestsize) != 0) {
- pr_err("alg: hash: %s test failed (wrong result) on test vector %u, cfg=\"%s\"\n",
- driver, vec_num, cfg->name);
+ pr_err("alg: hash: %s test failed (wrong result) on test vector %s, cfg=\"%s\"\n",
+ driver, vec_name, cfg->name);
return -EINVAL;
}
if (!testmgr_is_poison(&result[digestsize], TESTMGR_POISON_LEN)) {
- pr_err("alg: hash: %s overran result buffer on test vector %u, cfg=\"%s\"\n",
- driver, vec_num, cfg->name);
+ pr_err("alg: hash: %s overran result buffer on test vector %s, cfg=\"%s\"\n",
+ driver, vec_name, cfg->name);
return -EOVERFLOW;
}

@@ -1126,11 +1126,14 @@ static int test_hash_vec(const char *driver, const struct hash_testvec *vec,
unsigned int vec_num, struct ahash_request *req,
struct test_sglist *tsgl, u8 *hashstate)
{
+ char vec_name[16];
unsigned int i;
int err;

+ sprintf(vec_name, "%u", vec_num);
+
for (i = 0; i < ARRAY_SIZE(default_hash_testvec_configs); i++) {
- err = test_hash_vec_cfg(driver, vec, vec_num,
+ err = test_hash_vec_cfg(driver, vec, vec_name,
&default_hash_testvec_configs[i],
req, tsgl, hashstate);
if (err)
@@ -1145,7 +1148,7 @@ static int test_hash_vec(const char *driver, const struct hash_testvec *vec,
for (i = 0; i < fuzz_iterations; i++) {
generate_random_testvec_config(&cfg, cfgname,
sizeof(cfgname));
- err = test_hash_vec_cfg(driver, vec, vec_num, &cfg,
+ err = test_hash_vec_cfg(driver, vec, vec_name, &cfg,
req, tsgl, hashstate);
if (err)
return err;
@@ -1257,7 +1260,7 @@ static int alg_test_hash(const struct alg_test_desc *desc, const char *driver,

static int test_aead_vec_cfg(const char *driver, int enc,
const struct aead_testvec *vec,
- unsigned int vec_num,
+ const char *vec_name,
const struct testvec_config *cfg,
struct aead_request *req,
struct cipher_test_sglists *tsgls)
@@ -1283,26 +1286,26 @@ static int test_aead_vec_cfg(const char *driver, int enc,
crypto_aead_clear_flags(tfm, CRYPTO_TFM_REQ_FORBID_WEAK_KEYS);
err = crypto_aead_setkey(tfm, vec->key, vec->klen);
if (err && err != vec->setkey_error) {
- pr_err("alg: aead: %s setkey failed with err %d on test vector %u; flags=%#x\n",
- driver, err, vec_num, crypto_aead_get_flags(tfm));
+ pr_err("alg: aead: %s setkey failed with err %d on test vector %s; flags=%#x\n",
+ driver, err, vec_name, crypto_aead_get_flags(tfm));
return err;
}
if (!err && vec->setkey_error) {
- pr_err("alg: aead: %s setkey unexpectedly succeeded on test vector %u\n",
- driver, vec_num);
+ pr_err("alg: aead: %s setkey unexpectedly succeeded on test vector %s\n",
+ driver, vec_name);
return -EINVAL;
}

/* Set the authentication tag size */
err = crypto_aead_setauthsize(tfm, authsize);
if (err && err != vec->setauthsize_error) {
- pr_err("alg: aead: %s setauthsize failed with err %d on test vector %u\n",
- driver, err, vec_num);
+ pr_err("alg: aead: %s setauthsize failed with err %d on test vector %s\n",
+ driver, err, vec_name);
return err;
}
if (!err && vec->setauthsize_error) {
- pr_err("alg: aead: %s setauthsize unexpectedly succeeded on test vector %u\n",
- driver, vec_num);
+ pr_err("alg: aead: %s setauthsize unexpectedly succeeded on test vector %s\n",
+ driver, vec_name);
return -EINVAL;
}

@@ -1329,8 +1332,8 @@ static int test_aead_vec_cfg(const char *driver, int enc,
vec->plen),
input, 2);
if (err) {
- pr_err("alg: aead: %s %s: error preparing scatterlists for test vector %u, cfg=\"%s\"\n",
- driver, op, vec_num, cfg->name);
+ pr_err("alg: aead: %s %s: error preparing scatterlists for test vector %s, cfg=\"%s\"\n",
+ driver, op, vec_name, cfg->name);
return err;
}

@@ -1357,8 +1360,8 @@ static int test_aead_vec_cfg(const char *driver, int enc,
req->base.complete != crypto_req_done ||
req->base.flags != req_flags ||
req->base.data != &wait) {
- pr_err("alg: aead: %s %s corrupted request struct on test vector %u, cfg=\"%s\"\n",
- driver, op, vec_num, cfg->name);
+ pr_err("alg: aead: %s %s corrupted request struct on test vector %s, cfg=\"%s\"\n",
+ driver, op, vec_name, cfg->name);
if (req->cryptlen != (enc ? vec->plen : vec->clen))
pr_err("alg: aead: changed 'req->cryptlen'\n");
if (req->assoclen != vec->alen)
@@ -1380,27 +1383,27 @@ static int test_aead_vec_cfg(const char *driver, int enc,
return -EINVAL;
}
if (is_test_sglist_corrupted(&tsgls->src)) {
- pr_err("alg: aead: %s %s corrupted src sgl on test vector %u, cfg=\"%s\"\n",
- driver, op, vec_num, cfg->name);
+ pr_err("alg: aead: %s %s corrupted src sgl on test vector %s, cfg=\"%s\"\n",
+ driver, op, vec_name, cfg->name);
return -EINVAL;
}
if (tsgls->dst.sgl_ptr != tsgls->src.sgl &&
is_test_sglist_corrupted(&tsgls->dst)) {
- pr_err("alg: aead: %s %s corrupted dst sgl on test vector %u, cfg=\"%s\"\n",
- driver, op, vec_num, cfg->name);
+ pr_err("alg: aead: %s %s corrupted dst sgl on test vector %s, cfg=\"%s\"\n",
+ driver, op, vec_name, cfg->name);
return -EINVAL;
}

if (err) {
if (err == vec->crypt_error || (err == -EBADMSG && vec->novrfy))
return 0;
- pr_err("alg: aead: %s %s failed with err %d on test vector %u, cfg=\"%s\"\n",
- driver, op, err, vec_num, cfg->name);
+ pr_err("alg: aead: %s %s failed with err %d on test vector %s, cfg=\"%s\"\n",
+ driver, op, err, vec_name, cfg->name);
return err;
}
if (vec->crypt_error || vec->novrfy) {
- pr_err("alg: aead: %s %s unexpectedly succeeded on test vector %u, cfg=\"%s\"\n",
- driver, op, vec_num, cfg->name);
+ pr_err("alg: aead: %s %s unexpectedly succeeded on test vector %s, cfg=\"%s\"\n",
+ driver, op, vec_name, cfg->name);
return -EINVAL;
}

@@ -1409,13 +1412,13 @@ static int test_aead_vec_cfg(const char *driver, int enc,
enc ? vec->clen : vec->plen,
vec->alen, enc || !cfg->inplace);
if (err == -EOVERFLOW) {
- pr_err("alg: aead: %s %s overran dst buffer on test vector %u, cfg=\"%s\"\n",
- driver, op, vec_num, cfg->name);
+ pr_err("alg: aead: %s %s overran dst buffer on test vector %s, cfg=\"%s\"\n",
+ driver, op, vec_name, cfg->name);
return err;
}
if (err) {
- pr_err("alg: aead: %s %s test failed (wrong result) on test vector %u, cfg=\"%s\"\n",
- driver, op, vec_num, cfg->name);
+ pr_err("alg: aead: %s %s test failed (wrong result) on test vector %s, cfg=\"%s\"\n",
+ driver, op, vec_name, cfg->name);
return err;
}

@@ -1427,14 +1430,17 @@ static int test_aead_vec(const char *driver, int enc,
struct aead_request *req,
struct cipher_test_sglists *tsgls)
{
+ char vec_name[16];
unsigned int i;
int err;

if (enc && vec->novrfy)
return 0;

+ sprintf(vec_name, "%u", vec_num);
+
for (i = 0; i < ARRAY_SIZE(default_cipher_testvec_configs); i++) {
- err = test_aead_vec_cfg(driver, enc, vec, vec_num,
+ err = test_aead_vec_cfg(driver, enc, vec, vec_name,
&default_cipher_testvec_configs[i],
req, tsgls);
if (err)
@@ -1449,7 +1455,7 @@ static int test_aead_vec(const char *driver, int enc,
for (i = 0; i < fuzz_iterations; i++) {
generate_random_testvec_config(&cfg, cfgname,
sizeof(cfgname));
- err = test_aead_vec_cfg(driver, enc, vec, vec_num,
+ err = test_aead_vec_cfg(driver, enc, vec, vec_name,
&cfg, req, tsgls);
if (err)
return err;
@@ -1613,7 +1619,7 @@ static int test_cipher(struct crypto_cipher *tfm, int enc,

static int test_skcipher_vec_cfg(const char *driver, int enc,
const struct cipher_testvec *vec,
- unsigned int vec_num,
+ const char *vec_name,
const struct testvec_config *cfg,
struct skcipher_request *req,
struct cipher_test_sglists *tsgls)
@@ -1641,13 +1647,13 @@ static int test_skcipher_vec_cfg(const char *driver, int enc,
if (err) {
if (err == vec->setkey_error)
return 0;
- pr_err("alg: skcipher: %s setkey failed with err %d on test vector %u; flags=%#x\n",
- driver, err, vec_num, crypto_skcipher_get_flags(tfm));
+ pr_err("alg: skcipher: %s setkey failed with err %d on test vector %s; flags=%#x\n",
+ driver, err, vec_name, crypto_skcipher_get_flags(tfm));
return err;
}
if (vec->setkey_error) {
- pr_err("alg: skcipher: %s setkey unexpectedly succeeded on test vector %u\n",
- driver, vec_num);
+ pr_err("alg: skcipher: %s setkey unexpectedly succeeded on test vector %s\n",
+ driver, vec_name);
return -EINVAL;
}

@@ -1663,8 +1669,8 @@ static int test_skcipher_vec_cfg(const char *driver, int enc,
memset(iv, 0, ivsize);
} else {
if (vec->generates_iv) {
- pr_err("alg: skcipher: %s has ivsize=0 but test vector %u generates IV!\n",
- driver, vec_num);
+ pr_err("alg: skcipher: %s has ivsize=0 but test vector %s generates IV!\n",
+ driver, vec_name);
return -EINVAL;
}
iv = NULL;
@@ -1676,8 +1682,8 @@ static int test_skcipher_vec_cfg(const char *driver, int enc,
err = build_cipher_test_sglists(tsgls, cfg, alignmask,
vec->len, vec->len, &input, 1);
if (err) {
- pr_err("alg: skcipher: %s %s: error preparing scatterlists for test vector %u, cfg=\"%s\"\n",
- driver, op, vec_num, cfg->name);
+ pr_err("alg: skcipher: %s %s: error preparing scatterlists for test vector %s, cfg=\"%s\"\n",
+ driver, op, vec_name, cfg->name);
return err;
}

@@ -1702,8 +1708,8 @@ static int test_skcipher_vec_cfg(const char *driver, int enc,
req->base.complete != crypto_req_done ||
req->base.flags != req_flags ||
req->base.data != &wait) {
- pr_err("alg: skcipher: %s %s corrupted request struct on test vector %u, cfg=\"%s\"\n",
- driver, op, vec_num, cfg->name);
+ pr_err("alg: skcipher: %s %s corrupted request struct on test vector %s, cfg=\"%s\"\n",
+ driver, op, vec_name, cfg->name);
if (req->cryptlen != vec->len)
pr_err("alg: skcipher: changed 'req->cryptlen'\n");
if (req->iv != iv)
@@ -1723,27 +1729,27 @@ static int test_skcipher_vec_cfg(const char *driver, int enc,
return -EINVAL;
}
if (is_test_sglist_corrupted(&tsgls->src)) {
- pr_err("alg: skcipher: %s %s corrupted src sgl on test vector %u, cfg=\"%s\"\n",
- driver, op, vec_num, cfg->name);
+ pr_err("alg: skcipher: %s %s corrupted src sgl on test vector %s, cfg=\"%s\"\n",
+ driver, op, vec_name, cfg->name);
return -EINVAL;
}
if (tsgls->dst.sgl_ptr != tsgls->src.sgl &&
is_test_sglist_corrupted(&tsgls->dst)) {
- pr_err("alg: skcipher: %s %s corrupted dst sgl on test vector %u, cfg=\"%s\"\n",
- driver, op, vec_num, cfg->name);
+ pr_err("alg: skcipher: %s %s corrupted dst sgl on test vector %s, cfg=\"%s\"\n",
+ driver, op, vec_name, cfg->name);
return -EINVAL;
}

if (err) {
if (err == vec->crypt_error)
return 0;
- pr_err("alg: skcipher: %s %s failed with err %d on test vector %u, cfg=\"%s\"\n",
- driver, op, err, vec_num, cfg->name);
+ pr_err("alg: skcipher: %s %s failed with err %d on test vector %s, cfg=\"%s\"\n",
+ driver, op, err, vec_name, cfg->name);
return err;
}
if (vec->crypt_error) {
- pr_err("alg: skcipher: %s %s unexpectedly succeeded on test vector %u, cfg=\"%s\"\n",
- driver, op, vec_num, cfg->name);
+ pr_err("alg: skcipher: %s %s unexpectedly succeeded on test vector %s, cfg=\"%s\"\n",
+ driver, op, vec_name, cfg->name);
return -EINVAL;
}

@@ -1751,20 +1757,20 @@ static int test_skcipher_vec_cfg(const char *driver, int enc,
err = verify_correct_output(&tsgls->dst, enc ? vec->ctext : vec->ptext,
vec->len, 0, true);
if (err == -EOVERFLOW) {
- pr_err("alg: skcipher: %s %s overran dst buffer on test vector %u, cfg=\"%s\"\n",
- driver, op, vec_num, cfg->name);
+ pr_err("alg: skcipher: %s %s overran dst buffer on test vector %s, cfg=\"%s\"\n",
+ driver, op, vec_name, cfg->name);
return err;
}
if (err) {
- pr_err("alg: skcipher: %s %s test failed (wrong result) on test vector %u, cfg=\"%s\"\n",
- driver, op, vec_num, cfg->name);
+ pr_err("alg: skcipher: %s %s test failed (wrong result) on test vector %s, cfg=\"%s\"\n",
+ driver, op, vec_name, cfg->name);
return err;
}

/* If applicable, check that the algorithm generated the correct IV */
if (vec->iv_out && memcmp(iv, vec->iv_out, ivsize) != 0) {
- pr_err("alg: skcipher: %s %s test failed (wrong output IV) on test vector %u, cfg=\"%s\"\n",
- driver, op, vec_num, cfg->name);
+ pr_err("alg: skcipher: %s %s test failed (wrong output IV) on test vector %s, cfg=\"%s\"\n",
+ driver, op, vec_name, cfg->name);
hexdump(iv, ivsize);
return -EINVAL;
}
@@ -1778,14 +1784,17 @@ static int test_skcipher_vec(const char *driver, int enc,
struct skcipher_request *req,
struct cipher_test_sglists *tsgls)
{
+ char vec_name[16];
unsigned int i;
int err;

if (fips_enabled && vec->fips_skip)
return 0;

+ sprintf(vec_name, "%u", vec_num);
+
for (i = 0; i < ARRAY_SIZE(default_cipher_testvec_configs); i++) {
- err = test_skcipher_vec_cfg(driver, enc, vec, vec_num,
+ err = test_skcipher_vec_cfg(driver, enc, vec, vec_name,
&default_cipher_testvec_configs[i],
req, tsgls);
if (err)
@@ -1800,7 +1809,7 @@ static int test_skcipher_vec(const char *driver, int enc,
for (i = 0; i < fuzz_iterations; i++) {
generate_random_testvec_config(&cfg, cfgname,
sizeof(cfgname));
- err = test_skcipher_vec_cfg(driver, enc, vec, vec_num,
+ err = test_skcipher_vec_cfg(driver, enc, vec, vec_name,
&cfg, req, tsgls);
if (err)
return err;
--
2.21.0


2019-03-31 20:06:04

by Eric Biggers

[permalink] [raw]
Subject: [RFC/RFT PATCH 10/18] crypto: cts - don't support empty messages

From: Eric Biggers <[email protected]>

My patches to make testmgr fuzz algorithms against their generic
implementation detected that the arm64 implementations of
"cts(cbc(aes))" handle empty messages differently from the cts template.
Namely, the arm64 implementations forbids (with -EINVAL) all messages
shorter than the block size, including the empty message; but the cts
template permits empty messages as a special case.

No user should be CTS-encrypting/decrypting empty messages, but we need
to keep the behavior consistent. Unfortunately, as noted in the source
of OpenSSL's CTS implementation [1], there's no common specification for
CTS. This makes it somewhat debatable what the behavior should be.

However, all CTS specifications seem to agree that messages shorter than
the block size are not allowed, and OpenSSL follows this in both CTS
conventions it implements. It would also simplify the user-visible
semantics to have empty messages no longer be a special case.

Therefore, make the cts template return -EINVAL on *all* messages
shorter than the block size, including the empty message.

[1] https://github.com/openssl/openssl/blob/master/crypto/modes/cts128.c

Signed-off-by: Eric Biggers <[email protected]>
---
crypto/cts.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/crypto/cts.c b/crypto/cts.c
index 4e28d83ae37da..9441da797bb90 100644
--- a/crypto/cts.c
+++ b/crypto/cts.c
@@ -152,12 +152,14 @@ static int crypto_cts_encrypt(struct skcipher_request *req)
struct skcipher_request *subreq = &rctx->subreq;
int bsize = crypto_skcipher_blocksize(tfm);
unsigned int nbytes = req->cryptlen;
- int cbc_blocks = (nbytes + bsize - 1) / bsize - 1;
unsigned int offset;

skcipher_request_set_tfm(subreq, ctx->child);

- if (cbc_blocks <= 0) {
+ if (nbytes < bsize)
+ return -EINVAL;
+
+ if (nbytes == bsize) {
skcipher_request_set_callback(subreq, req->base.flags,
req->base.complete,
req->base.data);
@@ -166,7 +168,7 @@ static int crypto_cts_encrypt(struct skcipher_request *req)
return crypto_skcipher_encrypt(subreq);
}

- offset = cbc_blocks * bsize;
+ offset = rounddown(nbytes - 1, bsize);
rctx->offset = offset;

skcipher_request_set_callback(subreq, req->base.flags,
@@ -244,13 +246,15 @@ static int crypto_cts_decrypt(struct skcipher_request *req)
struct skcipher_request *subreq = &rctx->subreq;
int bsize = crypto_skcipher_blocksize(tfm);
unsigned int nbytes = req->cryptlen;
- int cbc_blocks = (nbytes + bsize - 1) / bsize - 1;
unsigned int offset;
u8 *space;

skcipher_request_set_tfm(subreq, ctx->child);

- if (cbc_blocks <= 0) {
+ if (nbytes < bsize)
+ return -EINVAL;
+
+ if (nbytes == bsize) {
skcipher_request_set_callback(subreq, req->base.flags,
req->base.complete,
req->base.data);
@@ -264,10 +268,10 @@ static int crypto_cts_decrypt(struct skcipher_request *req)

space = crypto_cts_reqctx_space(req);

- offset = cbc_blocks * bsize;
+ offset = rounddown(nbytes - 1, bsize);
rctx->offset = offset;

- if (cbc_blocks <= 1)
+ if (offset <= bsize)
memcpy(space, req->iv, bsize);
else
scatterwalk_map_and_copy(space, req->src, offset - 2 * bsize,
--
2.21.0


2019-03-31 20:06:04

by Eric Biggers

[permalink] [raw]
Subject: [RFC/RFT PATCH 12/18] crypto: testmgr - expand ability to test for errors

From: Eric Biggers <[email protected]>

Update testmgr to support testing for specific errors from setkey() and
digest() for hashes; setkey() and encrypt()/decrypt() for skciphers and
ciphers; and setkey(), setauthsize(), and encrypt()/decrypt() for AEADs.
This is useful because algorithms usually restrict the lengths or format
of the message, key, and/or authentication tag in some way. And bad
inputs should be tested too, not just good inputs.

As part of this change, remove the ambiguously-named 'fail' flag and
replace it with 'setkey_error = -EINVAL' for the only test vector that
used it -- the DES weak key test vector. Note that this tightens the
test to require -EINVAL rather than any error code, but AFAICS this
won't cause any test failure.

Other than that, these new fields aren't set on any test vectors yet.
Later patches will do so.

Signed-off-by: Eric Biggers <[email protected]>
---
crypto/testmgr.c | 98 +++++++++++++++++++++++++++++++++---------------
crypto/testmgr.h | 20 +++++++---
2 files changed, 82 insertions(+), 36 deletions(-)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 2c2ddebb48d37..f81043c76cf55 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -968,11 +968,18 @@ static int test_hash_vec_cfg(const char *driver,
if (vec->ksize) {
err = crypto_ahash_setkey(tfm, vec->key, vec->ksize);
if (err) {
+ if (err == vec->setkey_error)
+ return 0;
pr_err("alg: hash: %s setkey failed with err %d on test vector %u; flags=%#x\n",
driver, err, vec_num,
crypto_ahash_get_flags(tfm));
return err;
}
+ if (vec->setkey_error) {
+ pr_err("alg: hash: %s setkey unexpectedly succeeded on test vector %u\n",
+ driver, vec_num);
+ return -EINVAL;
+ }
}

/* Build the scatterlist for the source data */
@@ -992,17 +999,25 @@ static int test_hash_vec_cfg(const char *driver,
testmgr_poison(req->__ctx, crypto_ahash_reqsize(tfm));
testmgr_poison(result, digestsize + TESTMGR_POISON_LEN);

- if (cfg->finalization_type == FINALIZATION_TYPE_DIGEST) {
+ if (cfg->finalization_type == FINALIZATION_TYPE_DIGEST ||
+ vec->digest_error) {
/* Just using digest() */
ahash_request_set_callback(req, req_flags, crypto_req_done,
&wait);
ahash_request_set_crypt(req, tsgl->sgl, result, vec->psize);
err = do_ahash_op(crypto_ahash_digest, req, &wait, cfg->nosimd);
if (err) {
+ if (err == vec->digest_error)
+ return 0;
pr_err("alg: hash: %s digest() failed with err %d on test vector %u, cfg=\"%s\"\n",
driver, err, vec_num, cfg->name);
return err;
}
+ if (vec->digest_error) {
+ pr_err("alg: hash: %s digest() unexpectedly succeeded on test vector %u, cfg=\"%s\"\n",
+ driver, vec_num, cfg->name);
+ return -EINVAL;
+ }
goto result_ready;
}

@@ -1267,14 +1282,12 @@ static int test_aead_vec_cfg(const char *driver, int enc,
else
crypto_aead_clear_flags(tfm, CRYPTO_TFM_REQ_FORBID_WEAK_KEYS);
err = crypto_aead_setkey(tfm, vec->key, vec->klen);
- if (err) {
- if (vec->fail) /* expectedly failed to set key? */
- return 0;
+ if (err && err != vec->setkey_error) {
pr_err("alg: aead: %s setkey failed with err %d on test vector %u; flags=%#x\n",
driver, err, vec_num, crypto_aead_get_flags(tfm));
return err;
}
- if (vec->fail) {
+ if (!err && vec->setkey_error) {
pr_err("alg: aead: %s setkey unexpectedly succeeded on test vector %u\n",
driver, vec_num);
return -EINVAL;
@@ -1282,11 +1295,19 @@ static int test_aead_vec_cfg(const char *driver, int enc,

/* Set the authentication tag size */
err = crypto_aead_setauthsize(tfm, authsize);
- if (err) {
+ if (err && err != vec->setauthsize_error) {
pr_err("alg: aead: %s setauthsize failed with err %d on test vector %u\n",
driver, err, vec_num);
return err;
}
+ if (!err && vec->setauthsize_error) {
+ pr_err("alg: aead: %s setauthsize unexpectedly succeeded on test vector %u\n",
+ driver, vec_num);
+ return -EINVAL;
+ }
+
+ if (vec->setkey_error || vec->setauthsize_error)
+ return 0;

/* The IV must be copied to a buffer, as the algorithm may modify it */
if (WARN_ON(ivsize > MAX_IVLEN))
@@ -1325,18 +1346,6 @@ static int test_aead_vec_cfg(const char *driver, int enc,
if (cfg->nosimd)
crypto_reenable_simd_for_test();
err = crypto_wait_req(err, &wait);
- if (err) {
- if (err == -EBADMSG && vec->novrfy)
- return 0;
- pr_err("alg: aead: %s %s failed with err %d on test vector %u, cfg=\"%s\"\n",
- driver, op, err, vec_num, cfg->name);
- return err;
- }
- if (vec->novrfy) {
- pr_err("alg: aead: %s %s unexpectedly succeeded on test vector %u, cfg=\"%s\"\n",
- driver, op, vec_num, cfg->name);
- return -EINVAL;
- }

/* Check that the algorithm didn't overwrite things it shouldn't have */
if (req->cryptlen != (enc ? vec->plen : vec->clen) ||
@@ -1382,6 +1391,19 @@ static int test_aead_vec_cfg(const char *driver, int enc,
return -EINVAL;
}

+ if (err) {
+ if (err == vec->crypt_error || (err == -EBADMSG && vec->novrfy))
+ return 0;
+ pr_err("alg: aead: %s %s failed with err %d on test vector %u, cfg=\"%s\"\n",
+ driver, op, err, vec_num, cfg->name);
+ return err;
+ }
+ if (vec->crypt_error || vec->novrfy) {
+ pr_err("alg: aead: %s %s unexpectedly succeeded on test vector %u, cfg=\"%s\"\n",
+ driver, op, vec_num, cfg->name);
+ return -EINVAL;
+ }
+
/* Check for the correct output (ciphertext or plaintext) */
err = verify_correct_output(&tsgls->dst, enc ? vec->ctext : vec->ptext,
enc ? vec->clen : vec->plen,
@@ -1547,13 +1569,19 @@ static int test_cipher(struct crypto_cipher *tfm, int enc,

ret = crypto_cipher_setkey(tfm, template[i].key,
template[i].klen);
- if (template[i].fail == !ret) {
- printk(KERN_ERR "alg: cipher: setkey failed "
- "on test %d for %s: flags=%x\n", j,
- algo, crypto_cipher_get_flags(tfm));
+ if (ret) {
+ if (ret == template[i].setkey_error)
+ continue;
+ pr_err("alg: cipher: %s setkey failed with err %d on test vector %u; flags=%#x\n",
+ algo, ret, j, crypto_cipher_get_flags(tfm));
goto out;
- } else if (ret)
- continue;
+ }
+ if (template[i].setkey_error) {
+ pr_err("alg: cipher: %s setkey unexpectedly succeeded on test vector %u\n",
+ algo, j);
+ ret = -EINVAL;
+ goto out;
+ }

for (k = 0; k < template[i].len;
k += crypto_cipher_blocksize(tfm)) {
@@ -1611,13 +1639,13 @@ static int test_skcipher_vec_cfg(const char *driver, int enc,
CRYPTO_TFM_REQ_FORBID_WEAK_KEYS);
err = crypto_skcipher_setkey(tfm, vec->key, vec->klen);
if (err) {
- if (vec->fail) /* expectedly failed to set key? */
+ if (err == vec->setkey_error)
return 0;
pr_err("alg: skcipher: %s setkey failed with err %d on test vector %u; flags=%#x\n",
driver, err, vec_num, crypto_skcipher_get_flags(tfm));
return err;
}
- if (vec->fail) {
+ if (vec->setkey_error) {
pr_err("alg: skcipher: %s setkey unexpectedly succeeded on test vector %u\n",
driver, vec_num);
return -EINVAL;
@@ -1664,11 +1692,6 @@ static int test_skcipher_vec_cfg(const char *driver, int enc,
if (cfg->nosimd)
crypto_reenable_simd_for_test();
err = crypto_wait_req(err, &wait);
- if (err) {
- pr_err("alg: skcipher: %s %s failed with err %d on test vector %u, cfg=\"%s\"\n",
- driver, op, err, vec_num, cfg->name);
- return err;
- }

/* Check that the algorithm didn't overwrite things it shouldn't have */
if (req->cryptlen != vec->len ||
@@ -1711,6 +1734,19 @@ static int test_skcipher_vec_cfg(const char *driver, int enc,
return -EINVAL;
}

+ if (err) {
+ if (err == vec->crypt_error)
+ return 0;
+ pr_err("alg: skcipher: %s %s failed with err %d on test vector %u, cfg=\"%s\"\n",
+ driver, op, err, vec_num, cfg->name);
+ return err;
+ }
+ if (vec->crypt_error) {
+ pr_err("alg: skcipher: %s %s unexpectedly succeeded on test vector %u, cfg=\"%s\"\n",
+ driver, op, vec_num, cfg->name);
+ return -EINVAL;
+ }
+
/* Check for the correct output (ciphertext or plaintext) */
err = verify_correct_output(&tsgls->dst, enc ? vec->ctext : vec->ptext,
vec->len, 0, true);
diff --git a/crypto/testmgr.h b/crypto/testmgr.h
index d18a37629f053..beb9bd79fe1fa 100644
--- a/crypto/testmgr.h
+++ b/crypto/testmgr.h
@@ -34,6 +34,8 @@
* @digest: Pointer to expected digest
* @psize: Length of source data in bytes
* @ksize: Length of @key in bytes (0 if no key)
+ * @setkey_error: Expected error from setkey()
+ * @digest_error: Expected error from digest()
*/
struct hash_testvec {
const char *key;
@@ -41,6 +43,8 @@ struct hash_testvec {
const char *digest;
unsigned short psize;
unsigned short ksize;
+ int setkey_error;
+ int digest_error;
};

/*
@@ -52,12 +56,13 @@ struct hash_testvec {
* @ptext: Pointer to plaintext
* @ctext: Pointer to ciphertext
* @len: Length of @ptext and @ctext in bytes
- * @fail: If set to one, the test need to fail
* @wk: Does the test need CRYPTO_TFM_REQ_FORBID_WEAK_KEYS?
* ( e.g. test needs to fail due to a weak key )
* @fips_skip: Skip the test vector in FIPS mode
* @generates_iv: Encryption should ignore the given IV, and output @iv_out.
* Decryption takes @iv_out. Needed for AES Keywrap ("kw(aes)").
+ * @setkey_error: Expected error from setkey()
+ * @crypt_error: Expected error from encrypt() and decrypt()
*/
struct cipher_testvec {
const char *key;
@@ -65,12 +70,13 @@ struct cipher_testvec {
const char *iv_out;
const char *ptext;
const char *ctext;
- bool fail;
unsigned char wk; /* weak key flag */
unsigned char klen;
unsigned short len;
bool fips_skip;
bool generates_iv;
+ int setkey_error;
+ int crypt_error;
};

/*
@@ -82,7 +88,6 @@ struct cipher_testvec {
* @ctext: Pointer to the full authenticated ciphertext. For AEADs that
* produce a separate "ciphertext" and "authentication tag", these
* two parts are concatenated: ciphertext || tag.
- * @fail: setkey() failure expected?
* @novrfy: Decryption verification failure expected?
* @wk: Does the test need CRYPTO_TFM_REQ_FORBID_WEAK_KEYS?
* (e.g. setkey() needs to fail due to a weak key)
@@ -90,6 +95,9 @@ struct cipher_testvec {
* @plen: Length of @ptext in bytes
* @alen: Length of @assoc in bytes
* @clen: Length of @ctext in bytes
+ * @setkey_error: Expected error from setkey()
+ * @setauthsize_error: Expected error from setauthsize()
+ * @crypt_error: Expected error from encrypt() and decrypt()
*/
struct aead_testvec {
const char *key;
@@ -97,13 +105,15 @@ struct aead_testvec {
const char *ptext;
const char *assoc;
const char *ctext;
- bool fail;
unsigned char novrfy;
unsigned char wk;
unsigned char klen;
unsigned short plen;
unsigned short clen;
unsigned short alen;
+ int setkey_error;
+ int setauthsize_error;
+ int crypt_error;
};

struct cprng_testvec {
@@ -7084,7 +7094,7 @@ static const struct cipher_testvec des_tv_template[] = {
"\xb4\x99\x26\xf7\x1f\xe1\xd4\x90",
.len = 24,
}, { /* Weak key */
- .fail = true,
+ .setkey_error = -EINVAL,
.wk = 1,
.key = "\x01\x01\x01\x01\x01\x01\x01\x01",
.klen = 8,
--
2.21.0


2019-03-31 20:06:05

by Eric Biggers

[permalink] [raw]
Subject: [RFC/RFT PATCH 08/18] crypto: ccm - fix incompatibility between "ccm" and "ccm_base"

From: Eric Biggers <[email protected]>

[This is essentially the same as the corresponding patch to GCM.]

CCM instances can be created by either the "ccm" template, which only
allows choosing the block cipher, e.g. "ccm(aes)"; or by "ccm_base",
which allows choosing the ctr and cbcmac implementations, e.g.
"ccm_base(ctr(aes-generic),cbcmac(aes-generic))".

However, a "ccm_base" instance prevents a "ccm" instance from being
registered using the same implementations. Nor will the instance be
found by lookups of "ccm". This can be used as a denial of service.
Moreover, "ccm_base" instances are never tested by the crypto
self-tests, even if there are compatible "ccm" tests.

The root cause of these problems is that instances of the two templates
use different cra_names. Therefore, fix these problems by making
"ccm_base" instances set the same cra_name as "ccm" instances, e.g.
"ccm(aes)" instead of "ccm_base(ctr(aes-generic),cbcmac(aes-generic))".

This requires extracting the block cipher name from the name of the ctr
algorithm, which means starting to require that the stream cipher really
be ctr and not something else. But it would be pretty bizarre if anyone
was actually relying on being able to use a non-ctr stream cipher here.

Fixes: 4a49b499dfa0 ("[CRYPTO] ccm: Added CCM mode")
Cc: [email protected]
Signed-off-by: Eric Biggers <[email protected]>
---
crypto/ccm.c | 38 ++++++++++++--------------------------
1 file changed, 12 insertions(+), 26 deletions(-)

diff --git a/crypto/ccm.c b/crypto/ccm.c
index 50df8f001c1c9..3bb49776450b7 100644
--- a/crypto/ccm.c
+++ b/crypto/ccm.c
@@ -458,7 +458,6 @@ static void crypto_ccm_free(struct aead_instance *inst)

static int crypto_ccm_create_common(struct crypto_template *tmpl,
struct rtattr **tb,
- const char *full_name,
const char *ctr_name,
const char *mac_name)
{
@@ -509,23 +508,22 @@ static int crypto_ccm_create_common(struct crypto_template *tmpl,

ctr = crypto_spawn_skcipher_alg(&ictx->ctr);

- /* Not a stream cipher? */
+ /* Must be CTR mode with 16-byte blocks. */
err = -EINVAL;
- if (ctr->base.cra_blocksize != 1)
+ if (strncmp(ctr->base.cra_name, "ctr(", 4) != 0 ||
+ crypto_skcipher_alg_ivsize(ctr) != 16)
goto err_drop_ctr;

- /* We want the real thing! */
- if (crypto_skcipher_alg_ivsize(ctr) != 16)
+ err = -ENAMETOOLONG;
+ if (snprintf(inst->alg.base.cra_name, CRYPTO_MAX_ALG_NAME,
+ "ccm(%s", ctr->base.cra_name + 4) >= CRYPTO_MAX_ALG_NAME)
goto err_drop_ctr;

- err = -ENAMETOOLONG;
if (snprintf(inst->alg.base.cra_driver_name, CRYPTO_MAX_ALG_NAME,
"ccm_base(%s,%s)", ctr->base.cra_driver_name,
mac->base.cra_driver_name) >= CRYPTO_MAX_ALG_NAME)
goto err_drop_ctr;

- memcpy(inst->alg.base.cra_name, full_name, CRYPTO_MAX_ALG_NAME);
-
inst->alg.base.cra_flags = ctr->base.cra_flags & CRYPTO_ALG_ASYNC;
inst->alg.base.cra_priority = (mac->base.cra_priority +
ctr->base.cra_priority) / 2;
@@ -567,7 +565,6 @@ static int crypto_ccm_create(struct crypto_template *tmpl, struct rtattr **tb)
const char *cipher_name;
char ctr_name[CRYPTO_MAX_ALG_NAME];
char mac_name[CRYPTO_MAX_ALG_NAME];
- char full_name[CRYPTO_MAX_ALG_NAME];

cipher_name = crypto_attr_alg_name(tb[1]);
if (IS_ERR(cipher_name))
@@ -581,35 +578,24 @@ static int crypto_ccm_create(struct crypto_template *tmpl, struct rtattr **tb)
cipher_name) >= CRYPTO_MAX_ALG_NAME)
return -ENAMETOOLONG;

- if (snprintf(full_name, CRYPTO_MAX_ALG_NAME, "ccm(%s)", cipher_name) >=
- CRYPTO_MAX_ALG_NAME)
- return -ENAMETOOLONG;
-
- return crypto_ccm_create_common(tmpl, tb, full_name, ctr_name,
- mac_name);
+ return crypto_ccm_create_common(tmpl, tb, ctr_name, mac_name);
}

static int crypto_ccm_base_create(struct crypto_template *tmpl,
struct rtattr **tb)
{
const char *ctr_name;
- const char *cipher_name;
- char full_name[CRYPTO_MAX_ALG_NAME];
+ const char *mac_name;

ctr_name = crypto_attr_alg_name(tb[1]);
if (IS_ERR(ctr_name))
return PTR_ERR(ctr_name);

- cipher_name = crypto_attr_alg_name(tb[2]);
- if (IS_ERR(cipher_name))
- return PTR_ERR(cipher_name);
-
- if (snprintf(full_name, CRYPTO_MAX_ALG_NAME, "ccm_base(%s,%s)",
- ctr_name, cipher_name) >= CRYPTO_MAX_ALG_NAME)
- return -ENAMETOOLONG;
+ mac_name = crypto_attr_alg_name(tb[2]);
+ if (IS_ERR(mac_name))
+ return PTR_ERR(mac_name);

- return crypto_ccm_create_common(tmpl, tb, full_name, ctr_name,
- cipher_name);
+ return crypto_ccm_create_common(tmpl, tb, ctr_name, mac_name);
}

static int crypto_rfc4309_setkey(struct crypto_aead *parent, const u8 *key,
--
2.21.0


2019-03-31 20:06:05

by Eric Biggers

[permalink] [raw]
Subject: [RFC/RFT PATCH 18/18] crypto: run initcalls for generic implementations earlier

From: Eric Biggers <[email protected]>

Use subsys_initcall for registration of all templates and generic
algorithm implementations, rather than module_init.

This is needed so that when both a generic and optimized implementation
of an algorithm are built into the kernel (not loadable modules), the
generic implementation is registered before the optimized one.
Otherwise, the self-tests for the optimized implementation are unable to
allocate the generic implementation for the new comparison fuzz tests.

Note that on arm, a side effect of this change is that self-tests for
generic implementations may run before the unaligned access handler has
been installed. So, unaligned accesses will crash the kernel. This is
arguably a good thing as it makes it easier to detect that type of bug.

Signed-off-by: Eric Biggers <[email protected]>
---
crypto/842.c | 2 +-
crypto/Makefile | 7 ++++---
crypto/adiantum.c | 2 +-
crypto/aegis128.c | 2 +-
crypto/aegis128l.c | 2 +-
crypto/aegis256.c | 2 +-
crypto/aes_generic.c | 2 +-
crypto/ansi_cprng.c | 2 +-
crypto/anubis.c | 2 +-
crypto/arc4.c | 2 +-
crypto/authenc.c | 2 +-
crypto/authencesn.c | 2 +-
crypto/blowfish_generic.c | 2 +-
crypto/camellia_generic.c | 2 +-
crypto/cast5_generic.c | 2 +-
crypto/cast6_generic.c | 2 +-
crypto/cbc.c | 2 +-
crypto/ccm.c | 2 +-
crypto/cfb.c | 2 +-
crypto/chacha20poly1305.c | 2 +-
crypto/chacha_generic.c | 2 +-
crypto/cmac.c | 2 +-
crypto/crc32_generic.c | 2 +-
crypto/crc32c_generic.c | 2 +-
crypto/crct10dif_generic.c | 2 +-
crypto/crypto_null.c | 2 +-
crypto/ctr.c | 2 +-
crypto/cts.c | 2 +-
crypto/deflate.c | 2 +-
crypto/des_generic.c | 2 +-
crypto/dh.c | 2 +-
crypto/drbg.c | 2 +-
crypto/ecb.c | 2 +-
crypto/ecdh.c | 2 +-
crypto/echainiv.c | 2 +-
crypto/fcrypt.c | 2 +-
crypto/fips.c | 2 +-
crypto/gcm.c | 2 +-
crypto/ghash-generic.c | 2 +-
crypto/hmac.c | 2 +-
crypto/jitterentropy-kcapi.c | 2 +-
crypto/keywrap.c | 2 +-
crypto/khazad.c | 2 +-
crypto/lrw.c | 2 +-
crypto/lz4.c | 2 +-
crypto/lz4hc.c | 2 +-
crypto/lzo-rle.c | 2 +-
crypto/lzo.c | 2 +-
crypto/md4.c | 2 +-
crypto/md5.c | 2 +-
crypto/michael_mic.c | 2 +-
crypto/morus1280.c | 2 +-
crypto/morus640.c | 2 +-
crypto/nhpoly1305.c | 2 +-
crypto/ofb.c | 2 +-
crypto/pcbc.c | 2 +-
crypto/poly1305_generic.c | 2 +-
crypto/rmd128.c | 2 +-
crypto/rmd160.c | 2 +-
crypto/rmd256.c | 2 +-
crypto/rmd320.c | 2 +-
crypto/rsa.c | 2 +-
crypto/salsa20_generic.c | 2 +-
crypto/seed.c | 2 +-
crypto/seqiv.c | 2 +-
crypto/serpent_generic.c | 2 +-
crypto/sha1_generic.c | 2 +-
crypto/sha256_generic.c | 2 +-
crypto/sha3_generic.c | 2 +-
crypto/sha512_generic.c | 2 +-
crypto/sm3_generic.c | 2 +-
crypto/sm4_generic.c | 2 +-
crypto/streebog_generic.c | 2 +-
crypto/tcrypt.c | 2 +-
crypto/tea.c | 2 +-
crypto/tgr192.c | 2 +-
crypto/twofish_generic.c | 2 +-
crypto/vmac.c | 2 +-
crypto/wp512.c | 2 +-
crypto/xcbc.c | 2 +-
crypto/xts.c | 2 +-
crypto/zstd.c | 2 +-
82 files changed, 85 insertions(+), 84 deletions(-)

diff --git a/crypto/842.c b/crypto/842.c
index bc26dc942821c..5f98393b65d1e 100644
--- a/crypto/842.c
+++ b/crypto/842.c
@@ -144,7 +144,7 @@ static int __init crypto842_mod_init(void)

return ret;
}
-module_init(crypto842_mod_init);
+subsys_initcall(crypto842_mod_init);

static void __exit crypto842_mod_exit(void)
{
diff --git a/crypto/Makefile b/crypto/Makefile
index fb5bf2a3a6663..da71e2d29bd6c 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -31,6 +31,10 @@ obj-$(CONFIG_CRYPTO_HASH2) += crypto_hash.o
obj-$(CONFIG_CRYPTO_AKCIPHER2) += akcipher.o
obj-$(CONFIG_CRYPTO_KPP2) += kpp.o

+# Keep this before all algorithms.
+cryptomgr-y := algboss.o testmgr.o
+obj-$(CONFIG_CRYPTO_MANAGER2) += cryptomgr.o
+
dh_generic-y := dh.o
dh_generic-y += dh_helper.o
obj-$(CONFIG_CRYPTO_DH) += dh_generic.o
@@ -50,9 +54,6 @@ crypto_acompress-y := acompress.o
crypto_acompress-y += scompress.o
obj-$(CONFIG_CRYPTO_ACOMP2) += crypto_acompress.o

-cryptomgr-y := algboss.o testmgr.o
-
-obj-$(CONFIG_CRYPTO_MANAGER2) += cryptomgr.o
obj-$(CONFIG_CRYPTO_USER) += crypto_user.o
crypto_user-y := crypto_user_base.o
crypto_user-$(CONFIG_CRYPTO_STATS) += crypto_user_stat.o
diff --git a/crypto/adiantum.c b/crypto/adiantum.c
index 5564e73266a6a..e6de50f669aa7 100644
--- a/crypto/adiantum.c
+++ b/crypto/adiantum.c
@@ -659,7 +659,7 @@ static void __exit adiantum_module_exit(void)
crypto_unregister_template(&adiantum_tmpl);
}

-module_init(adiantum_module_init);
+subsys_initcall(adiantum_module_init);
module_exit(adiantum_module_exit);

MODULE_DESCRIPTION("Adiantum length-preserving encryption mode");
diff --git a/crypto/aegis128.c b/crypto/aegis128.c
index 3718a83413032..d78f77fc5dd18 100644
--- a/crypto/aegis128.c
+++ b/crypto/aegis128.c
@@ -448,7 +448,7 @@ static void __exit crypto_aegis128_module_exit(void)
crypto_unregister_aead(&crypto_aegis128_alg);
}

-module_init(crypto_aegis128_module_init);
+subsys_initcall(crypto_aegis128_module_init);
module_exit(crypto_aegis128_module_exit);

MODULE_LICENSE("GPL");
diff --git a/crypto/aegis128l.c b/crypto/aegis128l.c
index 275a8616d71bd..9bca3d619a22b 100644
--- a/crypto/aegis128l.c
+++ b/crypto/aegis128l.c
@@ -512,7 +512,7 @@ static void __exit crypto_aegis128l_module_exit(void)
crypto_unregister_aead(&crypto_aegis128l_alg);
}

-module_init(crypto_aegis128l_module_init);
+subsys_initcall(crypto_aegis128l_module_init);
module_exit(crypto_aegis128l_module_exit);

MODULE_LICENSE("GPL");
diff --git a/crypto/aegis256.c b/crypto/aegis256.c
index ecd6b7f34a2d2..b47fd39595ad7 100644
--- a/crypto/aegis256.c
+++ b/crypto/aegis256.c
@@ -463,7 +463,7 @@ static void __exit crypto_aegis256_module_exit(void)
crypto_unregister_aead(&crypto_aegis256_alg);
}

-module_init(crypto_aegis256_module_init);
+subsys_initcall(crypto_aegis256_module_init);
module_exit(crypto_aegis256_module_exit);

MODULE_LICENSE("GPL");
diff --git a/crypto/aes_generic.c b/crypto/aes_generic.c
index 13df33aca4631..1ff79518121ae 100644
--- a/crypto/aes_generic.c
+++ b/crypto/aes_generic.c
@@ -1470,7 +1470,7 @@ static void __exit aes_fini(void)
crypto_unregister_alg(&aes_alg);
}

-module_init(aes_init);
+subsys_initcall(aes_init);
module_exit(aes_fini);

MODULE_DESCRIPTION("Rijndael (AES) Cipher Algorithm");
diff --git a/crypto/ansi_cprng.c b/crypto/ansi_cprng.c
index eff337ce90037..e7c43ea4ce9d1 100644
--- a/crypto/ansi_cprng.c
+++ b/crypto/ansi_cprng.c
@@ -472,7 +472,7 @@ MODULE_DESCRIPTION("Software Pseudo Random Number Generator");
MODULE_AUTHOR("Neil Horman <[email protected]>");
module_param(dbg, int, 0);
MODULE_PARM_DESC(dbg, "Boolean to enable debugging (0/1 == off/on)");
-module_init(prng_mod_init);
+subsys_initcall(prng_mod_init);
module_exit(prng_mod_fini);
MODULE_ALIAS_CRYPTO("stdrng");
MODULE_ALIAS_CRYPTO("ansi_cprng");
diff --git a/crypto/anubis.c b/crypto/anubis.c
index 4bb187c2a9027..673927de0eb92 100644
--- a/crypto/anubis.c
+++ b/crypto/anubis.c
@@ -699,7 +699,7 @@ static void __exit anubis_mod_fini(void)
crypto_unregister_alg(&anubis_alg);
}

-module_init(anubis_mod_init);
+subsys_initcall(anubis_mod_init);
module_exit(anubis_mod_fini);

MODULE_LICENSE("GPL");
diff --git a/crypto/arc4.c b/crypto/arc4.c
index 6c93342e3405b..2233d36456e27 100644
--- a/crypto/arc4.c
+++ b/crypto/arc4.c
@@ -163,7 +163,7 @@ static void __exit arc4_exit(void)
crypto_unregister_skcipher(&arc4_skcipher);
}

-module_init(arc4_init);
+subsys_initcall(arc4_init);
module_exit(arc4_exit);

MODULE_LICENSE("GPL");
diff --git a/crypto/authenc.c b/crypto/authenc.c
index 4be293a4b5f0f..b3eddac7fa3a4 100644
--- a/crypto/authenc.c
+++ b/crypto/authenc.c
@@ -508,7 +508,7 @@ static void __exit crypto_authenc_module_exit(void)
crypto_unregister_template(&crypto_authenc_tmpl);
}

-module_init(crypto_authenc_module_init);
+subsys_initcall(crypto_authenc_module_init);
module_exit(crypto_authenc_module_exit);

MODULE_LICENSE("GPL");
diff --git a/crypto/authencesn.c b/crypto/authencesn.c
index 4741fe89ba2cd..58074308e5350 100644
--- a/crypto/authencesn.c
+++ b/crypto/authencesn.c
@@ -523,7 +523,7 @@ static void __exit crypto_authenc_esn_module_exit(void)
crypto_unregister_template(&crypto_authenc_esn_tmpl);
}

-module_init(crypto_authenc_esn_module_init);
+subsys_initcall(crypto_authenc_esn_module_init);
module_exit(crypto_authenc_esn_module_exit);

MODULE_LICENSE("GPL");
diff --git a/crypto/blowfish_generic.c b/crypto/blowfish_generic.c
index 87b392a77a939..8548ced8b0741 100644
--- a/crypto/blowfish_generic.c
+++ b/crypto/blowfish_generic.c
@@ -133,7 +133,7 @@ static void __exit blowfish_mod_fini(void)
crypto_unregister_alg(&alg);
}

-module_init(blowfish_mod_init);
+subsys_initcall(blowfish_mod_init);
module_exit(blowfish_mod_fini);

MODULE_LICENSE("GPL");
diff --git a/crypto/camellia_generic.c b/crypto/camellia_generic.c
index 32ddd4836ff55..15ce1281f5d9c 100644
--- a/crypto/camellia_generic.c
+++ b/crypto/camellia_generic.c
@@ -1092,7 +1092,7 @@ static void __exit camellia_fini(void)
crypto_unregister_alg(&camellia_alg);
}

-module_init(camellia_init);
+subsys_initcall(camellia_init);
module_exit(camellia_fini);

MODULE_DESCRIPTION("Camellia Cipher Algorithm");
diff --git a/crypto/cast5_generic.c b/crypto/cast5_generic.c
index 66169c1783148..24bc7d4e33be0 100644
--- a/crypto/cast5_generic.c
+++ b/crypto/cast5_generic.c
@@ -543,7 +543,7 @@ static void __exit cast5_mod_fini(void)
crypto_unregister_alg(&alg);
}

-module_init(cast5_mod_init);
+subsys_initcall(cast5_mod_init);
module_exit(cast5_mod_fini);

MODULE_LICENSE("GPL");
diff --git a/crypto/cast6_generic.c b/crypto/cast6_generic.c
index c8e5ec69790e2..edd59cc349919 100644
--- a/crypto/cast6_generic.c
+++ b/crypto/cast6_generic.c
@@ -285,7 +285,7 @@ static void __exit cast6_mod_fini(void)
crypto_unregister_alg(&alg);
}

-module_init(cast6_mod_init);
+subsys_initcall(cast6_mod_init);
module_exit(cast6_mod_fini);

MODULE_LICENSE("GPL");
diff --git a/crypto/cbc.c b/crypto/cbc.c
index d12efaac9230b..129f79d033658 100644
--- a/crypto/cbc.c
+++ b/crypto/cbc.c
@@ -98,7 +98,7 @@ static void __exit crypto_cbc_module_exit(void)
crypto_unregister_template(&crypto_cbc_tmpl);
}

-module_init(crypto_cbc_module_init);
+subsys_initcall(crypto_cbc_module_init);
module_exit(crypto_cbc_module_exit);

MODULE_LICENSE("GPL");
diff --git a/crypto/ccm.c b/crypto/ccm.c
index 3bb49776450b7..c3c175f165aca 100644
--- a/crypto/ccm.c
+++ b/crypto/ccm.c
@@ -1000,7 +1000,7 @@ static void __exit crypto_ccm_module_exit(void)
ARRAY_SIZE(crypto_ccm_tmpls));
}

-module_init(crypto_ccm_module_init);
+subsys_initcall(crypto_ccm_module_init);
module_exit(crypto_ccm_module_exit);

MODULE_LICENSE("GPL");
diff --git a/crypto/cfb.c b/crypto/cfb.c
index 03ac847f6d6ab..7b68fbb617324 100644
--- a/crypto/cfb.c
+++ b/crypto/cfb.c
@@ -243,7 +243,7 @@ static void __exit crypto_cfb_module_exit(void)
crypto_unregister_template(&crypto_cfb_tmpl);
}

-module_init(crypto_cfb_module_init);
+subsys_initcall(crypto_cfb_module_init);
module_exit(crypto_cfb_module_exit);

MODULE_LICENSE("GPL");
diff --git a/crypto/chacha20poly1305.c b/crypto/chacha20poly1305.c
index 279d816ab51dd..e38a2d61819a7 100644
--- a/crypto/chacha20poly1305.c
+++ b/crypto/chacha20poly1305.c
@@ -725,7 +725,7 @@ static void __exit chacha20poly1305_module_exit(void)
ARRAY_SIZE(rfc7539_tmpls));
}

-module_init(chacha20poly1305_module_init);
+subsys_initcall(chacha20poly1305_module_init);
module_exit(chacha20poly1305_module_exit);

MODULE_LICENSE("GPL");
diff --git a/crypto/chacha_generic.c b/crypto/chacha_generic.c
index a7fae9b73ec4e..d2ec04997832e 100644
--- a/crypto/chacha_generic.c
+++ b/crypto/chacha_generic.c
@@ -201,7 +201,7 @@ static void __exit chacha_generic_mod_fini(void)
crypto_unregister_skciphers(algs, ARRAY_SIZE(algs));
}

-module_init(chacha_generic_mod_init);
+subsys_initcall(chacha_generic_mod_init);
module_exit(chacha_generic_mod_fini);

MODULE_LICENSE("GPL");
diff --git a/crypto/cmac.c b/crypto/cmac.c
index 16301f52858ca..c60b6c011ec60 100644
--- a/crypto/cmac.c
+++ b/crypto/cmac.c
@@ -313,7 +313,7 @@ static void __exit crypto_cmac_module_exit(void)
crypto_unregister_template(&crypto_cmac_tmpl);
}

-module_init(crypto_cmac_module_init);
+subsys_initcall(crypto_cmac_module_init);
module_exit(crypto_cmac_module_exit);

MODULE_LICENSE("GPL");
diff --git a/crypto/crc32_generic.c b/crypto/crc32_generic.c
index 00facd27bcc29..9e97912280bdf 100644
--- a/crypto/crc32_generic.c
+++ b/crypto/crc32_generic.c
@@ -146,7 +146,7 @@ static void __exit crc32_mod_fini(void)
crypto_unregister_shash(&alg);
}

-module_init(crc32_mod_init);
+subsys_initcall(crc32_mod_init);
module_exit(crc32_mod_fini);

MODULE_AUTHOR("Alexander Boyko <[email protected]>");
diff --git a/crypto/crc32c_generic.c b/crypto/crc32c_generic.c
index 7283066ecc982..ad26f15d4c7b3 100644
--- a/crypto/crc32c_generic.c
+++ b/crypto/crc32c_generic.c
@@ -165,7 +165,7 @@ static void __exit crc32c_mod_fini(void)
crypto_unregister_shash(&alg);
}

-module_init(crc32c_mod_init);
+subsys_initcall(crc32c_mod_init);
module_exit(crc32c_mod_fini);

MODULE_AUTHOR("Clay Haapala <[email protected]>");
diff --git a/crypto/crct10dif_generic.c b/crypto/crct10dif_generic.c
index d08048ae55527..d90c0070710e8 100644
--- a/crypto/crct10dif_generic.c
+++ b/crypto/crct10dif_generic.c
@@ -112,7 +112,7 @@ static void __exit crct10dif_mod_fini(void)
crypto_unregister_shash(&alg);
}

-module_init(crct10dif_mod_init);
+subsys_initcall(crct10dif_mod_init);
module_exit(crct10dif_mod_fini);

MODULE_AUTHOR("Tim Chen <[email protected]>");
diff --git a/crypto/crypto_null.c b/crypto/crypto_null.c
index 01630a9c7e011..9320d4eaa4a8a 100644
--- a/crypto/crypto_null.c
+++ b/crypto/crypto_null.c
@@ -220,7 +220,7 @@ static void __exit crypto_null_mod_fini(void)
crypto_unregister_skcipher(&skcipher_null);
}

-module_init(crypto_null_mod_init);
+subsys_initcall(crypto_null_mod_init);
module_exit(crypto_null_mod_fini);

MODULE_LICENSE("GPL");
diff --git a/crypto/ctr.c b/crypto/ctr.c
index ec8f8b67473a0..52cdf2c5605fb 100644
--- a/crypto/ctr.c
+++ b/crypto/ctr.c
@@ -384,7 +384,7 @@ static void __exit crypto_ctr_module_exit(void)
ARRAY_SIZE(crypto_ctr_tmpls));
}

-module_init(crypto_ctr_module_init);
+subsys_initcall(crypto_ctr_module_init);
module_exit(crypto_ctr_module_exit);

MODULE_LICENSE("GPL");
diff --git a/crypto/cts.c b/crypto/cts.c
index 9441da797bb90..6b6087dbb62a2 100644
--- a/crypto/cts.c
+++ b/crypto/cts.c
@@ -423,7 +423,7 @@ static void __exit crypto_cts_module_exit(void)
crypto_unregister_template(&crypto_cts_tmpl);
}

-module_init(crypto_cts_module_init);
+subsys_initcall(crypto_cts_module_init);
module_exit(crypto_cts_module_exit);

MODULE_LICENSE("Dual BSD/GPL");
diff --git a/crypto/deflate.c b/crypto/deflate.c
index 94ec3b36a8e83..aab089cde1bf8 100644
--- a/crypto/deflate.c
+++ b/crypto/deflate.c
@@ -334,7 +334,7 @@ static void __exit deflate_mod_fini(void)
crypto_unregister_scomps(scomp, ARRAY_SIZE(scomp));
}

-module_init(deflate_mod_init);
+subsys_initcall(deflate_mod_init);
module_exit(deflate_mod_fini);

MODULE_LICENSE("GPL");
diff --git a/crypto/des_generic.c b/crypto/des_generic.c
index 1e6621665dd92..9efebe47f8f0a 100644
--- a/crypto/des_generic.c
+++ b/crypto/des_generic.c
@@ -993,7 +993,7 @@ static void __exit des_generic_mod_fini(void)
crypto_unregister_algs(des_algs, ARRAY_SIZE(des_algs));
}

-module_init(des_generic_mod_init);
+subsys_initcall(des_generic_mod_init);
module_exit(des_generic_mod_fini);

MODULE_LICENSE("GPL");
diff --git a/crypto/dh.c b/crypto/dh.c
index 09a44de4209d1..ce77fb4ee8b32 100644
--- a/crypto/dh.c
+++ b/crypto/dh.c
@@ -236,7 +236,7 @@ static void dh_exit(void)
crypto_unregister_kpp(&dh);
}

-module_init(dh_init);
+subsys_initcall(dh_init);
module_exit(dh_exit);
MODULE_ALIAS_CRYPTO("dh");
MODULE_LICENSE("GPL");
diff --git a/crypto/drbg.c b/crypto/drbg.c
index bc52d95626110..710b3046a4df2 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -2039,7 +2039,7 @@ static void __exit drbg_exit(void)
crypto_unregister_rngs(drbg_algs, (ARRAY_SIZE(drbg_cores) * 2));
}

-module_init(drbg_init);
+subsys_initcall(drbg_init);
module_exit(drbg_exit);
#ifndef CRYPTO_DRBG_HASH_STRING
#define CRYPTO_DRBG_HASH_STRING ""
diff --git a/crypto/ecb.c b/crypto/ecb.c
index 0732715c8d915..de839129d151c 100644
--- a/crypto/ecb.c
+++ b/crypto/ecb.c
@@ -101,7 +101,7 @@ static void __exit crypto_ecb_module_exit(void)
crypto_unregister_template(&crypto_ecb_tmpl);
}

-module_init(crypto_ecb_module_init);
+subsys_initcall(crypto_ecb_module_init);
module_exit(crypto_ecb_module_exit);

MODULE_LICENSE("GPL");
diff --git a/crypto/ecdh.c b/crypto/ecdh.c
index bf6300175b9cd..890092bd89899 100644
--- a/crypto/ecdh.c
+++ b/crypto/ecdh.c
@@ -166,7 +166,7 @@ static void ecdh_exit(void)
crypto_unregister_kpp(&ecdh);
}

-module_init(ecdh_init);
+subsys_initcall(ecdh_init);
module_exit(ecdh_exit);
MODULE_ALIAS_CRYPTO("ecdh");
MODULE_LICENSE("GPL");
diff --git a/crypto/echainiv.c b/crypto/echainiv.c
index 77e607fdbfb73..e71d1bc8d850d 100644
--- a/crypto/echainiv.c
+++ b/crypto/echainiv.c
@@ -174,7 +174,7 @@ static void __exit echainiv_module_exit(void)
crypto_unregister_template(&echainiv_tmpl);
}

-module_init(echainiv_module_init);
+subsys_initcall(echainiv_module_init);
module_exit(echainiv_module_exit);

MODULE_LICENSE("GPL");
diff --git a/crypto/fcrypt.c b/crypto/fcrypt.c
index 77286ea28865b..4e8704405a3b9 100644
--- a/crypto/fcrypt.c
+++ b/crypto/fcrypt.c
@@ -414,7 +414,7 @@ static void __exit fcrypt_mod_fini(void)
crypto_unregister_alg(&fcrypt_alg);
}

-module_init(fcrypt_mod_init);
+subsys_initcall(fcrypt_mod_init);
module_exit(fcrypt_mod_fini);

MODULE_LICENSE("Dual BSD/GPL");
diff --git a/crypto/fips.c b/crypto/fips.c
index 9d627c1cf8bc7..9dfed122d6da9 100644
--- a/crypto/fips.c
+++ b/crypto/fips.c
@@ -74,5 +74,5 @@ static void __exit fips_exit(void)
crypto_proc_fips_exit();
}

-module_init(fips_init);
+subsys_initcall(fips_init);
module_exit(fips_exit);
diff --git a/crypto/gcm.c b/crypto/gcm.c
index 12ff5ed569272..8a31095d9b412 100644
--- a/crypto/gcm.c
+++ b/crypto/gcm.c
@@ -1244,7 +1244,7 @@ static void __exit crypto_gcm_module_exit(void)
ARRAY_SIZE(crypto_gcm_tmpls));
}

-module_init(crypto_gcm_module_init);
+subsys_initcall(crypto_gcm_module_init);
module_exit(crypto_gcm_module_exit);

MODULE_LICENSE("GPL");
diff --git a/crypto/ghash-generic.c b/crypto/ghash-generic.c
index d9f192b953b22..e6307935413c1 100644
--- a/crypto/ghash-generic.c
+++ b/crypto/ghash-generic.c
@@ -149,7 +149,7 @@ static void __exit ghash_mod_exit(void)
crypto_unregister_shash(&ghash_alg);
}

-module_init(ghash_mod_init);
+subsys_initcall(ghash_mod_init);
module_exit(ghash_mod_exit);

MODULE_LICENSE("GPL");
diff --git a/crypto/hmac.c b/crypto/hmac.c
index e74730224f0a5..4ceb3f1f0eb84 100644
--- a/crypto/hmac.c
+++ b/crypto/hmac.c
@@ -268,7 +268,7 @@ static void __exit hmac_module_exit(void)
crypto_unregister_template(&hmac_tmpl);
}

-module_init(hmac_module_init);
+subsys_initcall(hmac_module_init);
module_exit(hmac_module_exit);

MODULE_LICENSE("GPL");
diff --git a/crypto/jitterentropy-kcapi.c b/crypto/jitterentropy-kcapi.c
index 787dccca37159..6ea1a270b8dc2 100644
--- a/crypto/jitterentropy-kcapi.c
+++ b/crypto/jitterentropy-kcapi.c
@@ -198,7 +198,7 @@ static void __exit jent_mod_exit(void)
crypto_unregister_rng(&jent_alg);
}

-module_init(jent_mod_init);
+subsys_initcall(jent_mod_init);
module_exit(jent_mod_exit);

MODULE_LICENSE("Dual BSD/GPL");
diff --git a/crypto/keywrap.c b/crypto/keywrap.c
index a5cfe610d8f40..a155c88105ea1 100644
--- a/crypto/keywrap.c
+++ b/crypto/keywrap.c
@@ -310,7 +310,7 @@ static void __exit crypto_kw_exit(void)
crypto_unregister_template(&crypto_kw_tmpl);
}

-module_init(crypto_kw_init);
+subsys_initcall(crypto_kw_init);
module_exit(crypto_kw_exit);

MODULE_LICENSE("Dual BSD/GPL");
diff --git a/crypto/khazad.c b/crypto/khazad.c
index 873eb5ded6d7a..b50aa8a3ab4cf 100644
--- a/crypto/khazad.c
+++ b/crypto/khazad.c
@@ -875,7 +875,7 @@ static void __exit khazad_mod_fini(void)
}


-module_init(khazad_mod_init);
+subsys_initcall(khazad_mod_init);
module_exit(khazad_mod_fini);

MODULE_LICENSE("GPL");
diff --git a/crypto/lrw.c b/crypto/lrw.c
index 0430ccd087286..190131ce248c0 100644
--- a/crypto/lrw.c
+++ b/crypto/lrw.c
@@ -431,7 +431,7 @@ static void __exit crypto_module_exit(void)
crypto_unregister_template(&crypto_tmpl);
}

-module_init(crypto_module_init);
+subsys_initcall(crypto_module_init);
module_exit(crypto_module_exit);

MODULE_LICENSE("GPL");
diff --git a/crypto/lz4.c b/crypto/lz4.c
index c160dfdbf2e07..1e35134d0a98d 100644
--- a/crypto/lz4.c
+++ b/crypto/lz4.c
@@ -164,7 +164,7 @@ static void __exit lz4_mod_fini(void)
crypto_unregister_scomp(&scomp);
}

-module_init(lz4_mod_init);
+subsys_initcall(lz4_mod_init);
module_exit(lz4_mod_fini);

MODULE_LICENSE("GPL");
diff --git a/crypto/lz4hc.c b/crypto/lz4hc.c
index 583b5e013d7a5..4a220b628fe7e 100644
--- a/crypto/lz4hc.c
+++ b/crypto/lz4hc.c
@@ -165,7 +165,7 @@ static void __exit lz4hc_mod_fini(void)
crypto_unregister_scomp(&scomp);
}

-module_init(lz4hc_mod_init);
+subsys_initcall(lz4hc_mod_init);
module_exit(lz4hc_mod_fini);

MODULE_LICENSE("GPL");
diff --git a/crypto/lzo-rle.c b/crypto/lzo-rle.c
index ea9c75b1db49b..4c82bf18440f0 100644
--- a/crypto/lzo-rle.c
+++ b/crypto/lzo-rle.c
@@ -167,7 +167,7 @@ static void __exit lzorle_mod_fini(void)
crypto_unregister_scomp(&scomp);
}

-module_init(lzorle_mod_init);
+subsys_initcall(lzorle_mod_init);
module_exit(lzorle_mod_fini);

MODULE_LICENSE("GPL");
diff --git a/crypto/lzo.c b/crypto/lzo.c
index 218567d717d68..4a6ac8f247d0a 100644
--- a/crypto/lzo.c
+++ b/crypto/lzo.c
@@ -167,7 +167,7 @@ static void __exit lzo_mod_fini(void)
crypto_unregister_scomp(&scomp);
}

-module_init(lzo_mod_init);
+subsys_initcall(lzo_mod_init);
module_exit(lzo_mod_fini);

MODULE_LICENSE("GPL");
diff --git a/crypto/md4.c b/crypto/md4.c
index 9965ec40d9f97..9a1a228a0c695 100644
--- a/crypto/md4.c
+++ b/crypto/md4.c
@@ -232,7 +232,7 @@ static void __exit md4_mod_fini(void)
crypto_unregister_shash(&alg);
}

-module_init(md4_mod_init);
+subsys_initcall(md4_mod_init);
module_exit(md4_mod_fini);

MODULE_LICENSE("GPL");
diff --git a/crypto/md5.c b/crypto/md5.c
index 94dd78144ba3e..221c2c0932f83 100644
--- a/crypto/md5.c
+++ b/crypto/md5.c
@@ -244,7 +244,7 @@ static void __exit md5_mod_fini(void)
crypto_unregister_shash(&alg);
}

-module_init(md5_mod_init);
+subsys_initcall(md5_mod_init);
module_exit(md5_mod_fini);

MODULE_LICENSE("GPL");
diff --git a/crypto/michael_mic.c b/crypto/michael_mic.c
index 46195e0d0f4d1..538ae79337957 100644
--- a/crypto/michael_mic.c
+++ b/crypto/michael_mic.c
@@ -178,7 +178,7 @@ static void __exit michael_mic_exit(void)
}


-module_init(michael_mic_init);
+subsys_initcall(michael_mic_init);
module_exit(michael_mic_exit);

MODULE_LICENSE("GPL v2");
diff --git a/crypto/morus1280.c b/crypto/morus1280.c
index 0747732d5b78a..f8734c6576af6 100644
--- a/crypto/morus1280.c
+++ b/crypto/morus1280.c
@@ -532,7 +532,7 @@ static void __exit crypto_morus1280_module_exit(void)
crypto_unregister_aead(&crypto_morus1280_alg);
}

-module_init(crypto_morus1280_module_init);
+subsys_initcall(crypto_morus1280_module_init);
module_exit(crypto_morus1280_module_exit);

MODULE_LICENSE("GPL");
diff --git a/crypto/morus640.c b/crypto/morus640.c
index 1617a1eb8be13..ae5aa9482cb45 100644
--- a/crypto/morus640.c
+++ b/crypto/morus640.c
@@ -523,7 +523,7 @@ static void __exit crypto_morus640_module_exit(void)
crypto_unregister_aead(&crypto_morus640_alg);
}

-module_init(crypto_morus640_module_init);
+subsys_initcall(crypto_morus640_module_init);
module_exit(crypto_morus640_module_exit);

MODULE_LICENSE("GPL");
diff --git a/crypto/nhpoly1305.c b/crypto/nhpoly1305.c
index ec831a5594d8f..9ab4e07cde4dc 100644
--- a/crypto/nhpoly1305.c
+++ b/crypto/nhpoly1305.c
@@ -244,7 +244,7 @@ static void __exit nhpoly1305_mod_exit(void)
crypto_unregister_shash(&nhpoly1305_alg);
}

-module_init(nhpoly1305_mod_init);
+subsys_initcall(nhpoly1305_mod_init);
module_exit(nhpoly1305_mod_exit);

MODULE_DESCRIPTION("NHPoly1305 ε-almost-∆-universal hash function");
diff --git a/crypto/ofb.c b/crypto/ofb.c
index 34b6e1f426f7a..133ff4c7f2c67 100644
--- a/crypto/ofb.c
+++ b/crypto/ofb.c
@@ -95,7 +95,7 @@ static void __exit crypto_ofb_module_exit(void)
crypto_unregister_template(&crypto_ofb_tmpl);
}

-module_init(crypto_ofb_module_init);
+subsys_initcall(crypto_ofb_module_init);
module_exit(crypto_ofb_module_exit);

MODULE_LICENSE("GPL");
diff --git a/crypto/pcbc.c b/crypto/pcbc.c
index 2fa03fc576fe4..31b3ce948474e 100644
--- a/crypto/pcbc.c
+++ b/crypto/pcbc.c
@@ -191,7 +191,7 @@ static void __exit crypto_pcbc_module_exit(void)
crypto_unregister_template(&crypto_pcbc_tmpl);
}

-module_init(crypto_pcbc_module_init);
+subsys_initcall(crypto_pcbc_module_init);
module_exit(crypto_pcbc_module_exit);

MODULE_LICENSE("GPL");
diff --git a/crypto/poly1305_generic.c b/crypto/poly1305_generic.c
index 2a06874204e87..adc40298c7492 100644
--- a/crypto/poly1305_generic.c
+++ b/crypto/poly1305_generic.c
@@ -318,7 +318,7 @@ static void __exit poly1305_mod_exit(void)
crypto_unregister_shash(&poly1305_alg);
}

-module_init(poly1305_mod_init);
+subsys_initcall(poly1305_mod_init);
module_exit(poly1305_mod_exit);

MODULE_LICENSE("GPL");
diff --git a/crypto/rmd128.c b/crypto/rmd128.c
index 5f4472256e272..faf4252c4b856 100644
--- a/crypto/rmd128.c
+++ b/crypto/rmd128.c
@@ -318,7 +318,7 @@ static void __exit rmd128_mod_fini(void)
crypto_unregister_shash(&alg);
}

-module_init(rmd128_mod_init);
+subsys_initcall(rmd128_mod_init);
module_exit(rmd128_mod_fini);

MODULE_LICENSE("GPL");
diff --git a/crypto/rmd160.c b/crypto/rmd160.c
index 737645344d1cb..b33309916d4fe 100644
--- a/crypto/rmd160.c
+++ b/crypto/rmd160.c
@@ -362,7 +362,7 @@ static void __exit rmd160_mod_fini(void)
crypto_unregister_shash(&alg);
}

-module_init(rmd160_mod_init);
+subsys_initcall(rmd160_mod_init);
module_exit(rmd160_mod_fini);

MODULE_LICENSE("GPL");
diff --git a/crypto/rmd256.c b/crypto/rmd256.c
index 0e9d30676a013..2a643250c9a5c 100644
--- a/crypto/rmd256.c
+++ b/crypto/rmd256.c
@@ -337,7 +337,7 @@ static void __exit rmd256_mod_fini(void)
crypto_unregister_shash(&alg);
}

-module_init(rmd256_mod_init);
+subsys_initcall(rmd256_mod_init);
module_exit(rmd256_mod_fini);

MODULE_LICENSE("GPL");
diff --git a/crypto/rmd320.c b/crypto/rmd320.c
index 3ae1df5bb48c8..2f062574fc8c8 100644
--- a/crypto/rmd320.c
+++ b/crypto/rmd320.c
@@ -386,7 +386,7 @@ static void __exit rmd320_mod_fini(void)
crypto_unregister_shash(&alg);
}

-module_init(rmd320_mod_init);
+subsys_initcall(rmd320_mod_init);
module_exit(rmd320_mod_fini);

MODULE_LICENSE("GPL");
diff --git a/crypto/rsa.c b/crypto/rsa.c
index 4167980c243d4..4019e9af34c34 100644
--- a/crypto/rsa.c
+++ b/crypto/rsa.c
@@ -391,7 +391,7 @@ static void rsa_exit(void)
crypto_unregister_akcipher(&rsa);
}

-module_init(rsa_init);
+subsys_initcall(rsa_init);
module_exit(rsa_exit);
MODULE_ALIAS_CRYPTO("rsa");
MODULE_LICENSE("GPL");
diff --git a/crypto/salsa20_generic.c b/crypto/salsa20_generic.c
index 443fba09cbed7..657b68baa5412 100644
--- a/crypto/salsa20_generic.c
+++ b/crypto/salsa20_generic.c
@@ -203,7 +203,7 @@ static void __exit salsa20_generic_mod_fini(void)
crypto_unregister_skcipher(&alg);
}

-module_init(salsa20_generic_mod_init);
+subsys_initcall(salsa20_generic_mod_init);
module_exit(salsa20_generic_mod_fini);

MODULE_LICENSE("GPL");
diff --git a/crypto/seed.c b/crypto/seed.c
index c6ba8438be430..a75ac50fa4fd4 100644
--- a/crypto/seed.c
+++ b/crypto/seed.c
@@ -470,7 +470,7 @@ static void __exit seed_fini(void)
crypto_unregister_alg(&seed_alg);
}

-module_init(seed_init);
+subsys_initcall(seed_init);
module_exit(seed_fini);

MODULE_DESCRIPTION("SEED Cipher Algorithm");
diff --git a/crypto/seqiv.c b/crypto/seqiv.c
index ed1b0e9f24367..3f2fad615d265 100644
--- a/crypto/seqiv.c
+++ b/crypto/seqiv.c
@@ -211,7 +211,7 @@ static void __exit seqiv_module_exit(void)
crypto_unregister_template(&seqiv_tmpl);
}

-module_init(seqiv_module_init);
+subsys_initcall(seqiv_module_init);
module_exit(seqiv_module_exit);

MODULE_LICENSE("GPL");
diff --git a/crypto/serpent_generic.c b/crypto/serpent_generic.c
index 7c3382facc82e..ec4ec89ad1085 100644
--- a/crypto/serpent_generic.c
+++ b/crypto/serpent_generic.c
@@ -664,7 +664,7 @@ static void __exit serpent_mod_fini(void)
crypto_unregister_algs(srp_algs, ARRAY_SIZE(srp_algs));
}

-module_init(serpent_mod_init);
+subsys_initcall(serpent_mod_init);
module_exit(serpent_mod_fini);

MODULE_LICENSE("GPL");
diff --git a/crypto/sha1_generic.c b/crypto/sha1_generic.c
index 2af64ef81f402..1b806d4584b2f 100644
--- a/crypto/sha1_generic.c
+++ b/crypto/sha1_generic.c
@@ -92,7 +92,7 @@ static void __exit sha1_generic_mod_fini(void)
crypto_unregister_shash(&alg);
}

-module_init(sha1_generic_mod_init);
+subsys_initcall(sha1_generic_mod_init);
module_exit(sha1_generic_mod_fini);

MODULE_LICENSE("GPL");
diff --git a/crypto/sha256_generic.c b/crypto/sha256_generic.c
index 1e5ba6649e8db..5844e9a469e8a 100644
--- a/crypto/sha256_generic.c
+++ b/crypto/sha256_generic.c
@@ -301,7 +301,7 @@ static void __exit sha256_generic_mod_fini(void)
crypto_unregister_shashes(sha256_algs, ARRAY_SIZE(sha256_algs));
}

-module_init(sha256_generic_mod_init);
+subsys_initcall(sha256_generic_mod_init);
module_exit(sha256_generic_mod_fini);

MODULE_LICENSE("GPL");
diff --git a/crypto/sha3_generic.c b/crypto/sha3_generic.c
index 7ed98367d4fbb..60fd2be609d8d 100644
--- a/crypto/sha3_generic.c
+++ b/crypto/sha3_generic.c
@@ -294,7 +294,7 @@ static void __exit sha3_generic_mod_fini(void)
crypto_unregister_shashes(algs, ARRAY_SIZE(algs));
}

-module_init(sha3_generic_mod_init);
+subsys_initcall(sha3_generic_mod_init);
module_exit(sha3_generic_mod_fini);

MODULE_LICENSE("GPL");
diff --git a/crypto/sha512_generic.c b/crypto/sha512_generic.c
index 4097cd555eb6b..0193ecb8ae102 100644
--- a/crypto/sha512_generic.c
+++ b/crypto/sha512_generic.c
@@ -223,7 +223,7 @@ static void __exit sha512_generic_mod_fini(void)
crypto_unregister_shashes(sha512_algs, ARRAY_SIZE(sha512_algs));
}

-module_init(sha512_generic_mod_init);
+subsys_initcall(sha512_generic_mod_init);
module_exit(sha512_generic_mod_fini);

MODULE_LICENSE("GPL");
diff --git a/crypto/sm3_generic.c b/crypto/sm3_generic.c
index c0cf87ae7ef6d..e227bcada2a20 100644
--- a/crypto/sm3_generic.c
+++ b/crypto/sm3_generic.c
@@ -199,7 +199,7 @@ static void __exit sm3_generic_mod_fini(void)
crypto_unregister_shash(&sm3_alg);
}

-module_init(sm3_generic_mod_init);
+subsys_initcall(sm3_generic_mod_init);
module_exit(sm3_generic_mod_fini);

MODULE_LICENSE("GPL v2");
diff --git a/crypto/sm4_generic.c b/crypto/sm4_generic.c
index c18eebfd5edda..71ffb343709a5 100644
--- a/crypto/sm4_generic.c
+++ b/crypto/sm4_generic.c
@@ -237,7 +237,7 @@ static void __exit sm4_fini(void)
crypto_unregister_alg(&sm4_alg);
}

-module_init(sm4_init);
+subsys_initcall(sm4_init);
module_exit(sm4_fini);

MODULE_DESCRIPTION("SM4 Cipher Algorithm");
diff --git a/crypto/streebog_generic.c b/crypto/streebog_generic.c
index b82fc3d79aa15..63663c3bab7e9 100644
--- a/crypto/streebog_generic.c
+++ b/crypto/streebog_generic.c
@@ -1128,7 +1128,7 @@ static void __exit streebog_mod_fini(void)
crypto_unregister_shashes(algs, ARRAY_SIZE(algs));
}

-module_init(streebog_mod_init);
+subsys_initcall(streebog_mod_init);
module_exit(streebog_mod_fini);

MODULE_LICENSE("GPL");
diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index 1ea2d5007ff56..798253f05203e 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -3053,7 +3053,7 @@ static int __init tcrypt_mod_init(void)
*/
static void __exit tcrypt_mod_fini(void) { }

-module_init(tcrypt_mod_init);
+subsys_initcall(tcrypt_mod_init);
module_exit(tcrypt_mod_fini);

module_param(alg, charp, 0);
diff --git a/crypto/tea.c b/crypto/tea.c
index b70b441c7d1e7..786b589e13995 100644
--- a/crypto/tea.c
+++ b/crypto/tea.c
@@ -274,7 +274,7 @@ MODULE_ALIAS_CRYPTO("tea");
MODULE_ALIAS_CRYPTO("xtea");
MODULE_ALIAS_CRYPTO("xeta");

-module_init(tea_mod_init);
+subsys_initcall(tea_mod_init);
module_exit(tea_mod_fini);

MODULE_LICENSE("GPL");
diff --git a/crypto/tgr192.c b/crypto/tgr192.c
index f8e1d9f9938f5..40020f8adc46a 100644
--- a/crypto/tgr192.c
+++ b/crypto/tgr192.c
@@ -677,7 +677,7 @@ MODULE_ALIAS_CRYPTO("tgr192");
MODULE_ALIAS_CRYPTO("tgr160");
MODULE_ALIAS_CRYPTO("tgr128");

-module_init(tgr192_mod_init);
+subsys_initcall(tgr192_mod_init);
module_exit(tgr192_mod_fini);

MODULE_LICENSE("GPL");
diff --git a/crypto/twofish_generic.c b/crypto/twofish_generic.c
index 07e62433fbfb9..dbac6e2332858 100644
--- a/crypto/twofish_generic.c
+++ b/crypto/twofish_generic.c
@@ -205,7 +205,7 @@ static void __exit twofish_mod_fini(void)
crypto_unregister_alg(&alg);
}

-module_init(twofish_mod_init);
+subsys_initcall(twofish_mod_init);
module_exit(twofish_mod_fini);

MODULE_LICENSE("GPL");
diff --git a/crypto/vmac.c b/crypto/vmac.c
index 5f436dfdfc61e..f50a85060b39f 100644
--- a/crypto/vmac.c
+++ b/crypto/vmac.c
@@ -690,7 +690,7 @@ static void __exit vmac_module_exit(void)
crypto_unregister_template(&vmac64_tmpl);
}

-module_init(vmac_module_init);
+subsys_initcall(vmac_module_init);
module_exit(vmac_module_exit);

MODULE_LICENSE("GPL");
diff --git a/crypto/wp512.c b/crypto/wp512.c
index 149e577fb7726..1b8e502d999ff 100644
--- a/crypto/wp512.c
+++ b/crypto/wp512.c
@@ -1168,7 +1168,7 @@ MODULE_ALIAS_CRYPTO("wp512");
MODULE_ALIAS_CRYPTO("wp384");
MODULE_ALIAS_CRYPTO("wp256");

-module_init(wp512_mod_init);
+subsys_initcall(wp512_mod_init);
module_exit(wp512_mod_fini);

MODULE_LICENSE("GPL");
diff --git a/crypto/xcbc.c b/crypto/xcbc.c
index c055f57fab11a..94ca694ef091a 100644
--- a/crypto/xcbc.c
+++ b/crypto/xcbc.c
@@ -282,7 +282,7 @@ static void __exit crypto_xcbc_module_exit(void)
crypto_unregister_template(&crypto_xcbc_tmpl);
}

-module_init(crypto_xcbc_module_init);
+subsys_initcall(crypto_xcbc_module_init);
module_exit(crypto_xcbc_module_exit);

MODULE_LICENSE("GPL");
diff --git a/crypto/xts.c b/crypto/xts.c
index 847f54f767897..aed11e63ca315 100644
--- a/crypto/xts.c
+++ b/crypto/xts.c
@@ -359,7 +359,7 @@ static void __exit crypto_module_exit(void)
crypto_unregister_template(&crypto_tmpl);
}

-module_init(crypto_module_init);
+subsys_initcall(crypto_module_init);
module_exit(crypto_module_exit);

MODULE_LICENSE("GPL");
diff --git a/crypto/zstd.c b/crypto/zstd.c
index 9a76b3ed8b8bc..2c04055e407f0 100644
--- a/crypto/zstd.c
+++ b/crypto/zstd.c
@@ -257,7 +257,7 @@ static void __exit zstd_mod_fini(void)
crypto_unregister_scomp(&scomp);
}

-module_init(zstd_mod_init);
+subsys_initcall(zstd_mod_init);
module_exit(zstd_mod_fini);

MODULE_LICENSE("GPL");
--
2.21.0


2019-03-31 20:06:06

by Eric Biggers

[permalink] [raw]
Subject: [RFC/RFT PATCH 17/18] crypto: testmgr - fuzz AEADs against their generic implementation

From: Eric Biggers <[email protected]>

When the extra crypto self-tests are enabled, test each AEAD algorithm
against its generic implementation when one is available. This
involves: checking the algorithm properties for consistency, then
randomly generating test vectors using the generic implementation and
running them against the implementation under test. Both bad and good
inputs are tested.

Signed-off-by: Eric Biggers <[email protected]>
---
crypto/testmgr.c | 226 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 226 insertions(+)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 3fe178e32b0fe..5cb93ce4e7ed0 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -1759,6 +1759,223 @@ static int test_aead_vec(const char *driver, int enc,
return 0;
}

+#ifdef CONFIG_CRYPTO_MANAGER_EXTRA_TESTS
+/*
+ * Generate an AEAD test vector from the given implementation.
+ * Assumes the buffers in 'vec' were already allocated.
+ */
+static void generate_random_aead_testvec(struct aead_request *req,
+ struct aead_testvec *vec,
+ unsigned int maxkeysize,
+ unsigned int maxdatasize,
+ char *name, size_t max_namelen)
+{
+ struct crypto_aead *tfm = crypto_aead_reqtfm(req);
+ const unsigned int ivsize = crypto_aead_ivsize(tfm);
+ unsigned int maxauthsize = crypto_aead_alg(tfm)->maxauthsize;
+ unsigned int authsize;
+ unsigned int total_len;
+ int i;
+ struct scatterlist src[2], dst;
+ u8 iv[MAX_IVLEN];
+ DECLARE_CRYPTO_WAIT(wait);
+
+ /* Key: length in [0, maxkeysize], but usually choose maxkeysize */
+ vec->klen = maxkeysize;
+ if (prandom_u32() % 4 == 0)
+ vec->klen = prandom_u32() % (maxkeysize + 1);
+ generate_random_bytes((u8 *)vec->key, vec->klen);
+ vec->setkey_error = crypto_aead_setkey(tfm, vec->key, vec->klen);
+
+ /* IV */
+ generate_random_bytes((u8 *)vec->iv, ivsize);
+
+ /* Tag length: in [0, maxauthsize], but usually choose maxauthsize */
+ authsize = maxauthsize;
+ if (prandom_u32() % 4 == 0)
+ authsize = prandom_u32() % (maxauthsize + 1);
+ if (WARN_ON(authsize > maxdatasize))
+ authsize = maxdatasize;
+ maxdatasize -= authsize;
+ vec->setauthsize_error = crypto_aead_setauthsize(tfm, authsize);
+
+ /* Plaintext and associated data */
+ total_len = generate_random_length(maxdatasize);
+ vec->alen = generate_random_length(total_len);
+ vec->plen = total_len - vec->alen;
+ generate_random_bytes((u8 *)vec->assoc, vec->alen);
+ generate_random_bytes((u8 *)vec->ptext, vec->plen);
+
+ vec->clen = vec->plen + authsize;
+
+ /*
+ * If the key or authentication tag size couldn't be set, no need to
+ * continue to encrypt.
+ */
+ if (vec->setkey_error || vec->setauthsize_error)
+ goto done;
+
+ /* Ciphertext */
+ sg_init_table(src, 2);
+ i = 0;
+ if (vec->alen)
+ sg_set_buf(&src[i++], vec->assoc, vec->alen);
+ if (vec->plen)
+ sg_set_buf(&src[i++], vec->ptext, vec->plen);
+ sg_init_one(&dst, vec->ctext, vec->alen + vec->clen);
+ memcpy(iv, vec->iv, ivsize);
+ aead_request_set_callback(req, 0, crypto_req_done, &wait);
+ aead_request_set_crypt(req, src, &dst, vec->plen, iv);
+ aead_request_set_ad(req, vec->alen);
+ vec->crypt_error = crypto_wait_req(crypto_aead_encrypt(req), &wait);
+ if (vec->crypt_error == 0)
+ memmove((u8 *)vec->ctext, vec->ctext + vec->alen, vec->clen);
+done:
+ snprintf(name, max_namelen,
+ "\"random: alen=%u plen=%u authsize=%u klen=%u\"",
+ vec->alen, vec->plen, authsize, vec->klen);
+}
+
+/*
+ * Test the AEAD algorithm represented by @req against the corresponding generic
+ * implementation, if one is available.
+ */
+static int test_aead_vs_generic_impl(const char *driver,
+ const struct alg_test_desc *test_desc,
+ struct aead_request *req,
+ struct cipher_test_sglists *tsgls)
+{
+ struct crypto_aead *tfm = crypto_aead_reqtfm(req);
+ const unsigned int ivsize = crypto_aead_ivsize(tfm);
+ const unsigned int maxauthsize = crypto_aead_alg(tfm)->maxauthsize;
+ const unsigned int blocksize = crypto_aead_blocksize(tfm);
+ const unsigned int maxdatasize = (2 * PAGE_SIZE) - TESTMGR_POISON_LEN;
+ const char *algname = tfm->base.__crt_alg->cra_name;
+ const char *generic_driver = test_desc->generic_driver;
+ char _generic_driver[CRYPTO_MAX_ALG_NAME];
+ struct crypto_aead *generic_tfm = NULL;
+ struct aead_request *generic_req = NULL;
+ unsigned int maxkeysize;
+ unsigned int i;
+ struct aead_testvec vec = { 0 };
+ char vec_name[64];
+ struct testvec_config cfg;
+ char cfgname[TESTVEC_CONFIG_NAMELEN];
+ int err;
+
+ if (noextratests)
+ return 0;
+
+ if (!generic_driver) { /* Use default naming convention? */
+ err = build_generic_driver_name(algname, _generic_driver);
+ if (err)
+ return err;
+ generic_driver = _generic_driver;
+ }
+
+ if (strcmp(generic_driver, driver) == 0) /* Already the generic impl? */
+ return 0;
+
+ generic_tfm = crypto_alloc_aead(generic_driver, 0, 0);
+ if (IS_ERR(generic_tfm)) {
+ err = PTR_ERR(generic_tfm);
+ if (err == -ENOENT) {
+ pr_warn("alg: aead: skipping comparison tests for %s because %s is unavailable\n",
+ driver, generic_driver);
+ return 0;
+ }
+ pr_err("alg: aead: error allocating %s (generic impl of %s): %d\n",
+ generic_driver, algname, err);
+ return err;
+ }
+
+ generic_req = aead_request_alloc(generic_tfm, GFP_KERNEL);
+ if (!generic_req) {
+ err = -ENOMEM;
+ goto out;
+ }
+
+ /* Check the algorithm properties for consistency. */
+
+ if (maxauthsize != crypto_aead_alg(generic_tfm)->maxauthsize) {
+ pr_err("alg: aead: maxauthsize for %s (%u) doesn't match generic impl (%u)\n",
+ driver, maxauthsize,
+ crypto_aead_alg(generic_tfm)->maxauthsize);
+ err = -EINVAL;
+ goto out;
+ }
+
+ if (ivsize != crypto_aead_ivsize(generic_tfm)) {
+ pr_err("alg: aead: ivsize for %s (%u) doesn't match generic impl (%u)\n",
+ driver, ivsize, crypto_aead_ivsize(generic_tfm));
+ err = -EINVAL;
+ goto out;
+ }
+
+ if (blocksize != crypto_aead_blocksize(generic_tfm)) {
+ pr_err("alg: aead: blocksize for %s (%u) doesn't match generic impl (%u)\n",
+ driver, blocksize, crypto_aead_blocksize(generic_tfm));
+ err = -EINVAL;
+ goto out;
+ }
+
+ /*
+ * Now generate test vectors using the generic implementation, and test
+ * the other implementation against them.
+ */
+
+ maxkeysize = 0;
+ for (i = 0; i < test_desc->suite.aead.count; i++)
+ maxkeysize = max_t(unsigned int, maxkeysize,
+ test_desc->suite.aead.vecs[i].klen);
+
+ vec.key = kmalloc(maxkeysize, GFP_KERNEL);
+ vec.iv = kmalloc(ivsize, GFP_KERNEL);
+ vec.assoc = kmalloc(maxdatasize, GFP_KERNEL);
+ vec.ptext = kmalloc(maxdatasize, GFP_KERNEL);
+ vec.ctext = kmalloc(maxdatasize, GFP_KERNEL);
+ if (!vec.key || !vec.iv || !vec.assoc || !vec.ptext || !vec.ctext) {
+ err = -ENOMEM;
+ goto out;
+ }
+
+ for (i = 0; i < fuzz_iterations * 8; i++) {
+ generate_random_aead_testvec(generic_req, &vec,
+ maxkeysize, maxdatasize,
+ vec_name, sizeof(vec_name));
+ generate_random_testvec_config(&cfg, cfgname, sizeof(cfgname));
+
+ err = test_aead_vec_cfg(driver, ENCRYPT, &vec, vec_name, &cfg,
+ req, tsgls);
+ if (err)
+ goto out;
+ err = test_aead_vec_cfg(driver, DECRYPT, &vec, vec_name, &cfg,
+ req, tsgls);
+ if (err)
+ goto out;
+ cond_resched();
+ }
+ err = 0;
+out:
+ kfree(vec.key);
+ kfree(vec.iv);
+ kfree(vec.assoc);
+ kfree(vec.ptext);
+ kfree(vec.ctext);
+ crypto_free_aead(generic_tfm);
+ aead_request_free(generic_req);
+ return err;
+}
+#else /* !CONFIG_CRYPTO_MANAGER_EXTRA_TESTS */
+static int test_aead_vs_generic_impl(const char *driver,
+ const struct alg_test_desc *test_desc,
+ struct aead_request *req,
+ struct cipher_test_sglists *tsgls)
+{
+ return 0;
+}
+#endif /* !CONFIG_CRYPTO_MANAGER_EXTRA_TESTS */
+
static int test_aead(const char *driver, int enc,
const struct aead_test_suite *suite,
struct aead_request *req,
@@ -1818,6 +2035,10 @@ static int alg_test_aead(const struct alg_test_desc *desc, const char *driver,
goto out;

err = test_aead(driver, DECRYPT, suite, req, tsgls);
+ if (err)
+ goto out;
+
+ err = test_aead_vs_generic_impl(driver, desc, req, tsgls);
out:
free_cipher_test_sglists(tsgls);
aead_request_free(req);
@@ -3562,6 +3783,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}
}, {
.alg = "ccm(aes)",
+ .generic_driver = "ccm_base(ctr(aes-generic),cbcmac(aes-generic))",
.test = alg_test_aead,
.fips_allowed = 1,
.suite = {
@@ -3974,6 +4196,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}
}, {
.alg = "gcm(aes)",
+ .generic_driver = "gcm_base(ctr(aes-generic),ghash-generic)",
.test = alg_test_aead,
.fips_allowed = 1,
.suite = {
@@ -4245,6 +4468,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}
}, {
.alg = "rfc4106(gcm(aes))",
+ .generic_driver = "rfc4106(gcm_base(ctr(aes-generic),ghash-generic))",
.test = alg_test_aead,
.fips_allowed = 1,
.suite = {
@@ -4252,6 +4476,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}
}, {
.alg = "rfc4309(ccm(aes))",
+ .generic_driver = "rfc4309(ccm_base(ctr(aes-generic),cbcmac(aes-generic)))",
.test = alg_test_aead,
.fips_allowed = 1,
.suite = {
@@ -4259,6 +4484,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}
}, {
.alg = "rfc4543(gcm(aes))",
+ .generic_driver = "rfc4543(gcm_base(ctr(aes-generic),ghash-generic))",
.test = alg_test_aead,
.suite = {
.aead = __VECS(aes_gcm_rfc4543_tv_template)
--
2.21.0


2019-03-31 20:06:06

by Eric Biggers

[permalink] [raw]
Subject: [RFC/RFT PATCH 09/18] crypto: streebog - fix unaligned memory accesses

From: Eric Biggers <[email protected]>

Don't cast the data buffer directly to streebog_uint512, as this
violates alignment rules.

Fixes: fe18957e8e87 ("crypto: streebog - add Streebog hash function")
Cc: Vitaly Chikunov <[email protected]>
Signed-off-by: Eric Biggers <[email protected]>
---
crypto/streebog_generic.c | 25 +++++++++++++------------
include/crypto/streebog.h | 5 ++++-
2 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/crypto/streebog_generic.c b/crypto/streebog_generic.c
index 5a2eafed9c29f..b82fc3d79aa15 100644
--- a/crypto/streebog_generic.c
+++ b/crypto/streebog_generic.c
@@ -996,7 +996,7 @@ static void streebog_add512(const struct streebog_uint512 *x,

static void streebog_g(struct streebog_uint512 *h,
const struct streebog_uint512 *N,
- const u8 *m)
+ const struct streebog_uint512 *m)
{
struct streebog_uint512 Ki, data;
unsigned int i;
@@ -1005,7 +1005,7 @@ static void streebog_g(struct streebog_uint512 *h,

/* Starting E() */
Ki = data;
- streebog_xlps(&Ki, (const struct streebog_uint512 *)&m[0], &data);
+ streebog_xlps(&Ki, m, &data);

for (i = 0; i < 11; i++)
streebog_round(i, &Ki, &data);
@@ -1015,16 +1015,19 @@ static void streebog_g(struct streebog_uint512 *h,
/* E() done */

streebog_xor(&data, h, &data);
- streebog_xor(&data, (const struct streebog_uint512 *)&m[0], h);
+ streebog_xor(&data, m, h);
}

static void streebog_stage2(struct streebog_state *ctx, const u8 *data)
{
- streebog_g(&ctx->h, &ctx->N, data);
+ struct streebog_uint512 m;
+
+ memcpy(&m, data, sizeof(m));
+
+ streebog_g(&ctx->h, &ctx->N, &m);

streebog_add512(&ctx->N, &buffer512, &ctx->N);
- streebog_add512(&ctx->Sigma, (const struct streebog_uint512 *)data,
- &ctx->Sigma);
+ streebog_add512(&ctx->Sigma, &m, &ctx->Sigma);
}

static void streebog_stage3(struct streebog_state *ctx)
@@ -1034,13 +1037,11 @@ static void streebog_stage3(struct streebog_state *ctx)
buf.qword[0] = cpu_to_le64(ctx->fillsize << 3);
streebog_pad(ctx);

- streebog_g(&ctx->h, &ctx->N, (const u8 *)&ctx->buffer);
+ streebog_g(&ctx->h, &ctx->N, &ctx->m);
streebog_add512(&ctx->N, &buf, &ctx->N);
- streebog_add512(&ctx->Sigma,
- (const struct streebog_uint512 *)&ctx->buffer[0],
- &ctx->Sigma);
- streebog_g(&ctx->h, &buffer0, (const u8 *)&ctx->N);
- streebog_g(&ctx->h, &buffer0, (const u8 *)&ctx->Sigma);
+ streebog_add512(&ctx->Sigma, &ctx->m, &ctx->Sigma);
+ streebog_g(&ctx->h, &buffer0, &ctx->N);
+ streebog_g(&ctx->h, &buffer0, &ctx->Sigma);
memcpy(&ctx->hash, &ctx->h, sizeof(struct streebog_uint512));
}

diff --git a/include/crypto/streebog.h b/include/crypto/streebog.h
index 856e32af86574..cae1b4a019713 100644
--- a/include/crypto/streebog.h
+++ b/include/crypto/streebog.h
@@ -23,7 +23,10 @@ struct streebog_uint512 {
};

struct streebog_state {
- u8 buffer[STREEBOG_BLOCK_SIZE];
+ union {
+ u8 buffer[STREEBOG_BLOCK_SIZE];
+ struct streebog_uint512 m;
+ };
struct streebog_uint512 hash;
struct streebog_uint512 h;
struct streebog_uint512 N;
--
2.21.0


2019-03-31 20:06:08

by Eric Biggers

[permalink] [raw]
Subject: [RFC/RFT PATCH 11/18] crypto: arm64/cbcmac - handle empty messages in same way as template

From: Eric Biggers <[email protected]>

My patches to make testmgr fuzz algorithms against their generic
implementation detected that the arm64 implementations of "cbcmac(aes)"
handle empty messages differently from the cbcmac template. Namely, the
arm64 implementations return the encrypted initial value, but the cbcmac
template returns the initial value directly.

This isn't actually a meaningful case because any user of cbcmac needs
to prepend the message length, as CCM does; otherwise it's insecure.
However, we should keep the behavior consistent; at the very least this
makes testing easier.

Do it the easy way, which is to change the arm64 implementations to have
the same behavior as the cbcmac template.

For what it's worth, ghash does things essentially the same way: it
returns its initial value when given an empty message, even though in
practice ghash is never passed an empty message.

Signed-off-by: Eric Biggers <[email protected]>
---
arch/arm64/crypto/aes-glue.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/crypto/aes-glue.c b/arch/arm64/crypto/aes-glue.c
index 692cb75f2ca2f..f0ceb545bd1ee 100644
--- a/arch/arm64/crypto/aes-glue.c
+++ b/arch/arm64/crypto/aes-glue.c
@@ -707,7 +707,7 @@ static int cbcmac_final(struct shash_desc *desc, u8 *out)
struct mac_tfm_ctx *tctx = crypto_shash_ctx(desc->tfm);
struct mac_desc_ctx *ctx = shash_desc_ctx(desc);

- mac_do_update(&tctx->key, NULL, 0, ctx->dg, 1, 0);
+ mac_do_update(&tctx->key, NULL, 0, ctx->dg, (ctx->len != 0), 0);

memcpy(out, ctx->dg, AES_BLOCK_SIZE);

--
2.21.0


2019-03-31 20:06:09

by Eric Biggers

[permalink] [raw]
Subject: [RFC/RFT PATCH 06/18] crypto: chacha20poly1305 - set cra_name correctly

From: Eric Biggers <[email protected]>

If the rfc7539 template is instantiated with specific implementations,
e.g. "rfc7539(chacha20-generic,poly1305-generic)" rather than
"rfc7539(chacha20,poly1305)", then the implementation names end up
included in the instance's cra_name. This is incorrect because it then
prevents all users from allocating "rfc7539(chacha20,poly1305)", if the
highest priority implementations of chacha20 and poly1305 were selected.
Also, the self-tests aren't run on an instance allocated in this way.

Fix it by setting the instance's cra_name from the underlying
algorithms' actual cra_names, rather than from the requested names.
This matches what other templates do.

Fixes: 71ebc4d1b27d ("crypto: chacha20poly1305 - Add a ChaCha20-Poly1305 AEAD construction, RFC7539")
Cc: <[email protected]> # v4.2+
Cc: Martin Willi <[email protected]>
Signed-off-by: Eric Biggers <[email protected]>
---
crypto/chacha20poly1305.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/crypto/chacha20poly1305.c b/crypto/chacha20poly1305.c
index ed2e12e26dd80..279d816ab51dd 100644
--- a/crypto/chacha20poly1305.c
+++ b/crypto/chacha20poly1305.c
@@ -645,8 +645,8 @@ static int chachapoly_create(struct crypto_template *tmpl, struct rtattr **tb,

err = -ENAMETOOLONG;
if (snprintf(inst->alg.base.cra_name, CRYPTO_MAX_ALG_NAME,
- "%s(%s,%s)", name, chacha_name,
- poly_name) >= CRYPTO_MAX_ALG_NAME)
+ "%s(%s,%s)", name, chacha->base.cra_name,
+ poly->cra_name) >= CRYPTO_MAX_ALG_NAME)
goto out_drop_chacha;
if (snprintf(inst->alg.base.cra_driver_name, CRYPTO_MAX_ALG_NAME,
"%s(%s,%s)", name, chacha->base.cra_driver_name,
--
2.21.0


2019-03-31 21:47:24

by Vitaly Chikunov

[permalink] [raw]
Subject: Re: [RFC/RFT PATCH 09/18] crypto: streebog - fix unaligned memory accesses

Eric,

On Sun, Mar 31, 2019 at 01:04:19PM -0700, Eric Biggers wrote:
> From: Eric Biggers <[email protected]>
>
> Don't cast the data buffer directly to streebog_uint512, as this
> violates alignment rules.
>
> Fixes: fe18957e8e87 ("crypto: streebog - add Streebog hash function")
> Cc: Vitaly Chikunov <[email protected]>
> Signed-off-by: Eric Biggers <[email protected]>
> ---
> crypto/streebog_generic.c | 25 +++++++++++++------------
> include/crypto/streebog.h | 5 ++++-
> 2 files changed, 17 insertions(+), 13 deletions(-)
>
> diff --git a/crypto/streebog_generic.c b/crypto/streebog_generic.c
> index 5a2eafed9c29f..b82fc3d79aa15 100644
> --- a/crypto/streebog_generic.c
> +++ b/crypto/streebog_generic.c
> @@ -996,7 +996,7 @@ static void streebog_add512(const struct streebog_uint512 *x,
>
> static void streebog_g(struct streebog_uint512 *h,
> const struct streebog_uint512 *N,
> - const u8 *m)
> + const struct streebog_uint512 *m)
> {
> struct streebog_uint512 Ki, data;
> unsigned int i;
> @@ -1005,7 +1005,7 @@ static void streebog_g(struct streebog_uint512 *h,
>
> /* Starting E() */
> Ki = data;
> - streebog_xlps(&Ki, (const struct streebog_uint512 *)&m[0], &data);
> + streebog_xlps(&Ki, m, &data);
>
> for (i = 0; i < 11; i++)
> streebog_round(i, &Ki, &data);
> @@ -1015,16 +1015,19 @@ static void streebog_g(struct streebog_uint512 *h,
> /* E() done */
>
> streebog_xor(&data, h, &data);
> - streebog_xor(&data, (const struct streebog_uint512 *)&m[0], h);
> + streebog_xor(&data, m, h);
> }
>
> static void streebog_stage2(struct streebog_state *ctx, const u8 *data)
> {
> - streebog_g(&ctx->h, &ctx->N, data);
> + struct streebog_uint512 m;
> +
> + memcpy(&m, data, sizeof(m));
> +
> + streebog_g(&ctx->h, &ctx->N, &m);
>
> streebog_add512(&ctx->N, &buffer512, &ctx->N);
> - streebog_add512(&ctx->Sigma, (const struct streebog_uint512 *)data,
> - &ctx->Sigma);
> + streebog_add512(&ctx->Sigma, &m, &ctx->Sigma);
> }

As I understand, this is the actual fix.

Reviewed-by: Vitaly Chikunov <[email protected]>

Thanks much!

>
> static void streebog_stage3(struct streebog_state *ctx)
> @@ -1034,13 +1037,11 @@ static void streebog_stage3(struct streebog_state *ctx)
> buf.qword[0] = cpu_to_le64(ctx->fillsize << 3);
> streebog_pad(ctx);
>
> - streebog_g(&ctx->h, &ctx->N, (const u8 *)&ctx->buffer);
> + streebog_g(&ctx->h, &ctx->N, &ctx->m);
> streebog_add512(&ctx->N, &buf, &ctx->N);
> - streebog_add512(&ctx->Sigma,
> - (const struct streebog_uint512 *)&ctx->buffer[0],
> - &ctx->Sigma);
> - streebog_g(&ctx->h, &buffer0, (const u8 *)&ctx->N);
> - streebog_g(&ctx->h, &buffer0, (const u8 *)&ctx->Sigma);
> + streebog_add512(&ctx->Sigma, &ctx->m, &ctx->Sigma);
> + streebog_g(&ctx->h, &buffer0, &ctx->N);
> + streebog_g(&ctx->h, &buffer0, &ctx->Sigma);
> memcpy(&ctx->hash, &ctx->h, sizeof(struct streebog_uint512));
> }
>
> diff --git a/include/crypto/streebog.h b/include/crypto/streebog.h
> index 856e32af86574..cae1b4a019713 100644
> --- a/include/crypto/streebog.h
> +++ b/include/crypto/streebog.h
> @@ -23,7 +23,10 @@ struct streebog_uint512 {
> };
>
> struct streebog_state {
> - u8 buffer[STREEBOG_BLOCK_SIZE];
> + union {
> + u8 buffer[STREEBOG_BLOCK_SIZE];
> + struct streebog_uint512 m;
> + };
> struct streebog_uint512 hash;
> struct streebog_uint512 h;
> struct streebog_uint512 N;
> --
> 2.21.0

2019-04-01 07:58:26

by Martin Willi

[permalink] [raw]
Subject: Re: [RFC/RFT PATCH 01/18] crypto: x86/poly1305 - fix overflow during partial reduction

Hi,

> The x86_64 implementation of Poly1305 produces the wrong result on
> some inputs because poly1305_4block_avx2() incorrectly assumes that
> when partially reducing the accumulator, the bits carried from limb
> 'd4' to limb 'h0' fit in a 32-bit integer.

> [...] This bug was originally detected by my patches that improve
> testmgr to fuzz algorithms against their generic implementation.

Thanks Eric. This shows how valuable your continued work on the crypto
testing code is, and how useful such a (common) testing infrastructure
can be.

Reviewed-by: Martin Willi <[email protected]>


2019-04-01 08:03:24

by Martin Willi

[permalink] [raw]
Subject: Re: [RFC/RFT PATCH 06/18] crypto: chacha20poly1305 - set cra_name correctly


> If the rfc7539 template is instantiated with specific
> implementations, e.g. "rfc7539(chacha20-generic,poly1305-generic)"
> rather than "rfc7539(chacha20,poly1305)", then the implementation
> names end up included in the instance's cra_name. This is incorrect
> [...]

Reviewed-by: Martin Willi <[email protected]>

Thanks,
Martin


2019-04-01 15:56:36

by Eric Biggers

[permalink] [raw]
Subject: Re: [RFC/RFT PATCH 07/18] crypto: gcm - fix incompatibility between "gcm" and "gcm_base"

On Sun, Mar 31, 2019 at 01:04:17PM -0700, Eric Biggers wrote:
> From: Eric Biggers <[email protected]>
>
> GCM instances can be created by either the "gcm" template, which only
> allows choosing the block cipher, e.g. "gcm(aes)"; or by "gcm_base",
> which allows choosing the ctr and ghash implementations, e.g.
> "gcm_base(ctr(aes-generic),ghash-generic)".
>
> However, a "gcm_base" instance prevents a "gcm" instance from being
> registered using the same implementations. Nor will the instance be
> found by lookups of "gcm". This can be used as a denial of service.
> Moreover, "gcm_base" instances are never tested by the crypto
> self-tests, even if there are compatible "gcm" tests.
>
> The root cause of these problems is that instances of the two templates
> use different cra_names. Therefore, fix these problems by making
> "gcm_base" instances set the same cra_name as "gcm" instances, e.g.
> "gcm(aes)" instead of "gcm_base(ctr(aes-generic),ghash-generic)".
>
> This requires extracting the block cipher name from the name of the ctr
> algorithm, which means starting to require that the stream cipher really
> be ctr and not something else. But it would be pretty bizarre if anyone
> was actually relying on being able to use a non-ctr stream cipher here.
>
> Fixes: d00aa19b507b ("[CRYPTO] gcm: Allow block cipher parameter")
> Cc: [email protected]
> Signed-off-by: Eric Biggers <[email protected]>
> ---
> crypto/gcm.c | 30 ++++++++----------------------
> 1 file changed, 8 insertions(+), 22 deletions(-)
>
> diff --git a/crypto/gcm.c b/crypto/gcm.c
> index e1a11f529d257..12ff5ed569272 100644
> --- a/crypto/gcm.c
> +++ b/crypto/gcm.c
> @@ -597,7 +597,6 @@ static void crypto_gcm_free(struct aead_instance *inst)
>
> static int crypto_gcm_create_common(struct crypto_template *tmpl,
> struct rtattr **tb,
> - const char *full_name,
> const char *ctr_name,
> const char *ghash_name)
> {
> @@ -650,24 +649,23 @@ static int crypto_gcm_create_common(struct crypto_template *tmpl,
>
> ctr = crypto_spawn_skcipher_alg(&ctx->ctr);
>
> - /* We only support 16-byte blocks. */
> + /* Must be CTR mode with 16-byte blocks. */
> err = -EINVAL;
> - if (crypto_skcipher_alg_ivsize(ctr) != 16)
> + if (strncmp(ctr->base.cra_name, "ctr(", 4) != 0 ||
> + crypto_skcipher_alg_ivsize(ctr) != 16)
> goto out_put_ctr;
>
> - /* Not a stream cipher? */
> - if (ctr->base.cra_blocksize != 1)
> + err = -ENAMETOOLONG;
> + if (snprintf(inst->alg.base.cra_name, CRYPTO_MAX_ALG_NAME,
> + "gcm(%s", ctr->base.cra_name + 4) >= CRYPTO_MAX_ALG_NAME)
> goto out_put_ctr;
>
> - err = -ENAMETOOLONG;
> if (snprintf(inst->alg.base.cra_driver_name, CRYPTO_MAX_ALG_NAME,
> "gcm_base(%s,%s)", ctr->base.cra_driver_name,
> ghash_alg->cra_driver_name) >=
> CRYPTO_MAX_ALG_NAME)
> goto out_put_ctr;
>
> - memcpy(inst->alg.base.cra_name, full_name, CRYPTO_MAX_ALG_NAME);
> -
> inst->alg.base.cra_flags = (ghash->base.cra_flags |
> ctr->base.cra_flags) & CRYPTO_ALG_ASYNC;
> inst->alg.base.cra_priority = (ghash->base.cra_priority +
> @@ -709,7 +707,6 @@ static int crypto_gcm_create(struct crypto_template *tmpl, struct rtattr **tb)
> {
> const char *cipher_name;
> char ctr_name[CRYPTO_MAX_ALG_NAME];
> - char full_name[CRYPTO_MAX_ALG_NAME];
>
> cipher_name = crypto_attr_alg_name(tb[1]);
> if (IS_ERR(cipher_name))
> @@ -719,12 +716,7 @@ static int crypto_gcm_create(struct crypto_template *tmpl, struct rtattr **tb)
> CRYPTO_MAX_ALG_NAME)
> return -ENAMETOOLONG;
>
> - if (snprintf(full_name, CRYPTO_MAX_ALG_NAME, "gcm(%s)", cipher_name) >=
> - CRYPTO_MAX_ALG_NAME)
> - return -ENAMETOOLONG;
> -
> - return crypto_gcm_create_common(tmpl, tb, full_name,
> - ctr_name, "ghash");
> + return crypto_gcm_create_common(tmpl, tb, ctr_name, "ghash");
> }
>
> static int crypto_gcm_base_create(struct crypto_template *tmpl,
> @@ -732,7 +724,6 @@ static int crypto_gcm_base_create(struct crypto_template *tmpl,
> {
> const char *ctr_name;
> const char *ghash_name;
> - char full_name[CRYPTO_MAX_ALG_NAME];
>
> ctr_name = crypto_attr_alg_name(tb[1]);
> if (IS_ERR(ctr_name))
> @@ -742,12 +733,7 @@ static int crypto_gcm_base_create(struct crypto_template *tmpl,
> if (IS_ERR(ghash_name))
> return PTR_ERR(ghash_name);
>
> - if (snprintf(full_name, CRYPTO_MAX_ALG_NAME, "gcm_base(%s,%s)",
> - ctr_name, ghash_name) >= CRYPTO_MAX_ALG_NAME)
> - return -ENAMETOOLONG;
> -
> - return crypto_gcm_create_common(tmpl, tb, full_name,
> - ctr_name, ghash_name);
> + return crypto_gcm_create_common(tmpl, tb, ctr_name, ghash_name);
> }
>
> static int crypto_rfc4106_setkey(struct crypto_aead *parent, const u8 *key,
> --
> 2.21.0
>

Oops, I think there's a bug here: we also need to check that the hash algorithm
passed to gcm_base is really "ghash". Similarly, in the next patch, ccm_base
requires "cbcmac".

- Eric

2019-04-02 15:48:33

by Sasha Levin

[permalink] [raw]
Subject: Re: [RFC/RFT PATCH 08/18] crypto: ccm - fix incompatibility between "ccm" and "ccm_base"

Hi,

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag,
fixing commit: 4a49b499dfa0 [CRYPTO] ccm: Added CCM mode.

The bot has tested the following trees: v5.0.5, v4.19.32, v4.14.109, v4.9.166, v4.4.177, v3.18.137.

v5.0.5: Build OK!
v4.19.32: Build OK!
v4.14.109: Build OK!
v4.9.166: Failed to apply! Possible dependencies:
f15f05b0a5de ("crypto: ccm - switch to separate cbcmac driver")

v4.4.177: Failed to apply! Possible dependencies:
464b93a3c7d1 ("crypto: ccm - Use skcipher")
f15f05b0a5de ("crypto: ccm - switch to separate cbcmac driver")

v3.18.137: Failed to apply! Possible dependencies:
2c221ad39424 ("crypto: ccm - Use crypto_aead_set_reqsize helper")
464b93a3c7d1 ("crypto: ccm - Use skcipher")
81c4c35eb61a ("crypto: ccm - Convert to new AEAD interface")
f15f05b0a5de ("crypto: ccm - switch to separate cbcmac driver")


How should we proceed with this patch?

--
Thanks,
Sasha

2019-04-02 16:16:05

by Vitaly Chikunov

[permalink] [raw]
Subject: Re: [RFC/RFT PATCH 09/18] crypto: streebog - fix unaligned memory accesses

Eric,

On Mon, Apr 01, 2019 at 12:47:19AM +0300, Vitaly Chikunov wrote:
> On Sun, Mar 31, 2019 at 01:04:19PM -0700, Eric Biggers wrote:
> > From: Eric Biggers <[email protected]>
> >
> > Don't cast the data buffer directly to streebog_uint512, as this
> > violates alignment rules.
> >
> > Fixes: fe18957e8e87 ("crypto: streebog - add Streebog hash function")
> > Cc: Vitaly Chikunov <[email protected]>
> > Signed-off-by: Eric Biggers <[email protected]>
> > ---
> > crypto/streebog_generic.c | 25 +++++++++++++------------
> > include/crypto/streebog.h | 5 ++++-
> > 2 files changed, 17 insertions(+), 13 deletions(-)
> >
> > diff --git a/crypto/streebog_generic.c b/crypto/streebog_generic.c
> > index 5a2eafed9c29f..b82fc3d79aa15 100644
> > --- a/crypto/streebog_generic.c
> > +++ b/crypto/streebog_generic.c
> > @@ -996,7 +996,7 @@ static void streebog_add512(const struct streebog_uint512 *x,
> >
> > static void streebog_g(struct streebog_uint512 *h,
> > const struct streebog_uint512 *N,
> > - const u8 *m)
> > + const struct streebog_uint512 *m)
> > {
> > struct streebog_uint512 Ki, data;
> > unsigned int i;
> > @@ -1005,7 +1005,7 @@ static void streebog_g(struct streebog_uint512 *h,
> >
> > /* Starting E() */
> > Ki = data;
> > - streebog_xlps(&Ki, (const struct streebog_uint512 *)&m[0], &data);
> > + streebog_xlps(&Ki, m, &data);
> >
> > for (i = 0; i < 11; i++)
> > streebog_round(i, &Ki, &data);
> > @@ -1015,16 +1015,19 @@ static void streebog_g(struct streebog_uint512 *h,
> > /* E() done */
> >
> > streebog_xor(&data, h, &data);
> > - streebog_xor(&data, (const struct streebog_uint512 *)&m[0], h);
> > + streebog_xor(&data, m, h);
> > }
> >
> > static void streebog_stage2(struct streebog_state *ctx, const u8 *data)
> > {
> > - streebog_g(&ctx->h, &ctx->N, data);
> > + struct streebog_uint512 m;
> > +
> > + memcpy(&m, data, sizeof(m));
> > +
> > + streebog_g(&ctx->h, &ctx->N, &m);
> >
> > streebog_add512(&ctx->N, &buffer512, &ctx->N);
> > - streebog_add512(&ctx->Sigma, (const struct streebog_uint512 *)data,
> > - &ctx->Sigma);
> > + streebog_add512(&ctx->Sigma, &m, &ctx->Sigma);
> > }
>
> As I understand, this is the actual fix.

Probably, even better would be to use CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
to optimize out memcpy() for such architectures.

Thanks,

> Reviewed-by: Vitaly Chikunov <[email protected]>
>
> Thanks much!
>
> >
> > static void streebog_stage3(struct streebog_state *ctx)
> > @@ -1034,13 +1037,11 @@ static void streebog_stage3(struct streebog_state *ctx)
> > buf.qword[0] = cpu_to_le64(ctx->fillsize << 3);
> > streebog_pad(ctx);
> >
> > - streebog_g(&ctx->h, &ctx->N, (const u8 *)&ctx->buffer);
> > + streebog_g(&ctx->h, &ctx->N, &ctx->m);
> > streebog_add512(&ctx->N, &buf, &ctx->N);
> > - streebog_add512(&ctx->Sigma,
> > - (const struct streebog_uint512 *)&ctx->buffer[0],
> > - &ctx->Sigma);
> > - streebog_g(&ctx->h, &buffer0, (const u8 *)&ctx->N);
> > - streebog_g(&ctx->h, &buffer0, (const u8 *)&ctx->Sigma);
> > + streebog_add512(&ctx->Sigma, &ctx->m, &ctx->Sigma);
> > + streebog_g(&ctx->h, &buffer0, &ctx->N);
> > + streebog_g(&ctx->h, &buffer0, &ctx->Sigma);
> > memcpy(&ctx->hash, &ctx->h, sizeof(struct streebog_uint512));
> > }
> >
> > diff --git a/include/crypto/streebog.h b/include/crypto/streebog.h
> > index 856e32af86574..cae1b4a019713 100644
> > --- a/include/crypto/streebog.h
> > +++ b/include/crypto/streebog.h
> > @@ -23,7 +23,10 @@ struct streebog_uint512 {
> > };
> >
> > struct streebog_state {
> > - u8 buffer[STREEBOG_BLOCK_SIZE];
> > + union {
> > + u8 buffer[STREEBOG_BLOCK_SIZE];
> > + struct streebog_uint512 m;
> > + };
> > struct streebog_uint512 hash;
> > struct streebog_uint512 h;
> > struct streebog_uint512 N;
> > --
> > 2.21.0

2019-04-02 16:57:53

by Eric Biggers

[permalink] [raw]
Subject: Re: [RFC/RFT PATCH 09/18] crypto: streebog - fix unaligned memory accesses

On Tue, Apr 02, 2019 at 07:15:57PM +0300, Vitaly Chikunov wrote:
> > >
> > > static void streebog_stage2(struct streebog_state *ctx, const u8 *data)
> > > {
> > > - streebog_g(&ctx->h, &ctx->N, data);
> > > + struct streebog_uint512 m;
> > > +
> > > + memcpy(&m, data, sizeof(m));
> > > +
> > > + streebog_g(&ctx->h, &ctx->N, &m);
> > >
> > > streebog_add512(&ctx->N, &buffer512, &ctx->N);
> > > - streebog_add512(&ctx->Sigma, (const struct streebog_uint512 *)data,
> > > - &ctx->Sigma);
> > > + streebog_add512(&ctx->Sigma, &m, &ctx->Sigma);
> > > }
> >
> > As I understand, this is the actual fix.
>
> Probably, even better would be to use CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
> to optimize out memcpy() for such architectures.
>

Having multiple code paths is more error-prone, and contrary to popular belief
you can't break alignment rules without informing the compiler, even when
CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS. See
https://patchwork.kernel.org/cover/10631429/.

If you want to code up something yourself using get_unaligned_le64() or
__attribute__((packed)), that probably would be the way to go. But for now I
just want to fix it to not cause a test failure. I don't have any particular
interest in optimizing Streebog myself, especially the C implementation (if you
really cared about performance you'd add an assembly implementation).

- Eric

2019-04-08 05:53:29

by Herbert Xu

[permalink] [raw]
Subject: Re: [RFC/RFT PATCH 18/18] crypto: run initcalls for generic implementations earlier

Hi Eric:

Eric Biggers <[email protected]> wrote:
> From: Eric Biggers <[email protected]>
>
> Use subsys_initcall for registration of all templates and generic
> algorithm implementations, rather than module_init.

I think this is fine except that algboss already hooks in at
subsys_initcall and either it needs to move further up or we
need to change these ones to occur after subsys_initcall.

Otherwise the generic algorithms themselves may not get tested
if algboss isn't loaded first.

Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2019-04-08 06:24:00

by Herbert Xu

[permalink] [raw]
Subject: Re: [RFC/RFT PATCH 04/18] crypto: skcipher - restore default skcipher_walk::iv on error

Eric Biggers <[email protected]> wrote:
> From: Eric Biggers <[email protected]>
>
> When the user-provided IV buffer is not aligned to the algorithm's
> alignmask, skcipher_walk_virt() allocates an aligned buffer and copies
> the IV into it. However, skcipher_walk_virt() can fail after that
> point, and in this case the buffer will be freed.
>
> This causes a use-after-free read in callers that read from walk->iv
> unconditionally, e.g. the LRW template. For example, this can be
> reproduced by trying to encrypt fewer than 16 bytes using "lrw(aes)".

This looks like a bug in LRW. Relying on walk->iv to be set to
anything after a failed skcipher_walk_virt call is wrong. So we
should fix it there instead.

Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2019-04-08 06:44:24

by Herbert Xu

[permalink] [raw]
Subject: Re: [RFC/RFT PATCH 00/18] crypto: fuzz algorithms against their generic implementation

Eric Biggers <[email protected]> wrote:
>
> As usual, the series begins with fixes for the bugs found, roughly in
> order of decreasing importance. Please consider sending the Poly1305
> fix to Linus soon.

I have applied patches 1-3, 5-6, 9-11.

Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2019-04-08 17:27:40

by Eric Biggers

[permalink] [raw]
Subject: Re: [RFC/RFT PATCH 04/18] crypto: skcipher - restore default skcipher_walk::iv on error

On Mon, Apr 08, 2019 at 02:23:54PM +0800, Herbert Xu wrote:
> Eric Biggers <[email protected]> wrote:
> > From: Eric Biggers <[email protected]>
> >
> > When the user-provided IV buffer is not aligned to the algorithm's
> > alignmask, skcipher_walk_virt() allocates an aligned buffer and copies
> > the IV into it. However, skcipher_walk_virt() can fail after that
> > point, and in this case the buffer will be freed.
> >
> > This causes a use-after-free read in callers that read from walk->iv
> > unconditionally, e.g. the LRW template. For example, this can be
> > reproduced by trying to encrypt fewer than 16 bytes using "lrw(aes)".
>
> This looks like a bug in LRW. Relying on walk->iv to be set to
> anything after a failed skcipher_walk_virt call is wrong. So we
> should fix it there instead.
>
> Cheers,
> --

It's not just LRW though. It's actually 7 places:

arch/arm/crypto/aes-neonbs-glue.c
arch/arm/crypto/chacha-neon-glue.c
arch/arm64/crypto/aes-neonbs-glue.c
arch/arm64/crypto/chacha-neon-glue.c
crypto/chacha-generic.c
crypto/lrw.c
crypto/salsa20-generic.c

Do you prefer that all those be updated?

- Eric

2019-04-08 17:44:06

by Eric Biggers

[permalink] [raw]
Subject: Re: [RFC/RFT PATCH 18/18] crypto: run initcalls for generic implementations earlier

On Mon, Apr 08, 2019 at 01:53:22PM +0800, Herbert Xu wrote:
> Hi Eric:
>
> Eric Biggers <[email protected]> wrote:
> > From: Eric Biggers <[email protected]>
> >
> > Use subsys_initcall for registration of all templates and generic
> > algorithm implementations, rather than module_init.
>
> I think this is fine except that algboss already hooks in at
> subsys_initcall and either it needs to move further up or we
> need to change these ones to occur after subsys_initcall.
>
> Otherwise the generic algorithms themselves may not get tested
> if algboss isn't loaded first.
>
> Thanks,
> --
> Email: Herbert Xu <[email protected]>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

My patch also moved cryptomgr earlier in the Makefile, so it's linked first. Do
you think that's too fragile? Otherwise, I'll probably change cryptomgr_init()
to arch_initcall().

BTW, I found another problem caused by allocating the generic implementation
during the self-tests, which is that the generic implementation will fulfill any
outstanding allocation requests before the other implementation has finished
testing. E.g. on x86_64, the first call to

crypto_alloc_skcipher("adiantum(xchacha12,aes)", 0, 0)

will get "adiantum(xchacha12-generic,aes-generic,nhpoly1305-generic)" rather
than "adiantum(xchacha12-simd,aes-aesni,nhpoly1305-avx2)" as expected. Do you
have any suggestion on the best way to fix this?

- Eric

2019-04-09 06:24:14

by Herbert Xu

[permalink] [raw]
Subject: Re: [RFC/RFT PATCH 18/18] crypto: run initcalls for generic implementations earlier

On Mon, Apr 08, 2019 at 10:44:02AM -0700, Eric Biggers wrote:
>
> My patch also moved cryptomgr earlier in the Makefile, so it's linked first. Do
> you think that's too fragile? Otherwise, I'll probably change cryptomgr_init()
> to arch_initcall().

I don't think we should rely on link-time ordering to fulfil
this as it will just fail silently when it breaks.

> BTW, I found another problem caused by allocating the generic implementation
> during the self-tests, which is that the generic implementation will fulfill any
> outstanding allocation requests before the other implementation has finished
> testing. E.g. on x86_64, the first call to
>
> crypto_alloc_skcipher("adiantum(xchacha12,aes)", 0, 0)
>
> will get "adiantum(xchacha12-generic,aes-generic,nhpoly1305-generic)" rather
> than "adiantum(xchacha12-simd,aes-aesni,nhpoly1305-avx2)" as expected. Do you
> have any suggestion on the best way to fix this?

Well this is to be expected. Normally we don't start the allocation
until all probing is done. That is, all modules should have been
loaded before (or rather during) the first allocation call. If it
occurs during the first allocation call then a larval algorithm will
hold any subsequent allocations until the testing is done.

So the question is who is doing the allocation prior the registration
of the accelerated algorithms? Is it the fuzzing test?

Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2019-04-09 06:37:46

by Herbert Xu

[permalink] [raw]
Subject: Re: [RFC/RFT PATCH 04/18] crypto: skcipher - restore default skcipher_walk::iv on error

On Mon, Apr 08, 2019 at 10:27:37AM -0700, Eric Biggers wrote:
>
> It's not just LRW though. It's actually 7 places:
>
> arch/arm/crypto/aes-neonbs-glue.c
> arch/arm/crypto/chacha-neon-glue.c
> arch/arm64/crypto/aes-neonbs-glue.c
> arch/arm64/crypto/chacha-neon-glue.c
> crypto/chacha-generic.c
> crypto/lrw.c
> crypto/salsa20-generic.c
>
> Do you prefer that all those be updated?

We have to because if memory allocation fails then walk->iv will
just be the origial IV. If you can use the original IV then you
might as well just do that.

I just checked chacha-generic and it is fine as it's using the
original IV and not walk->iv (the difference is that one may be
unaligned while the other is guaranteed to be aligned).

arm*/chacha-neon-glue.c are also correct.

salsa20 is indeed broken but the fix is trivial since it's already
using unaligned loads.

arm/aes-neonbs-glue seems easily fixed as it can simply use the
unaligned original IV since it's going through the crypto API
again.

arm64/aes-neonbs-glue I'm not quite sure as it's calling into
assembly so depending on whether that wants aligned/unaligned
this can either use the original IV or check for errors and
abort if necessary.

Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2019-04-09 18:16:13

by Eric Biggers

[permalink] [raw]
Subject: Re: [RFC/RFT PATCH 18/18] crypto: run initcalls for generic implementations earlier

Hi Herbert,

On Tue, Apr 09, 2019 at 02:24:09PM +0800, Herbert Xu wrote:
>
> > BTW, I found another problem caused by allocating the generic implementation
> > during the self-tests, which is that the generic implementation will fulfill any
> > outstanding allocation requests before the other implementation has finished
> > testing. E.g. on x86_64, the first call to
> >
> > crypto_alloc_skcipher("adiantum(xchacha12,aes)", 0, 0)
> >
> > will get "adiantum(xchacha12-generic,aes-generic,nhpoly1305-generic)" rather
> > than "adiantum(xchacha12-simd,aes-aesni,nhpoly1305-avx2)" as expected. Do you
> > have any suggestion on the best way to fix this?
>
> Well this is to be expected. Normally we don't start the allocation
> until all probing is done. That is, all modules should have been
> loaded before (or rather during) the first allocation call. If it
> occurs during the first allocation call then a larval algorithm will
> hold any subsequent allocations until the testing is done.
>
> So the question is who is doing the allocation prior the registration
> of the accelerated algorithms? Is it the fuzzing test?
>

All modules are already loaded, but there's no "adiantum(xchacha12,aes)"
algorithm directly available so it has be instantiated using the template.

Then with CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y, the self-tests for the
accelerated instance will allocate a generic instance to compare it against.
This causes the generic instance to be tested and registered first, so the first
user will get the generic instance, not the accelerated one as expected.

- Eric

2019-04-11 06:26:59

by Herbert Xu

[permalink] [raw]
Subject: Re: [RFC/RFT PATCH 18/18] crypto: run initcalls for generic implementations earlier

On Tue, Apr 09, 2019 at 11:16:09AM -0700, Eric Biggers wrote:
>
> All modules are already loaded, but there's no "adiantum(xchacha12,aes)"
> algorithm directly available so it has be instantiated using the template.
>
> Then with CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y, the self-tests for the
> accelerated instance will allocate a generic instance to compare it against.
> This causes the generic instance to be tested and registered first, so the first
> user will get the generic instance, not the accelerated one as expected.

Thanks for the explanation Eric.

This sounds like a bug in how we mature the larvals where we should
not fulfil an outstanding request against an existing larval if a
higher-priority larval is still waiting for test completion. Let me
see if I can come up with a fix for it.

Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt