From c946e3602165945515acf9d24ffe074143c5d5c7 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Thu, 26 Apr 2018 19:57:28 -0700 Subject: crypto: tcrypt - Remove VLA usage In the quest to remove all stack VLA usage from the kernel[1], this allocates the return code buffers before starting jiffie timers, rather than using stack space for the array. Additionally cleans up some exit paths and make sure that the num_mb module_param() is used only once per execution to avoid possible races in the value changing. [1] https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qPXydAacU1RqZWA@mail.gmail.com Signed-off-by: Kees Cook Signed-off-by: Herbert Xu --- crypto/tcrypt.c | 118 +++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 79 insertions(+), 39 deletions(-) (limited to 'crypto/tcrypt.c') diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c index 51fe7c87..e721faab 100644 --- a/crypto/tcrypt.c +++ b/crypto/tcrypt.c @@ -158,9 +158,9 @@ struct test_mb_aead_data { }; static int do_mult_aead_op(struct test_mb_aead_data *data, int enc, - u32 num_mb) + u32 num_mb, int *rc) { - int i, rc[num_mb], err = 0; + int i, err = 0; /* Fire up a bunch of concurrent requests */ for (i = 0; i < num_mb; i++) { @@ -188,18 +188,26 @@ static int test_mb_aead_jiffies(struct test_mb_aead_data *data, int enc, { unsigned long start, end; int bcount; - int ret; + int ret = 0; + int *rc; + + rc = kcalloc(num_mb, sizeof(*rc), GFP_KERNEL); + if (!rc) + return -ENOMEM; for (start = jiffies, end = start + secs * HZ, bcount = 0; time_before(jiffies, end); bcount++) { - ret = do_mult_aead_op(data, enc, num_mb); + ret = do_mult_aead_op(data, enc, num_mb, rc); if (ret) - return ret; + goto out; } pr_cont("%d operations in %d seconds (%ld bytes)\n", bcount * num_mb, secs, (long)bcount * blen * num_mb); - return 0; + +out: + kfree(rc); + return ret; } static int test_mb_aead_cycles(struct test_mb_aead_data *data, int enc, @@ -208,10 +216,15 @@ static int test_mb_aead_cycles(struct test_mb_aead_data *data, int enc, unsigned long cycles = 0; int ret = 0; int i; + int *rc; + + rc = kcalloc(num_mb, sizeof(*rc), GFP_KERNEL); + if (!rc) + return -ENOMEM; /* Warm-up run. */ for (i = 0; i < 4; i++) { - ret = do_mult_aead_op(data, enc, num_mb); + ret = do_mult_aead_op(data, enc, num_mb, rc); if (ret) goto out; } @@ -221,7 +234,7 @@ static int test_mb_aead_cycles(struct test_mb_aead_data *data, int enc, cycles_t start, end; start = get_cycles(); - ret = do_mult_aead_op(data, enc, num_mb); + ret = do_mult_aead_op(data, enc, num_mb, rc); end = get_cycles(); if (ret) @@ -230,11 +243,11 @@ static int test_mb_aead_cycles(struct test_mb_aead_data *data, int enc, cycles += end - start; } -out: - if (ret == 0) - pr_cont("1 operation in %lu cycles (%d bytes)\n", - (cycles + 4) / (8 * num_mb), blen); + pr_cont("1 operation in %lu cycles (%d bytes)\n", + (cycles + 4) / (8 * num_mb), blen); +out: + kfree(rc); return ret; } @@ -705,9 +718,10 @@ struct test_mb_ahash_data { char *xbuf[XBUFSIZE]; }; -static inline int do_mult_ahash_op(struct test_mb_ahash_data *data, u32 num_mb) +static inline int do_mult_ahash_op(struct test_mb_ahash_data *data, u32 num_mb, + int *rc) { - int i, rc[num_mb], err = 0; + int i, err = 0; /* Fire up a bunch of concurrent requests */ for (i = 0; i < num_mb; i++) @@ -731,18 +745,26 @@ static int test_mb_ahash_jiffies(struct test_mb_ahash_data *data, int blen, { unsigned long start, end; int bcount; - int ret; + int ret = 0; + int *rc; + + rc = kcalloc(num_mb, sizeof(*rc), GFP_KERNEL); + if (!rc) + return -ENOMEM; for (start = jiffies, end = start + secs * HZ, bcount = 0; time_before(jiffies, end); bcount++) { - ret = do_mult_ahash_op(data, num_mb); + ret = do_mult_ahash_op(data, num_mb, rc); if (ret) - return ret; + goto out; } pr_cont("%d operations in %d seconds (%ld bytes)\n", bcount * num_mb, secs, (long)bcount * blen * num_mb); - return 0; + +out: + kfree(rc); + return ret; } static int test_mb_ahash_cycles(struct test_mb_ahash_data *data, int blen, @@ -751,10 +773,15 @@ static int test_mb_ahash_cycles(struct test_mb_ahash_data *data, int blen, unsigned long cycles = 0; int ret = 0; int i; + int *rc; + + rc = kcalloc(num_mb, sizeof(*rc), GFP_KERNEL); + if (!rc) + return -ENOMEM; /* Warm-up run. */ for (i = 0; i < 4; i++) { - ret = do_mult_ahash_op(data, num_mb); + ret = do_mult_ahash_op(data, num_mb, rc); if (ret) goto out; } @@ -764,7 +791,7 @@ static int test_mb_ahash_cycles(struct test_mb_ahash_data *data, int blen, cycles_t start, end; start = get_cycles(); - ret = do_mult_ahash_op(data, num_mb); + ret = do_mult_ahash_op(data, num_mb, rc); end = get_cycles(); if (ret) @@ -773,11 +800,11 @@ static int test_mb_ahash_cycles(struct test_mb_ahash_data *data, int blen, cycles += end - start; } -out: - if (ret == 0) - pr_cont("1 operation in %lu cycles (%d bytes)\n", - (cycles + 4) / (8 * num_mb), blen); + pr_cont("1 operation in %lu cycles (%d bytes)\n", + (cycles + 4) / (8 * num_mb), blen); +out: + kfree(rc); return ret; } @@ -1118,9 +1145,9 @@ struct test_mb_skcipher_data { }; static int do_mult_acipher_op(struct test_mb_skcipher_data *data, int enc, - u32 num_mb) + u32 num_mb, int *rc) { - int i, rc[num_mb], err = 0; + int i, err = 0; /* Fire up a bunch of concurrent requests */ for (i = 0; i < num_mb; i++) { @@ -1148,18 +1175,26 @@ static int test_mb_acipher_jiffies(struct test_mb_skcipher_data *data, int enc, { unsigned long start, end; int bcount; - int ret; + int ret = 0; + int *rc; + + rc = kcalloc(num_mb, sizeof(*rc), GFP_KERNEL); + if (!rc) + return -ENOMEM; for (start = jiffies, end = start + secs * HZ, bcount = 0; time_before(jiffies, end); bcount++) { - ret = do_mult_acipher_op(data, enc, num_mb); + ret = do_mult_acipher_op(data, enc, num_mb, rc); if (ret) - return ret; + goto out; } pr_cont("%d operations in %d seconds (%ld bytes)\n", bcount * num_mb, secs, (long)bcount * blen * num_mb); - return 0; + +out: + kfree(rc); + return ret; } static int test_mb_acipher_cycles(struct test_mb_skcipher_data *data, int enc, @@ -1168,10 +1203,15 @@ static int test_mb_acipher_cycles(struct test_mb_skcipher_data *data, int enc, unsigned long cycles = 0; int ret = 0; int i; + int *rc; + + rc = kcalloc(num_mb, sizeof(*rc), GFP_KERNEL); + if (!rc) + return -ENOMEM; /* Warm-up run. */ for (i = 0; i < 4; i++) { - ret = do_mult_acipher_op(data, enc, num_mb); + ret = do_mult_acipher_op(data, enc, num_mb, rc); if (ret) goto out; } @@ -1181,7 +1221,7 @@ static int test_mb_acipher_cycles(struct test_mb_skcipher_data *data, int enc, cycles_t start, end; start = get_cycles(); - ret = do_mult_acipher_op(data, enc, num_mb); + ret = do_mult_acipher_op(data, enc, num_mb, rc); end = get_cycles(); if (ret) @@ -1190,11 +1230,11 @@ static int test_mb_acipher_cycles(struct test_mb_skcipher_data *data, int enc, cycles += end - start; } -out: - if (ret == 0) - pr_cont("1 operation in %lu cycles (%d bytes)\n", - (cycles + 4) / (8 * num_mb), blen); + pr_cont("1 operation in %lu cycles (%d bytes)\n", + (cycles + 4) / (8 * num_mb), blen); +out: + kfree(rc); return ret; } @@ -1606,7 +1646,7 @@ static inline int tcrypt_test(const char *alg) return ret; } -static int do_test(const char *alg, u32 type, u32 mask, int m) +static int do_test(const char *alg, u32 type, u32 mask, int m, u32 num_mb) { int i; int ret = 0; @@ -1621,7 +1661,7 @@ static int do_test(const char *alg, u32 type, u32 mask, int m) } for (i = 1; i < 200; i++) - ret += do_test(NULL, 0, 0, i); + ret += do_test(NULL, 0, 0, i, num_mb); break; case 1: @@ -2903,7 +2943,7 @@ static int __init tcrypt_mod_init(void) goto err_free_tv; } - err = do_test(alg, type, mask, mode); + err = do_test(alg, type, mask, mode, num_mb); if (err) { printk(KERN_ERR "tcrypt: one or more tests failed!\n"); -- cgit v1.2.3 From dfea953b0f4848a4ed0798090a1332e6f6bd019f Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Sat, 19 May 2018 22:07:40 -0700 Subject: crypto: testmgr - remove bfin_crc "hmac(crc32)" test vectors The Blackfin CRC driver was removed by commit 9678a8dc53c1 ("crypto: bfin_crc - remove blackfin CRC driver"), but it was forgotten to remove the corresponding "hmac(crc32)" test vectors. I see no point in keeping them since nothing else appears to implement or use "hmac(crc32)", which isn't an algorithm that makes sense anyway because HMAC is meant to be used with a cryptographically secure hash function, which CRC's are not. Thus, remove the unneeded test vectors. Signed-off-by: Eric Biggers Signed-off-by: Herbert Xu --- crypto/tcrypt.c | 4 --- crypto/testmgr.c | 6 ---- crypto/testmgr.h | 88 -------------------------------------------------------- 3 files changed, 98 deletions(-) (limited to 'crypto/tcrypt.c') diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c index e721faab..d5bcdd90 100644 --- a/crypto/tcrypt.c +++ b/crypto/tcrypt.c @@ -1942,10 +1942,6 @@ static int do_test(const char *alg, u32 type, u32 mask, int m, u32 num_mb) ret += tcrypt_test("vmac(aes)"); break; - case 110: - ret += tcrypt_test("hmac(crc32)"); - break; - case 111: ret += tcrypt_test("hmac(sha3-224)"); break; diff --git a/crypto/testmgr.c b/crypto/testmgr.c index 41a5f42d..7e57530e 100644 --- a/crypto/testmgr.c +++ b/crypto/testmgr.c @@ -3168,12 +3168,6 @@ static const struct alg_test_desc alg_test_descs[] = { .suite = { .hash = __VECS(ghash_tv_template) } - }, { - .alg = "hmac(crc32)", - .test = alg_test_hash, - .suite = { - .hash = __VECS(bfin_crc_tv_template) - } }, { .alg = "hmac(md5)", .test = alg_test_hash, diff --git a/crypto/testmgr.h b/crypto/testmgr.h index 552d8f00..816e3eb1 100644 --- a/crypto/testmgr.h +++ b/crypto/testmgr.h @@ -43156,94 +43156,6 @@ static const struct hash_testvec crc32c_tv_template[] = { } }; -/* - * Blakcifn CRC test vectors - */ -static const struct hash_testvec bfin_crc_tv_template[] = { - { - .psize = 0, - .digest = "\x00\x00\x00\x00", - }, - { - .key = "\x87\xa9\xcb\xed", - .ksize = 4, - .psize = 0, - .digest = "\x87\xa9\xcb\xed", - }, - { - .key = "\xff\xff\xff\xff", - .ksize = 4, - .plaintext = "\x01\x02\x03\x04\x05\x06\x07\x08" - "\x09\x0a\x0b\x0c\x0d\x0e\x0f\x10" - "\x11\x12\x13\x14\x15\x16\x17\x18" - "\x19\x1a\x1b\x1c\x1d\x1e\x1f\x20" - "\x21\x22\x23\x24\x25\x26\x27\x28", - .psize = 40, - .digest = "\x84\x0c\x8d\xa2", - }, - { - .key = "\xff\xff\xff\xff", - .ksize = 4, - .plaintext = "\x01\x02\x03\x04\x05\x06\x07\x08" - "\x09\x0a\x0b\x0c\x0d\x0e\x0f\x10" - "\x11\x12\x13\x14\x15\x16\x17\x18" - "\x19\x1a\x1b\x1c\x1d\x1e\x1f\x20" - "\x21\x22\x23\x24\x25\x26", - .psize = 38, - .digest = "\x8c\x58\xec\xb7", - }, - { - .key = "\xff\xff\xff\xff", - .ksize = 4, - .plaintext = "\x01\x02\x03\x04\x05\x06\x07\x08" - "\x09\x0a\x0b\x0c\x0d\x0e\x0f\x10" - "\x11\x12\x13\x14\x15\x16\x17\x18" - "\x19\x1a\x1b\x1c\x1d\x1e\x1f\x20" - "\x21\x22\x23\x24\x25\x26\x27", - .psize = 39, - .digest = "\xdc\x50\x28\x7b", - }, - { - .key = "\xff\xff\xff\xff", - .ksize = 4, - .plaintext = "\x01\x02\x03\x04\x05\x06\x07\x08" - "\x09\x0a\x0b\x0c\x0d\x0e\x0f\x10" - "\x11\x12\x13\x14\x15\x16\x17\x18" - "\x19\x1a\x1b\x1c\x1d\x1e\x1f\x20" - "\x21\x22\x23\x24\x25\x26\x27\x28" - "\x29\x2a\x2b\x2c\x2d\x2e\x2f\x30" - "\x31\x32\x33\x34\x35\x36\x37\x38" - "\x39\x3a\x3b\x3c\x3d\x3e\x3f\x40" - "\x41\x42\x43\x44\x45\x46\x47\x48" - "\x49\x4a\x4b\x4c\x4d\x4e\x4f\x50" - "\x51\x52\x53\x54\x55\x56\x57\x58" - "\x59\x5a\x5b\x5c\x5d\x5e\x5f\x60" - "\x61\x62\x63\x64\x65\x66\x67\x68" - "\x69\x6a\x6b\x6c\x6d\x6e\x6f\x70" - "\x71\x72\x73\x74\x75\x76\x77\x78" - "\x79\x7a\x7b\x7c\x7d\x7e\x7f\x80" - "\x81\x82\x83\x84\x85\x86\x87\x88" - "\x89\x8a\x8b\x8c\x8d\x8e\x8f\x90" - "\x91\x92\x93\x94\x95\x96\x97\x98" - "\x99\x9a\x9b\x9c\x9d\x9e\x9f\xa0" - "\xa1\xa2\xa3\xa4\xa5\xa6\xa7\xa8" - "\xa9\xaa\xab\xac\xad\xae\xaf\xb0" - "\xb1\xb2\xb3\xb4\xb5\xb6\xb7\xb8" - "\xb9\xba\xbb\xbc\xbd\xbe\xbf\xc0" - "\xc1\xc2\xc3\xc4\xc5\xc6\xc7\xc8" - "\xc9\xca\xcb\xcc\xcd\xce\xcf\xd0" - "\xd1\xd2\xd3\xd4\xd5\xd6\xd7\xd8" - "\xd9\xda\xdb\xdc\xdd\xde\xdf\xe0" - "\xe1\xe2\xe3\xe4\xe5\xe6\xe7\xe8" - "\xe9\xea\xeb\xec\xed\xee\xef\xf0", - .psize = 240, - .digest = "\x10\x19\x4a\x5c", - .np = 2, - .tap = { 31, 209 } - }, - -}; - static const struct comp_testvec lz4_comp_tv_template[] = { { .inlen = 255, -- cgit v1.2.3