From 8994fee63119f4ee189c0a4c96ac25db9089ef52 Mon Sep 17 00:00:00 2001 From: Tianjia Zhang Date: Thu, 15 Oct 2020 17:24:41 +0800 Subject: crypto: sm2 - remove unnecessary reset operations This is an algorithm optimization. The reset operation when setting the public key is repeated and redundant, so remove it. At the same time, `sm2_ecc_os2ec()` is optimized to make the function more simpler and more in line with the Linux code style. Signed-off-by: Tianjia Zhang Signed-off-by: Herbert Xu --- crypto/sm2.c | 75 +++++++++++++++++++++++------------------------------------- 1 file changed, 29 insertions(+), 46 deletions(-) diff --git a/crypto/sm2.c b/crypto/sm2.c index 767e1603..b21addc3 100644 --- a/crypto/sm2.c +++ b/crypto/sm2.c @@ -119,12 +119,6 @@ static void sm2_ec_ctx_deinit(struct mpi_ec_ctx *ec) memset(ec, 0, sizeof(*ec)); } -static int sm2_ec_ctx_reset(struct mpi_ec_ctx *ec) -{ - sm2_ec_ctx_deinit(ec); - return sm2_ec_ctx_init(ec); -} - /* RESULT must have been initialized and is set on success to the * point given by VALUE. */ @@ -132,55 +126,48 @@ static int sm2_ecc_os2ec(MPI_POINT result, MPI value) { int rc; size_t n; - const unsigned char *buf; - unsigned char *buf_memory; + unsigned char *buf; MPI x, y; - n = (mpi_get_nbits(value)+7)/8; - buf_memory = kmalloc(n, GFP_KERNEL); - rc = mpi_print(GCRYMPI_FMT_USG, buf_memory, n, &n, value); - if (rc) { - kfree(buf_memory); - return rc; - } - buf = buf_memory; + n = MPI_NBYTES(value); + buf = kmalloc(n, GFP_KERNEL); + if (!buf) + return -ENOMEM; - if (n < 1) { - kfree(buf_memory); - return -EINVAL; - } - if (*buf != 4) { - kfree(buf_memory); - return -EINVAL; /* No support for point compression. */ - } - if (((n-1)%2)) { - kfree(buf_memory); - return -EINVAL; - } - n = (n-1)/2; + rc = mpi_print(GCRYMPI_FMT_USG, buf, n, &n, value); + if (rc) + goto err_freebuf; + + rc = -EINVAL; + if (n < 1 || ((n - 1) % 2)) + goto err_freebuf; + /* No support for point compression */ + if (*buf != 0x4) + goto err_freebuf; + + rc = -ENOMEM; + n = (n - 1) / 2; x = mpi_read_raw_data(buf + 1, n); - if (!x) { - kfree(buf_memory); - return -ENOMEM; - } + if (!x) + goto err_freebuf; y = mpi_read_raw_data(buf + 1 + n, n); - kfree(buf_memory); - if (!y) { - mpi_free(x); - return -ENOMEM; - } + if (!y) + goto err_freex; mpi_normalize(x); mpi_normalize(y); - mpi_set(result->x, x); mpi_set(result->y, y); mpi_set_ui(result->z, 1); - mpi_free(x); - mpi_free(y); + rc = 0; - return 0; + mpi_free(y); +err_freex: + mpi_free(x); +err_freebuf: + kfree(buf); + return rc; } struct sm2_signature_ctx { @@ -399,10 +386,6 @@ static int sm2_set_pub_key(struct crypto_akcipher *tfm, MPI a; int rc; - rc = sm2_ec_ctx_reset(ec); - if (rc) - return rc; - ec->Q = mpi_point_new(0); if (!ec->Q) return -ENOMEM; -- cgit v1.2.3 From c0ea3617458f0f49199718e2869bcac1bb890364 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Mon, 26 Oct 2020 09:17:00 -0700 Subject: crypto: testmgr - always print the actual hash driver name When alg_test() is called from tcrypt.ko rather than from the algorithm registration code, "driver" is actually the algorithm name, not the driver name. So it shouldn't be used in places where a driver name is wanted, e.g. when reporting a test failure or when checking whether the driver is the generic driver or not. Fix this for the hash algorithm tests by getting the driver name from the crypto_ahash or crypto_shash that actually got allocated. Signed-off-by: Eric Biggers Signed-off-by: Herbert Xu --- crypto/testmgr.c | 43 ++++++++++++++++++++----------------------- 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/crypto/testmgr.c b/crypto/testmgr.c index a64a639e..ec64b70a 100644 --- a/crypto/testmgr.c +++ b/crypto/testmgr.c @@ -1171,8 +1171,7 @@ static inline const void *sg_data(struct scatterlist *sg) } /* Test one hash test vector in one configuration, using the shash API */ -static int test_shash_vec_cfg(const char *driver, - const struct hash_testvec *vec, +static int test_shash_vec_cfg(const struct hash_testvec *vec, const char *vec_name, const struct testvec_config *cfg, struct shash_desc *desc, @@ -1183,6 +1182,7 @@ static int test_shash_vec_cfg(const char *driver, const unsigned int alignmask = crypto_shash_alignmask(tfm); const unsigned int digestsize = crypto_shash_digestsize(tfm); const unsigned int statesize = crypto_shash_statesize(tfm); + const char *driver = crypto_shash_driver_name(tfm); const struct test_sg_division *divs[XBUFSIZE]; unsigned int i; u8 result[HASH_MAX_DIGESTSIZE + TESTMGR_POISON_LEN]; @@ -1355,8 +1355,7 @@ static int check_nonfinal_ahash_op(const char *op, int err, } /* Test one hash test vector in one configuration, using the ahash API */ -static int test_ahash_vec_cfg(const char *driver, - const struct hash_testvec *vec, +static int test_ahash_vec_cfg(const struct hash_testvec *vec, const char *vec_name, const struct testvec_config *cfg, struct ahash_request *req, @@ -1367,6 +1366,7 @@ static int test_ahash_vec_cfg(const char *driver, const unsigned int alignmask = crypto_ahash_alignmask(tfm); const unsigned int digestsize = crypto_ahash_digestsize(tfm); const unsigned int statesize = crypto_ahash_statesize(tfm); + const char *driver = crypto_ahash_driver_name(tfm); const u32 req_flags = CRYPTO_TFM_REQ_MAY_BACKLOG | cfg->req_flags; const struct test_sg_division *divs[XBUFSIZE]; DECLARE_CRYPTO_WAIT(wait); @@ -1521,8 +1521,7 @@ result_ready: driver, cfg); } -static int test_hash_vec_cfg(const char *driver, - const struct hash_testvec *vec, +static int test_hash_vec_cfg(const struct hash_testvec *vec, const char *vec_name, const struct testvec_config *cfg, struct ahash_request *req, @@ -1539,20 +1538,18 @@ static int test_hash_vec_cfg(const char *driver, */ if (desc) { - err = test_shash_vec_cfg(driver, vec, vec_name, cfg, desc, tsgl, + err = test_shash_vec_cfg(vec, vec_name, cfg, desc, tsgl, hashstate); if (err) return err; } - return test_ahash_vec_cfg(driver, vec, vec_name, cfg, req, tsgl, - hashstate); + return test_ahash_vec_cfg(vec, vec_name, cfg, req, tsgl, hashstate); } -static int test_hash_vec(const char *driver, const struct hash_testvec *vec, - unsigned int vec_num, struct ahash_request *req, - struct shash_desc *desc, struct test_sglist *tsgl, - u8 *hashstate) +static int test_hash_vec(const struct hash_testvec *vec, unsigned int vec_num, + struct ahash_request *req, struct shash_desc *desc, + struct test_sglist *tsgl, u8 *hashstate) { char vec_name[16]; unsigned int i; @@ -1561,7 +1558,7 @@ static int test_hash_vec(const char *driver, const struct hash_testvec *vec, 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_name, + err = test_hash_vec_cfg(vec, vec_name, &default_hash_testvec_configs[i], req, desc, tsgl, hashstate); if (err) @@ -1576,7 +1573,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_name, &cfg, + err = test_hash_vec_cfg(vec, vec_name, &cfg, req, desc, tsgl, hashstate); if (err) return err; @@ -1633,8 +1630,7 @@ done: * 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, +static int test_hash_vs_generic_impl(const char *generic_driver, unsigned int maxkeysize, struct ahash_request *req, struct shash_desc *desc, @@ -1646,6 +1642,7 @@ static int test_hash_vs_generic_impl(const char *driver, const unsigned int blocksize = crypto_ahash_blocksize(tfm); const unsigned int maxdatasize = (2 * PAGE_SIZE) - TESTMGR_POISON_LEN; const char *algname = crypto_hash_alg_common(tfm)->base.cra_name; + const char *driver = crypto_ahash_driver_name(tfm); char _generic_driver[CRYPTO_MAX_ALG_NAME]; struct crypto_shash *generic_tfm = NULL; struct shash_desc *generic_desc = NULL; @@ -1732,7 +1729,7 @@ static int test_hash_vs_generic_impl(const char *driver, vec_name, sizeof(vec_name)); generate_random_testvec_config(cfg, cfgname, sizeof(cfgname)); - err = test_hash_vec_cfg(driver, &vec, vec_name, cfg, + err = test_hash_vec_cfg(&vec, vec_name, cfg, req, desc, tsgl, hashstate); if (err) goto out; @@ -1749,8 +1746,7 @@ out: return err; } #else /* !CONFIG_CRYPTO_MANAGER_EXTRA_TESTS */ -static int test_hash_vs_generic_impl(const char *driver, - const char *generic_driver, +static int test_hash_vs_generic_impl(const char *generic_driver, unsigned int maxkeysize, struct ahash_request *req, struct shash_desc *desc, @@ -1820,6 +1816,7 @@ static int __alg_test_hash(const struct hash_testvec *vecs, driver, PTR_ERR(atfm)); return PTR_ERR(atfm); } + driver = crypto_ahash_driver_name(atfm); req = ahash_request_alloc(atfm, GFP_KERNEL); if (!req) { @@ -1859,13 +1856,12 @@ static int __alg_test_hash(const struct hash_testvec *vecs, } for (i = 0; i < num_vecs; i++) { - err = test_hash_vec(driver, &vecs[i], i, req, desc, tsgl, - hashstate); + err = test_hash_vec(&vecs[i], i, req, desc, tsgl, hashstate); if (err) goto out; cond_resched(); } - err = test_hash_vs_generic_impl(driver, generic_driver, maxkeysize, req, + err = test_hash_vs_generic_impl(generic_driver, maxkeysize, req, desc, tsgl, hashstate); out: kfree(hashstate); @@ -3602,6 +3598,7 @@ static int alg_test_crc32c(const struct alg_test_desc *desc, "%ld\n", driver, PTR_ERR(tfm)); return PTR_ERR(tfm); } + driver = crypto_shash_driver_name(tfm); do { SHASH_DESC_ON_STACK(shash, tfm); -- cgit v1.2.3 From 5a84030a62eaeeb7ae5b18b2bba3a2b1c9ec38c1 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Mon, 26 Oct 2020 09:17:01 -0700 Subject: crypto: testmgr - always print the actual AEAD driver name When alg_test() is called from tcrypt.ko rather than from the algorithm registration code, "driver" is actually the algorithm name, not the driver name. So it shouldn't be used in places where a driver name is wanted, e.g. when reporting a test failure or when checking whether the driver is the generic driver or not. Fix this for the AEAD algorithm tests by getting the driver name from the crypto_aead that actually got allocated. Signed-off-by: Eric Biggers Signed-off-by: Herbert Xu --- crypto/testmgr.c | 42 ++++++++++++++++++------------------------ 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/crypto/testmgr.c b/crypto/testmgr.c index ec64b70a..1b785b2f 100644 --- a/crypto/testmgr.c +++ b/crypto/testmgr.c @@ -1919,8 +1919,7 @@ static int alg_test_hash(const struct alg_test_desc *desc, const char *driver, return err; } -static int test_aead_vec_cfg(const char *driver, int enc, - const struct aead_testvec *vec, +static int test_aead_vec_cfg(int enc, const struct aead_testvec *vec, const char *vec_name, const struct testvec_config *cfg, struct aead_request *req, @@ -1930,6 +1929,7 @@ static int test_aead_vec_cfg(const char *driver, int enc, const unsigned int alignmask = crypto_aead_alignmask(tfm); const unsigned int ivsize = crypto_aead_ivsize(tfm); const unsigned int authsize = vec->clen - vec->plen; + const char *driver = crypto_aead_driver_name(tfm); const u32 req_flags = CRYPTO_TFM_REQ_MAY_BACKLOG | cfg->req_flags; const char *op = enc ? "encryption" : "decryption"; DECLARE_CRYPTO_WAIT(wait); @@ -2102,9 +2102,8 @@ static int test_aead_vec_cfg(const char *driver, int enc, return 0; } -static int test_aead_vec(const char *driver, int enc, - const struct aead_testvec *vec, unsigned int vec_num, - struct aead_request *req, +static int test_aead_vec(int enc, const struct aead_testvec *vec, + unsigned int vec_num, struct aead_request *req, struct cipher_test_sglists *tsgls) { char vec_name[16]; @@ -2117,7 +2116,7 @@ static int test_aead_vec(const char *driver, int enc, 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_name, + err = test_aead_vec_cfg(enc, vec, vec_name, &default_cipher_testvec_configs[i], req, tsgls); if (err) @@ -2132,7 +2131,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_name, + err = test_aead_vec_cfg(enc, vec, vec_name, &cfg, req, tsgls); if (err) return err; @@ -2148,7 +2147,6 @@ static int test_aead_vec(const char *driver, int enc, struct aead_extra_tests_ctx { struct aead_request *req; struct crypto_aead *tfm; - const char *driver; const struct alg_test_desc *test_desc; struct cipher_test_sglists *tsgls; unsigned int maxdatasize; @@ -2354,7 +2352,7 @@ static int test_aead_inauthentic_inputs(struct aead_extra_tests_ctx *ctx) if (ctx->vec.novrfy) { generate_random_testvec_config(&ctx->cfg, ctx->cfgname, sizeof(ctx->cfgname)); - err = test_aead_vec_cfg(ctx->driver, DECRYPT, &ctx->vec, + err = test_aead_vec_cfg(DECRYPT, &ctx->vec, ctx->vec_name, &ctx->cfg, ctx->req, ctx->tsgls); if (err) @@ -2373,7 +2371,7 @@ static int test_aead_vs_generic_impl(struct aead_extra_tests_ctx *ctx) { struct crypto_aead *tfm = ctx->tfm; const char *algname = crypto_aead_alg(tfm)->base.cra_name; - const char *driver = ctx->driver; + const char *driver = crypto_aead_driver_name(tfm); const char *generic_driver = ctx->test_desc->generic_driver; char _generic_driver[CRYPTO_MAX_ALG_NAME]; struct crypto_aead *generic_tfm = NULL; @@ -2450,14 +2448,14 @@ static int test_aead_vs_generic_impl(struct aead_extra_tests_ctx *ctx) generate_random_testvec_config(&ctx->cfg, ctx->cfgname, sizeof(ctx->cfgname)); if (!ctx->vec.novrfy) { - err = test_aead_vec_cfg(driver, ENCRYPT, &ctx->vec, + err = test_aead_vec_cfg(ENCRYPT, &ctx->vec, ctx->vec_name, &ctx->cfg, ctx->req, ctx->tsgls); if (err) goto out; } if (ctx->vec.crypt_error == 0 || ctx->vec.novrfy) { - err = test_aead_vec_cfg(driver, DECRYPT, &ctx->vec, + err = test_aead_vec_cfg(DECRYPT, &ctx->vec, ctx->vec_name, &ctx->cfg, ctx->req, ctx->tsgls); if (err) @@ -2472,8 +2470,7 @@ out: return err; } -static int test_aead_extra(const char *driver, - const struct alg_test_desc *test_desc, +static int test_aead_extra(const struct alg_test_desc *test_desc, struct aead_request *req, struct cipher_test_sglists *tsgls) { @@ -2489,7 +2486,6 @@ static int test_aead_extra(const char *driver, return -ENOMEM; ctx->req = req; ctx->tfm = crypto_aead_reqtfm(req); - ctx->driver = driver; ctx->test_desc = test_desc; ctx->tsgls = tsgls; ctx->maxdatasize = (2 * PAGE_SIZE) - TESTMGR_POISON_LEN; @@ -2524,8 +2520,7 @@ out: return err; } #else /* !CONFIG_CRYPTO_MANAGER_EXTRA_TESTS */ -static int test_aead_extra(const char *driver, - const struct alg_test_desc *test_desc, +static int test_aead_extra(const struct alg_test_desc *test_desc, struct aead_request *req, struct cipher_test_sglists *tsgls) { @@ -2533,8 +2528,7 @@ static int test_aead_extra(const char *driver, } #endif /* !CONFIG_CRYPTO_MANAGER_EXTRA_TESTS */ -static int test_aead(const char *driver, int enc, - const struct aead_test_suite *suite, +static int test_aead(int enc, const struct aead_test_suite *suite, struct aead_request *req, struct cipher_test_sglists *tsgls) { @@ -2542,8 +2536,7 @@ static int test_aead(const char *driver, int enc, int err; for (i = 0; i < suite->count; i++) { - err = test_aead_vec(driver, enc, &suite->vecs[i], i, req, - tsgls); + err = test_aead_vec(enc, &suite->vecs[i], i, req, tsgls); if (err) return err; cond_resched(); @@ -2571,6 +2564,7 @@ static int alg_test_aead(const struct alg_test_desc *desc, const char *driver, driver, PTR_ERR(tfm)); return PTR_ERR(tfm); } + driver = crypto_aead_driver_name(tfm); req = aead_request_alloc(tfm, GFP_KERNEL); if (!req) { @@ -2588,15 +2582,15 @@ static int alg_test_aead(const struct alg_test_desc *desc, const char *driver, goto out; } - err = test_aead(driver, ENCRYPT, suite, req, tsgls); + err = test_aead(ENCRYPT, suite, req, tsgls); if (err) goto out; - err = test_aead(driver, DECRYPT, suite, req, tsgls); + err = test_aead(DECRYPT, suite, req, tsgls); if (err) goto out; - err = test_aead_extra(driver, desc, req, tsgls); + err = test_aead_extra(desc, req, tsgls); out: free_cipher_test_sglists(tsgls); aead_request_free(req); -- cgit v1.2.3 From f164a97cbca10866edc212d4f60f9e2bd51d1cf2 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Mon, 26 Oct 2020 09:17:02 -0700 Subject: crypto: testmgr - always print the actual skcipher driver name When alg_test() is called from tcrypt.ko rather than from the algorithm registration code, "driver" is actually the algorithm name, not the driver name. So it shouldn't be used in places where a driver name is wanted, e.g. when reporting a test failure or when checking whether the driver is the generic driver or not. Fix this for the skcipher algorithm tests by getting the driver name from the crypto_skcipher that actually got allocated. Signed-off-by: Eric Biggers Signed-off-by: Herbert Xu --- crypto/testmgr.c | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/crypto/testmgr.c b/crypto/testmgr.c index 1b785b2f..dcc1fa41 100644 --- a/crypto/testmgr.c +++ b/crypto/testmgr.c @@ -2685,8 +2685,7 @@ out_nobuf: return ret; } -static int test_skcipher_vec_cfg(const char *driver, int enc, - const struct cipher_testvec *vec, +static int test_skcipher_vec_cfg(int enc, const struct cipher_testvec *vec, const char *vec_name, const struct testvec_config *cfg, struct skcipher_request *req, @@ -2695,6 +2694,7 @@ static int test_skcipher_vec_cfg(const char *driver, int enc, struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req); const unsigned int alignmask = crypto_skcipher_alignmask(tfm); const unsigned int ivsize = crypto_skcipher_ivsize(tfm); + const char *driver = crypto_skcipher_driver_name(tfm); const u32 req_flags = CRYPTO_TFM_REQ_MAY_BACKLOG | cfg->req_flags; const char *op = enc ? "encryption" : "decryption"; DECLARE_CRYPTO_WAIT(wait); @@ -2849,8 +2849,7 @@ static int test_skcipher_vec_cfg(const char *driver, int enc, return 0; } -static int test_skcipher_vec(const char *driver, int enc, - const struct cipher_testvec *vec, +static int test_skcipher_vec(int enc, const struct cipher_testvec *vec, unsigned int vec_num, struct skcipher_request *req, struct cipher_test_sglists *tsgls) @@ -2865,7 +2864,7 @@ static int test_skcipher_vec(const char *driver, int enc, 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_name, + err = test_skcipher_vec_cfg(enc, vec, vec_name, &default_cipher_testvec_configs[i], req, tsgls); if (err) @@ -2880,7 +2879,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_name, + err = test_skcipher_vec_cfg(enc, vec, vec_name, &cfg, req, tsgls); if (err) return err; @@ -2951,8 +2950,7 @@ done: * 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, +static int test_skcipher_vs_generic_impl(const char *generic_driver, struct skcipher_request *req, struct cipher_test_sglists *tsgls) { @@ -2962,6 +2960,7 @@ static int test_skcipher_vs_generic_impl(const char *driver, const unsigned int blocksize = crypto_skcipher_blocksize(tfm); const unsigned int maxdatasize = (2 * PAGE_SIZE) - TESTMGR_POISON_LEN; const char *algname = crypto_skcipher_alg(tfm)->base.cra_name; + const char *driver = crypto_skcipher_driver_name(tfm); char _generic_driver[CRYPTO_MAX_ALG_NAME]; struct crypto_skcipher *generic_tfm = NULL; struct skcipher_request *generic_req = NULL; @@ -3067,11 +3066,11 @@ static int test_skcipher_vs_generic_impl(const char *driver, vec_name, sizeof(vec_name)); generate_random_testvec_config(cfg, cfgname, sizeof(cfgname)); - err = test_skcipher_vec_cfg(driver, ENCRYPT, &vec, vec_name, + err = test_skcipher_vec_cfg(ENCRYPT, &vec, vec_name, cfg, req, tsgls); if (err) goto out; - err = test_skcipher_vec_cfg(driver, DECRYPT, &vec, vec_name, + err = test_skcipher_vec_cfg(DECRYPT, &vec, vec_name, cfg, req, tsgls); if (err) goto out; @@ -3089,8 +3088,7 @@ out: return err; } #else /* !CONFIG_CRYPTO_MANAGER_EXTRA_TESTS */ -static int test_skcipher_vs_generic_impl(const char *driver, - const char *generic_driver, +static int test_skcipher_vs_generic_impl(const char *generic_driver, struct skcipher_request *req, struct cipher_test_sglists *tsgls) { @@ -3098,8 +3096,7 @@ static int test_skcipher_vs_generic_impl(const char *driver, } #endif /* !CONFIG_CRYPTO_MANAGER_EXTRA_TESTS */ -static int test_skcipher(const char *driver, int enc, - const struct cipher_test_suite *suite, +static int test_skcipher(int enc, const struct cipher_test_suite *suite, struct skcipher_request *req, struct cipher_test_sglists *tsgls) { @@ -3107,8 +3104,7 @@ static int test_skcipher(const char *driver, int enc, int err; for (i = 0; i < suite->count; i++) { - err = test_skcipher_vec(driver, enc, &suite->vecs[i], i, req, - tsgls); + err = test_skcipher_vec(enc, &suite->vecs[i], i, req, tsgls); if (err) return err; cond_resched(); @@ -3136,6 +3132,7 @@ static int alg_test_skcipher(const struct alg_test_desc *desc, driver, PTR_ERR(tfm)); return PTR_ERR(tfm); } + driver = crypto_skcipher_driver_name(tfm); req = skcipher_request_alloc(tfm, GFP_KERNEL); if (!req) { @@ -3153,16 +3150,15 @@ static int alg_test_skcipher(const struct alg_test_desc *desc, goto out; } - err = test_skcipher(driver, ENCRYPT, suite, req, tsgls); + err = test_skcipher(ENCRYPT, suite, req, tsgls); if (err) goto out; - err = test_skcipher(driver, DECRYPT, suite, req, tsgls); + err = test_skcipher(DECRYPT, suite, req, tsgls); if (err) goto out; - err = test_skcipher_vs_generic_impl(driver, desc->generic_driver, req, - tsgls); + err = test_skcipher_vs_generic_impl(desc->generic_driver, req, tsgls); out: free_cipher_test_sglists(tsgls); skcipher_request_free(req); -- cgit v1.2.3 From 83793cc0613d70185e68b414c97507ce8790a0be Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Mon, 26 Oct 2020 09:31:12 -0700 Subject: crypto: testmgr - WARN on test failure Currently, by default crypto self-test failures only result in a pr_warn() message and an "unknown" status in /proc/crypto. Both of these are easy to miss. There is also an option to panic the kernel when a test fails, but that can't be the default behavior. A crypto self-test failure always indicates a kernel bug, however, and there's already a standard way to report (recoverable) kernel bugs -- the WARN() family of macros. WARNs are noisier and harder to miss, and existing test systems already know to look for them in dmesg or via /proc/sys/kernel/tainted. Therefore, call WARN() when an algorithm fails its self-tests. Signed-off-by: Eric Biggers Signed-off-by: Herbert Xu --- crypto/testmgr.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/crypto/testmgr.c b/crypto/testmgr.c index dcc1fa41..321e38ee 100644 --- a/crypto/testmgr.c +++ b/crypto/testmgr.c @@ -5664,15 +5664,21 @@ int alg_test(const char *driver, const char *alg, u32 type, u32 mask) type, mask); test_done: - if (rc && (fips_enabled || panic_on_fail)) { - fips_fail_notify(); - panic("alg: self-tests for %s (%s) failed in %s mode!\n", - driver, alg, fips_enabled ? "fips" : "panic_on_fail"); + if (rc) { + if (fips_enabled || panic_on_fail) { + fips_fail_notify(); + panic("alg: self-tests for %s (%s) failed in %s mode!\n", + driver, alg, + fips_enabled ? "fips" : "panic_on_fail"); + } + WARN(1, "alg: self-tests for %s (%s) failed (rc=%d)", + driver, alg, rc); + } else { + if (fips_enabled) + pr_info("alg: self-tests for %s (%s) passed\n", + driver, alg); } - if (fips_enabled && !rc) - pr_info("alg: self-tests for %s (%s) passed\n", driver, alg); - return rc; notest: -- cgit v1.2.3 From e1cbfc28701813b40fe3891526dfc55f475bae8d Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Mon, 26 Oct 2020 13:07:15 -0700 Subject: crypto: af_alg - avoid undefined behavior accessing salg_name Commit 7175dac1d936 ("crypto: af_alg - Allow arbitrarily long algorithm names") made the kernel start accepting arbitrarily long algorithm names in sockaddr_alg. However, the actual length of the salg_name field stayed at the original 64 bytes. This is broken because the kernel can access indices >= 64 in salg_name, which is undefined behavior -- even though the memory that is accessed is still located within the sockaddr structure. It would only be defined behavior if the array were properly marked as arbitrary-length (either by making it a flexible array, which is the recommended way these days, or by making it an array of length 0 or 1). We can't simply change salg_name into a flexible array, since that would break source compatibility with userspace programs that embed sockaddr_alg into another struct, or (more commonly) declare a sockaddr_alg like 'struct sockaddr_alg sa = { .salg_name = "foo" };'. One solution would be to change salg_name into a flexible array only when '#ifdef __KERNEL__'. However, that would keep userspace without an easy way to actually use the longer algorithm names. Instead, add a new structure 'sockaddr_alg_new' that has the flexible array field, and expose it to both userspace and the kernel. Make the kernel use it correctly in alg_bind(). This addresses the syzbot report "UBSAN: array-index-out-of-bounds in alg_bind" (https://syzkaller.appspot.com/bug?extid=92ead4eb8e26a26d465e). Reported-by: syzbot+92ead4eb8e26a26d465e@syzkaller.appspotmail.com Fixes: 7175dac1d936 ("crypto: af_alg - Allow arbitrarily long algorithm names") Cc: # v4.12+ Signed-off-by: Eric Biggers Signed-off-by: Herbert Xu --- crypto/af_alg.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/crypto/af_alg.c b/crypto/af_alg.c index d11db80d..9acb9d2c 100644 --- a/crypto/af_alg.c +++ b/crypto/af_alg.c @@ -147,7 +147,7 @@ static int alg_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len) const u32 allowed = CRYPTO_ALG_KERN_DRIVER_ONLY; struct sock *sk = sock->sk; struct alg_sock *ask = alg_sk(sk); - struct sockaddr_alg *sa = (void *)uaddr; + struct sockaddr_alg_new *sa = (void *)uaddr; const struct af_alg_type *type; void *private; int err; @@ -155,7 +155,11 @@ static int alg_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len) if (sock->state == SS_CONNECTED) return -EINVAL; - if (addr_len < sizeof(*sa)) + BUILD_BUG_ON(offsetof(struct sockaddr_alg_new, salg_name) != + offsetof(struct sockaddr_alg, salg_name)); + BUILD_BUG_ON(offsetof(struct sockaddr_alg, salg_name) != sizeof(*sa)); + + if (addr_len < sizeof(*sa) + 1) return -EINVAL; /* If caller uses non-allowed flag, return error. */ @@ -163,7 +167,7 @@ static int alg_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len) return -EINVAL; sa->salg_type[sizeof(sa->salg_type) - 1] = 0; - sa->salg_name[sizeof(sa->salg_name) + addr_len - sizeof(*sa) - 1] = 0; + sa->salg_name[addr_len - sizeof(*sa) - 1] = 0; type = alg_get_type(sa->salg_type); if (PTR_ERR(type) == -ENOENT) { -- cgit v1.2.3 From 6a34f3afc26eca1ed55b984d4ac359ec7aa74236 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Mon, 2 Nov 2020 14:48:15 +0100 Subject: crypto: Kconfig - CRYPTO_MANAGER_EXTRA_TESTS requires the manager The extra tests in the manager actually require the manager to be selected too. Otherwise the linker gives errors like: ld: arch/x86/crypto/chacha_glue.o: in function `chacha_simd_stream_xor': chacha_glue.c:(.text+0x422): undefined reference to `crypto_simd_disabled_for_test' Fixes: da1db20dc767 ("crypto: Kconfig - allow tests to be disabled when manager is disabled") Signed-off-by: Jason A. Donenfeld Signed-off-by: Herbert Xu --- crypto/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crypto/Kconfig b/crypto/Kconfig index 094ef56a..37de7d00 100644 --- a/crypto/Kconfig +++ b/crypto/Kconfig @@ -145,7 +145,7 @@ config CRYPTO_MANAGER_DISABLE_TESTS config CRYPTO_MANAGER_EXTRA_TESTS bool "Enable extra run-time crypto self tests" - depends on DEBUG_KERNEL && !CRYPTO_MANAGER_DISABLE_TESTS + depends on DEBUG_KERNEL && !CRYPTO_MANAGER_DISABLE_TESTS && CRYPTO_MANAGER help Enable extra run-time self tests of registered crypto algorithms, including randomized fuzz tests. -- cgit v1.2.3 From d505b0a6aacaeeb60fc6aff50ed153c10d5983fb Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Thu, 12 Nov 2020 21:20:21 -0800 Subject: crypto: sha - split sha.h into sha1.h and sha2.h Currently contains declarations for both SHA-1 and SHA-2, and contains declarations for SHA-3. This organization is inconsistent, but more importantly SHA-1 is no longer considered to be cryptographically secure. So to the extent possible, SHA-1 shouldn't be grouped together with any of the other SHA versions, and usage of it should be phased out. Therefore, split into two headers and , and make everyone explicitly specify whether they want the declarations for SHA-1, SHA-2, or both. This avoids making the SHA-1 declarations visible to files that don't want anything to do with SHA-1. It also prepares for potentially moving sha1.h into a new insecure/ or dangerous/ directory. Signed-off-by: Eric Biggers Acked-by: Ard Biesheuvel Acked-by: Jason A. Donenfeld Signed-off-by: Herbert Xu --- crypto/asymmetric_keys/asym_tpm.c | 2 +- crypto/sha1_generic.c | 2 +- crypto/sha256_generic.c | 2 +- crypto/sha512_generic.c | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crypto/asymmetric_keys/asym_tpm.c b/crypto/asymmetric_keys/asym_tpm.c index 378b18b9..511932aa 100644 --- a/crypto/asymmetric_keys/asym_tpm.c +++ b/crypto/asymmetric_keys/asym_tpm.c @@ -10,7 +10,7 @@ #include #include #include -#include +#include #include #include #include diff --git a/crypto/sha1_generic.c b/crypto/sha1_generic.c index 1d43472f..325b57fe 100644 --- a/crypto/sha1_generic.c +++ b/crypto/sha1_generic.c @@ -16,7 +16,7 @@ #include #include #include -#include +#include #include #include diff --git a/crypto/sha256_generic.c b/crypto/sha256_generic.c index 88156e3e..3b377197 100644 --- a/crypto/sha256_generic.c +++ b/crypto/sha256_generic.c @@ -12,7 +12,7 @@ #include #include #include -#include +#include #include #include #include diff --git a/crypto/sha512_generic.c b/crypto/sha512_generic.c index e34d09dd..c72d72ad 100644 --- a/crypto/sha512_generic.c +++ b/crypto/sha512_generic.c @@ -12,7 +12,7 @@ #include #include #include -#include +#include #include #include #include -- cgit v1.2.3 From ebe11b5625b5237d4560ab71c2d81b378049e3e8 Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Tue, 17 Nov 2020 14:32:11 +0100 Subject: crypto: aegis128 - wipe plaintext and tag if decryption fails The AEGIS spec mentions explicitly that the security guarantees hold only if the resulting plaintext and tag of a failed decryption are withheld. So ensure that we abide by this. While at it, drop the unused struct aead_request *req parameter from crypto_aegis128_process_crypt(). Reviewed-by: Ondrej Mosnacek Signed-off-by: Ard Biesheuvel Signed-off-by: Herbert Xu --- crypto/aegis128-core.c | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/crypto/aegis128-core.c b/crypto/aegis128-core.c index 44fb4956..3a712358 100644 --- a/crypto/aegis128-core.c +++ b/crypto/aegis128-core.c @@ -154,6 +154,12 @@ static void crypto_aegis128_ad(struct aegis_state *state, } } +static void crypto_aegis128_wipe_chunk(struct aegis_state *state, u8 *dst, + const u8 *src, unsigned int size) +{ + memzero_explicit(dst, size); +} + static void crypto_aegis128_encrypt_chunk(struct aegis_state *state, u8 *dst, const u8 *src, unsigned int size) { @@ -324,7 +330,6 @@ static void crypto_aegis128_process_ad(struct aegis_state *state, static __always_inline int crypto_aegis128_process_crypt(struct aegis_state *state, - struct aead_request *req, struct skcipher_walk *walk, void (*crypt)(struct aegis_state *state, u8 *dst, const u8 *src, @@ -403,14 +408,14 @@ static int crypto_aegis128_encrypt(struct aead_request *req) if (aegis128_do_simd()) { crypto_aegis128_init_simd(&state, &ctx->key, req->iv); crypto_aegis128_process_ad(&state, req->src, req->assoclen); - crypto_aegis128_process_crypt(&state, req, &walk, + crypto_aegis128_process_crypt(&state, &walk, crypto_aegis128_encrypt_chunk_simd); crypto_aegis128_final_simd(&state, &tag, req->assoclen, cryptlen); } else { crypto_aegis128_init(&state, &ctx->key, req->iv); crypto_aegis128_process_ad(&state, req->src, req->assoclen); - crypto_aegis128_process_crypt(&state, req, &walk, + crypto_aegis128_process_crypt(&state, &walk, crypto_aegis128_encrypt_chunk); crypto_aegis128_final(&state, &tag, req->assoclen, cryptlen); } @@ -438,19 +443,34 @@ static int crypto_aegis128_decrypt(struct aead_request *req) if (aegis128_do_simd()) { crypto_aegis128_init_simd(&state, &ctx->key, req->iv); crypto_aegis128_process_ad(&state, req->src, req->assoclen); - crypto_aegis128_process_crypt(&state, req, &walk, + crypto_aegis128_process_crypt(&state, &walk, crypto_aegis128_decrypt_chunk_simd); crypto_aegis128_final_simd(&state, &tag, req->assoclen, cryptlen); } else { crypto_aegis128_init(&state, &ctx->key, req->iv); crypto_aegis128_process_ad(&state, req->src, req->assoclen); - crypto_aegis128_process_crypt(&state, req, &walk, + crypto_aegis128_process_crypt(&state, &walk, crypto_aegis128_decrypt_chunk); crypto_aegis128_final(&state, &tag, req->assoclen, cryptlen); } - return crypto_memneq(tag.bytes, zeros, authsize) ? -EBADMSG : 0; + if (unlikely(crypto_memneq(tag.bytes, zeros, authsize))) { + /* + * From Chapter 4. 'Security Analysis' of the AEGIS spec [0] + * + * "3. If verification fails, the decrypted plaintext and the + * wrong authentication tag should not be given as output." + * + * [0] https://competitions.cr.yp.to/round3/aegisv11.pdf + */ + skcipher_walk_aead_decrypt(&walk, req, false); + crypto_aegis128_process_crypt(NULL, &walk, + crypto_aegis128_wipe_chunk); + memzero_explicit(&tag, sizeof(tag)); + return -EBADMSG; + } + return 0; } static struct aead_alg crypto_aegis128_alg = { -- cgit v1.2.3 From 71bbed437c533f08c19535abe2c1fe92cbdabc1c Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Tue, 17 Nov 2020 14:32:12 +0100 Subject: crypto: aegis128/neon - optimize tail block handling Avoid copying the tail block via a stack buffer if the total size exceeds a single AEGIS block. In this case, we can use overlapping loads and stores and NEON permutation instructions instead, which leads to a modest performance improvement on some cores (< 5%), and is slightly cleaner. Note that we still need to use a stack buffer if the entire input is smaller than 16 bytes, given that we cannot use 16 byte NEON loads and stores safely in this case. Signed-off-by: Ard Biesheuvel Reviewed-by: Ondrej Mosnacek Signed-off-by: Herbert Xu --- crypto/aegis128-neon-inner.c | 89 +++++++++++++++++++++++++++++++++++++------- 1 file changed, 75 insertions(+), 14 deletions(-) diff --git a/crypto/aegis128-neon-inner.c b/crypto/aegis128-neon-inner.c index 2a660ac1..cd1b3ad1 100644 --- a/crypto/aegis128-neon-inner.c +++ b/crypto/aegis128-neon-inner.c @@ -20,7 +20,6 @@ extern int aegis128_have_aes_insn; void *memcpy(void *dest, const void *src, size_t n); -void *memset(void *s, int c, size_t n); struct aegis128_state { uint8x16_t v[5]; @@ -173,10 +172,46 @@ void crypto_aegis128_update_neon(void *state, const void *msg) aegis128_save_state_neon(st, state); } +#ifdef CONFIG_ARM +/* + * AArch32 does not provide these intrinsics natively because it does not + * implement the underlying instructions. AArch32 only provides 64-bit + * wide vtbl.8/vtbx.8 instruction, so use those instead. + */ +static uint8x16_t vqtbl1q_u8(uint8x16_t a, uint8x16_t b) +{ + union { + uint8x16_t val; + uint8x8x2_t pair; + } __a = { a }; + + return vcombine_u8(vtbl2_u8(__a.pair, vget_low_u8(b)), + vtbl2_u8(__a.pair, vget_high_u8(b))); +} + +static uint8x16_t vqtbx1q_u8(uint8x16_t v, uint8x16_t a, uint8x16_t b) +{ + union { + uint8x16_t val; + uint8x8x2_t pair; + } __a = { a }; + + return vcombine_u8(vtbx2_u8(vget_low_u8(v), __a.pair, vget_low_u8(b)), + vtbx2_u8(vget_high_u8(v), __a.pair, vget_high_u8(b))); +} +#endif + +static const uint8_t permute[] __aligned(64) = { + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, +}; + void crypto_aegis128_encrypt_chunk_neon(void *state, void *dst, const void *src, unsigned int size) { struct aegis128_state st = aegis128_load_state_neon(state); + const int short_input = size < AEGIS_BLOCK_SIZE; uint8x16_t msg; preload_sbox(); @@ -186,7 +221,8 @@ void crypto_aegis128_encrypt_chunk_neon(void *state, void *dst, const void *src, msg = vld1q_u8(src); st = aegis128_update_neon(st, msg); - vst1q_u8(dst, msg ^ s); + msg ^= s; + vst1q_u8(dst, msg); size -= AEGIS_BLOCK_SIZE; src += AEGIS_BLOCK_SIZE; @@ -195,13 +231,26 @@ void crypto_aegis128_encrypt_chunk_neon(void *state, void *dst, const void *src, if (size > 0) { uint8x16_t s = st.v[1] ^ (st.v[2] & st.v[3]) ^ st.v[4]; - uint8_t buf[AEGIS_BLOCK_SIZE] = {}; + uint8_t buf[AEGIS_BLOCK_SIZE]; + const void *in = src; + void *out = dst; + uint8x16_t m; - memcpy(buf, src, size); - msg = vld1q_u8(buf); - st = aegis128_update_neon(st, msg); - vst1q_u8(buf, msg ^ s); - memcpy(dst, buf, size); + if (__builtin_expect(short_input, 0)) + in = out = memcpy(buf + AEGIS_BLOCK_SIZE - size, src, size); + + m = vqtbl1q_u8(vld1q_u8(in + size - AEGIS_BLOCK_SIZE), + vld1q_u8(permute + 32 - size)); + + st = aegis128_update_neon(st, m); + + vst1q_u8(out + size - AEGIS_BLOCK_SIZE, + vqtbl1q_u8(m ^ s, vld1q_u8(permute + size))); + + if (__builtin_expect(short_input, 0)) + memcpy(dst, out, size); + else + vst1q_u8(out - AEGIS_BLOCK_SIZE, msg); } aegis128_save_state_neon(st, state); @@ -211,6 +260,7 @@ void crypto_aegis128_decrypt_chunk_neon(void *state, void *dst, const void *src, unsigned int size) { struct aegis128_state st = aegis128_load_state_neon(state); + const int short_input = size < AEGIS_BLOCK_SIZE; uint8x16_t msg; preload_sbox(); @@ -228,14 +278,25 @@ void crypto_aegis128_decrypt_chunk_neon(void *state, void *dst, const void *src, if (size > 0) { uint8x16_t s = st.v[1] ^ (st.v[2] & st.v[3]) ^ st.v[4]; uint8_t buf[AEGIS_BLOCK_SIZE]; + const void *in = src; + void *out = dst; + uint8x16_t m; - vst1q_u8(buf, s); - memcpy(buf, src, size); - msg = vld1q_u8(buf) ^ s; - vst1q_u8(buf, msg); - memcpy(dst, buf, size); + if (__builtin_expect(short_input, 0)) + in = out = memcpy(buf + AEGIS_BLOCK_SIZE - size, src, size); - st = aegis128_update_neon(st, msg); + m = s ^ vqtbx1q_u8(s, vld1q_u8(in + size - AEGIS_BLOCK_SIZE), + vld1q_u8(permute + 32 - size)); + + st = aegis128_update_neon(st, m); + + vst1q_u8(out + size - AEGIS_BLOCK_SIZE, + vqtbl1q_u8(m, vld1q_u8(permute + size))); + + if (__builtin_expect(short_input, 0)) + memcpy(dst, out, size); + else + vst1q_u8(out - AEGIS_BLOCK_SIZE, msg); } aegis128_save_state_neon(st, state); -- cgit v1.2.3 From 2664e3ee1b5cedc5d4acd2c1e4c9b2f2e3a865a0 Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Tue, 17 Nov 2020 14:32:13 +0100 Subject: crypto: aegis128/neon - move final tag check to SIMD domain Instead of calculating the tag and returning it to the caller on decryption, use a SIMD compare and min across vector to perform the comparison. This is slightly more efficient, and removes the need on the caller's part to wipe the tag from memory if the decryption failed. While at it, switch to unsigned int when passing cryptlen and assoclen - we don't support input sizes where it matters anyway. Signed-off-by: Ard Biesheuvel Reviewed-by: Ondrej Mosnacek Signed-off-by: Herbert Xu --- crypto/aegis128-core.c | 21 +++++++++++++++------ crypto/aegis128-neon-inner.c | 33 +++++++++++++++++++++++++++------ crypto/aegis128-neon.c | 21 +++++++++++++++------ 3 files changed, 57 insertions(+), 18 deletions(-) diff --git a/crypto/aegis128-core.c b/crypto/aegis128-core.c index 3a712358..859c7b90 100644 --- a/crypto/aegis128-core.c +++ b/crypto/aegis128-core.c @@ -67,9 +67,11 @@ void crypto_aegis128_encrypt_chunk_simd(struct aegis_state *state, u8 *dst, const u8 *src, unsigned int size); void crypto_aegis128_decrypt_chunk_simd(struct aegis_state *state, u8 *dst, const u8 *src, unsigned int size); -void crypto_aegis128_final_simd(struct aegis_state *state, - union aegis_block *tag_xor, - u64 assoclen, u64 cryptlen); +int crypto_aegis128_final_simd(struct aegis_state *state, + union aegis_block *tag_xor, + unsigned int assoclen, + unsigned int cryptlen, + unsigned int authsize); static void crypto_aegis128_update(struct aegis_state *state) { @@ -411,7 +413,7 @@ static int crypto_aegis128_encrypt(struct aead_request *req) crypto_aegis128_process_crypt(&state, &walk, crypto_aegis128_encrypt_chunk_simd); crypto_aegis128_final_simd(&state, &tag, req->assoclen, - cryptlen); + cryptlen, 0); } else { crypto_aegis128_init(&state, &ctx->key, req->iv); crypto_aegis128_process_ad(&state, req->src, req->assoclen); @@ -445,8 +447,15 @@ static int crypto_aegis128_decrypt(struct aead_request *req) crypto_aegis128_process_ad(&state, req->src, req->assoclen); crypto_aegis128_process_crypt(&state, &walk, crypto_aegis128_decrypt_chunk_simd); - crypto_aegis128_final_simd(&state, &tag, req->assoclen, - cryptlen); + if (unlikely(crypto_aegis128_final_simd(&state, &tag, + req->assoclen, + cryptlen, authsize))) { + skcipher_walk_aead_decrypt(&walk, req, false); + crypto_aegis128_process_crypt(NULL, req, &walk, + crypto_aegis128_wipe_chunk); + return -EBADMSG; + } + return 0; } else { crypto_aegis128_init(&state, &ctx->key, req->iv); crypto_aegis128_process_ad(&state, req->src, req->assoclen); diff --git a/crypto/aegis128-neon-inner.c b/crypto/aegis128-neon-inner.c index cd1b3ad1..7de48590 100644 --- a/crypto/aegis128-neon-inner.c +++ b/crypto/aegis128-neon-inner.c @@ -199,6 +199,17 @@ static uint8x16_t vqtbx1q_u8(uint8x16_t v, uint8x16_t a, uint8x16_t b) return vcombine_u8(vtbx2_u8(vget_low_u8(v), __a.pair, vget_low_u8(b)), vtbx2_u8(vget_high_u8(v), __a.pair, vget_high_u8(b))); } + +static int8_t vminvq_s8(int8x16_t v) +{ + int8x8_t s = vpmin_s8(vget_low_s8(v), vget_high_s8(v)); + + s = vpmin_s8(s, s); + s = vpmin_s8(s, s); + s = vpmin_s8(s, s); + + return vget_lane_s8(s, 0); +} #endif static const uint8_t permute[] __aligned(64) = { @@ -302,8 +313,10 @@ void crypto_aegis128_decrypt_chunk_neon(void *state, void *dst, const void *src, aegis128_save_state_neon(st, state); } -void crypto_aegis128_final_neon(void *state, void *tag_xor, uint64_t assoclen, - uint64_t cryptlen) +int crypto_aegis128_final_neon(void *state, void *tag_xor, + unsigned int assoclen, + unsigned int cryptlen, + unsigned int authsize) { struct aegis128_state st = aegis128_load_state_neon(state); uint8x16_t v; @@ -311,13 +324,21 @@ void crypto_aegis128_final_neon(void *state, void *tag_xor, uint64_t assoclen, preload_sbox(); - v = st.v[3] ^ (uint8x16_t)vcombine_u64(vmov_n_u64(8 * assoclen), - vmov_n_u64(8 * cryptlen)); + v = st.v[3] ^ (uint8x16_t)vcombine_u64(vmov_n_u64(8ULL * assoclen), + vmov_n_u64(8ULL * cryptlen)); for (i = 0; i < 7; i++) st = aegis128_update_neon(st, v); - v = vld1q_u8(tag_xor); - v ^= st.v[0] ^ st.v[1] ^ st.v[2] ^ st.v[3] ^ st.v[4]; + v = st.v[0] ^ st.v[1] ^ st.v[2] ^ st.v[3] ^ st.v[4]; + + if (authsize > 0) { + v = vqtbl1q_u8(~vceqq_u8(v, vld1q_u8(tag_xor)), + vld1q_u8(permute + authsize)); + + return vminvq_s8((int8x16_t)v); + } + vst1q_u8(tag_xor, v); + return 0; } diff --git a/crypto/aegis128-neon.c b/crypto/aegis128-neon.c index 8271b1fa..94d591a0 100644 --- a/crypto/aegis128-neon.c +++ b/crypto/aegis128-neon.c @@ -14,8 +14,10 @@ void crypto_aegis128_encrypt_chunk_neon(void *state, void *dst, const void *src, unsigned int size); void crypto_aegis128_decrypt_chunk_neon(void *state, void *dst, const void *src, unsigned int size); -void crypto_aegis128_final_neon(void *state, void *tag_xor, uint64_t assoclen, - uint64_t cryptlen); +int crypto_aegis128_final_neon(void *state, void *tag_xor, + unsigned int assoclen, + unsigned int cryptlen, + unsigned int authsize); int aegis128_have_aes_insn __ro_after_init; @@ -60,11 +62,18 @@ void crypto_aegis128_decrypt_chunk_simd(union aegis_block *state, u8 *dst, kernel_neon_end(); } -void crypto_aegis128_final_simd(union aegis_block *state, - union aegis_block *tag_xor, - u64 assoclen, u64 cryptlen) +int crypto_aegis128_final_simd(union aegis_block *state, + union aegis_block *tag_xor, + unsigned int assoclen, + unsigned int cryptlen, + unsigned int authsize) { + int ret; + kernel_neon_begin(); - crypto_aegis128_final_neon(state, tag_xor, assoclen, cryptlen); + ret = crypto_aegis128_final_neon(state, tag_xor, assoclen, cryptlen, + authsize); kernel_neon_end(); + + return ret; } -- cgit v1.2.3 From 34b8df62f1aa4d89e4865740d95bf763575afb7d Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Tue, 17 Nov 2020 14:32:14 +0100 Subject: crypto: aegis128 - expose SIMD code path as separate driver Wiring the SIMD code into the generic driver has the unfortunate side effect that the tcrypt testing code cannot distinguish them, and will therefore not use the latter to fuzz test the former, as it does for other algorithms. So let's refactor the code a bit so we can register two implementations: aegis128-generic and aegis128-simd. Signed-off-by: Ard Biesheuvel Reviewed-by: Ondrej Mosnacek Signed-off-by: Herbert Xu --- crypto/aegis128-core.c | 220 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 143 insertions(+), 77 deletions(-) diff --git a/crypto/aegis128-core.c b/crypto/aegis128-core.c index 859c7b90..2b05f794 100644 --- a/crypto/aegis128-core.c +++ b/crypto/aegis128-core.c @@ -86,9 +86,10 @@ static void crypto_aegis128_update(struct aegis_state *state) } static void crypto_aegis128_update_a(struct aegis_state *state, - const union aegis_block *msg) + const union aegis_block *msg, + bool do_simd) { - if (aegis128_do_simd()) { + if (do_simd) { crypto_aegis128_update_simd(state, msg); return; } @@ -97,9 +98,10 @@ static void crypto_aegis128_update_a(struct aegis_state *state, crypto_aegis_block_xor(&state->blocks[0], msg); } -static void crypto_aegis128_update_u(struct aegis_state *state, const void *msg) +static void crypto_aegis128_update_u(struct aegis_state *state, const void *msg, + bool do_simd) { - if (aegis128_do_simd()) { + if (do_simd) { crypto_aegis128_update_simd(state, msg); return; } @@ -128,27 +130,28 @@ static void crypto_aegis128_init(struct aegis_state *state, crypto_aegis_block_xor(&state->blocks[4], &crypto_aegis_const[1]); for (i = 0; i < 5; i++) { - crypto_aegis128_update_a(state, key); - crypto_aegis128_update_a(state, &key_iv); + crypto_aegis128_update_a(state, key, false); + crypto_aegis128_update_a(state, &key_iv, false); } } static void crypto_aegis128_ad(struct aegis_state *state, - const u8 *src, unsigned int size) + const u8 *src, unsigned int size, + bool do_simd) { if (AEGIS_ALIGNED(src)) { const union aegis_block *src_blk = (const union aegis_block *)src; while (size >= AEGIS_BLOCK_SIZE) { - crypto_aegis128_update_a(state, src_blk); + crypto_aegis128_update_a(state, src_blk, do_simd); size -= AEGIS_BLOCK_SIZE; src_blk++; } } else { while (size >= AEGIS_BLOCK_SIZE) { - crypto_aegis128_update_u(state, src); + crypto_aegis128_update_u(state, src, do_simd); size -= AEGIS_BLOCK_SIZE; src += AEGIS_BLOCK_SIZE; @@ -180,7 +183,7 @@ static void crypto_aegis128_encrypt_chunk(struct aegis_state *state, u8 *dst, crypto_aegis_block_xor(&tmp, &state->blocks[1]); crypto_aegis_block_xor(&tmp, src_blk); - crypto_aegis128_update_a(state, src_blk); + crypto_aegis128_update_a(state, src_blk, false); *dst_blk = tmp; @@ -196,7 +199,7 @@ static void crypto_aegis128_encrypt_chunk(struct aegis_state *state, u8 *dst, crypto_aegis_block_xor(&tmp, &state->blocks[1]); crypto_xor(tmp.bytes, src, AEGIS_BLOCK_SIZE); - crypto_aegis128_update_u(state, src); + crypto_aegis128_update_u(state, src, false); memcpy(dst, tmp.bytes, AEGIS_BLOCK_SIZE); @@ -215,7 +218,7 @@ static void crypto_aegis128_encrypt_chunk(struct aegis_state *state, u8 *dst, crypto_aegis_block_xor(&tmp, &state->blocks[4]); crypto_aegis_block_xor(&tmp, &state->blocks[1]); - crypto_aegis128_update_a(state, &msg); + crypto_aegis128_update_a(state, &msg, false); crypto_aegis_block_xor(&msg, &tmp); @@ -241,7 +244,7 @@ static void crypto_aegis128_decrypt_chunk(struct aegis_state *state, u8 *dst, crypto_aegis_block_xor(&tmp, &state->blocks[1]); crypto_aegis_block_xor(&tmp, src_blk); - crypto_aegis128_update_a(state, &tmp); + crypto_aegis128_update_a(state, &tmp, false); *dst_blk = tmp; @@ -257,7 +260,7 @@ static void crypto_aegis128_decrypt_chunk(struct aegis_state *state, u8 *dst, crypto_aegis_block_xor(&tmp, &state->blocks[1]); crypto_xor(tmp.bytes, src, AEGIS_BLOCK_SIZE); - crypto_aegis128_update_a(state, &tmp); + crypto_aegis128_update_a(state, &tmp, false); memcpy(dst, tmp.bytes, AEGIS_BLOCK_SIZE); @@ -279,7 +282,7 @@ static void crypto_aegis128_decrypt_chunk(struct aegis_state *state, u8 *dst, memset(msg.bytes + size, 0, AEGIS_BLOCK_SIZE - size); - crypto_aegis128_update_a(state, &msg); + crypto_aegis128_update_a(state, &msg, false); memcpy(dst, msg.bytes, size); } @@ -287,7 +290,8 @@ static void crypto_aegis128_decrypt_chunk(struct aegis_state *state, u8 *dst, static void crypto_aegis128_process_ad(struct aegis_state *state, struct scatterlist *sg_src, - unsigned int assoclen) + unsigned int assoclen, + bool do_simd) { struct scatter_walk walk; union aegis_block buf; @@ -304,13 +308,13 @@ static void crypto_aegis128_process_ad(struct aegis_state *state, if (pos > 0) { unsigned int fill = AEGIS_BLOCK_SIZE - pos; memcpy(buf.bytes + pos, src, fill); - crypto_aegis128_update_a(state, &buf); + crypto_aegis128_update_a(state, &buf, do_simd); pos = 0; left -= fill; src += fill; } - crypto_aegis128_ad(state, src, left); + crypto_aegis128_ad(state, src, left, do_simd); src += left & ~(AEGIS_BLOCK_SIZE - 1); left &= AEGIS_BLOCK_SIZE - 1; } @@ -326,7 +330,7 @@ static void crypto_aegis128_process_ad(struct aegis_state *state, if (pos > 0) { memset(buf.bytes + pos, 0, AEGIS_BLOCK_SIZE - pos); - crypto_aegis128_update_a(state, &buf); + crypto_aegis128_update_a(state, &buf, do_simd); } } @@ -368,7 +372,7 @@ static void crypto_aegis128_final(struct aegis_state *state, crypto_aegis_block_xor(&tmp, &state->blocks[3]); for (i = 0; i < 7; i++) - crypto_aegis128_update_a(state, &tmp); + crypto_aegis128_update_a(state, &tmp, false); for (i = 0; i < AEGIS128_STATE_BLOCKS; i++) crypto_aegis_block_xor(tag_xor, &state->blocks[i]); @@ -396,7 +400,7 @@ static int crypto_aegis128_setauthsize(struct crypto_aead *tfm, return 0; } -static int crypto_aegis128_encrypt(struct aead_request *req) +static int crypto_aegis128_encrypt_generic(struct aead_request *req) { struct crypto_aead *tfm = crypto_aead_reqtfm(req); union aegis_block tag = {}; @@ -407,27 +411,18 @@ static int crypto_aegis128_encrypt(struct aead_request *req) struct aegis_state state; skcipher_walk_aead_encrypt(&walk, req, false); - if (aegis128_do_simd()) { - crypto_aegis128_init_simd(&state, &ctx->key, req->iv); - crypto_aegis128_process_ad(&state, req->src, req->assoclen); - crypto_aegis128_process_crypt(&state, &walk, - crypto_aegis128_encrypt_chunk_simd); - crypto_aegis128_final_simd(&state, &tag, req->assoclen, - cryptlen, 0); - } else { - crypto_aegis128_init(&state, &ctx->key, req->iv); - crypto_aegis128_process_ad(&state, req->src, req->assoclen); - crypto_aegis128_process_crypt(&state, &walk, - crypto_aegis128_encrypt_chunk); - crypto_aegis128_final(&state, &tag, req->assoclen, cryptlen); - } + crypto_aegis128_init(&state, &ctx->key, req->iv); + crypto_aegis128_process_ad(&state, req->src, req->assoclen, false); + crypto_aegis128_process_crypt(&state, &walk, + crypto_aegis128_encrypt_chunk); + crypto_aegis128_final(&state, &tag, req->assoclen, cryptlen); scatterwalk_map_and_copy(tag.bytes, req->dst, req->assoclen + cryptlen, authsize, 1); return 0; } -static int crypto_aegis128_decrypt(struct aead_request *req) +static int crypto_aegis128_decrypt_generic(struct aead_request *req) { static const u8 zeros[AEGIS128_MAX_AUTH_SIZE] = {}; struct crypto_aead *tfm = crypto_aead_reqtfm(req); @@ -442,27 +437,11 @@ static int crypto_aegis128_decrypt(struct aead_request *req) authsize, 0); skcipher_walk_aead_decrypt(&walk, req, false); - if (aegis128_do_simd()) { - crypto_aegis128_init_simd(&state, &ctx->key, req->iv); - crypto_aegis128_process_ad(&state, req->src, req->assoclen); - crypto_aegis128_process_crypt(&state, &walk, - crypto_aegis128_decrypt_chunk_simd); - if (unlikely(crypto_aegis128_final_simd(&state, &tag, - req->assoclen, - cryptlen, authsize))) { - skcipher_walk_aead_decrypt(&walk, req, false); - crypto_aegis128_process_crypt(NULL, req, &walk, - crypto_aegis128_wipe_chunk); - return -EBADMSG; - } - return 0; - } else { - crypto_aegis128_init(&state, &ctx->key, req->iv); - crypto_aegis128_process_ad(&state, req->src, req->assoclen); - crypto_aegis128_process_crypt(&state, &walk, - crypto_aegis128_decrypt_chunk); - crypto_aegis128_final(&state, &tag, req->assoclen, cryptlen); - } + crypto_aegis128_init(&state, &ctx->key, req->iv); + crypto_aegis128_process_ad(&state, req->src, req->assoclen, false); + crypto_aegis128_process_crypt(&state, &walk, + crypto_aegis128_decrypt_chunk); + crypto_aegis128_final(&state, &tag, req->assoclen, cryptlen); if (unlikely(crypto_memneq(tag.bytes, zeros, authsize))) { /* @@ -482,42 +461,128 @@ static int crypto_aegis128_decrypt(struct aead_request *req) return 0; } -static struct aead_alg crypto_aegis128_alg = { - .setkey = crypto_aegis128_setkey, - .setauthsize = crypto_aegis128_setauthsize, - .encrypt = crypto_aegis128_encrypt, - .decrypt = crypto_aegis128_decrypt, +static int crypto_aegis128_encrypt_simd(struct aead_request *req) +{ + struct crypto_aead *tfm = crypto_aead_reqtfm(req); + union aegis_block tag = {}; + unsigned int authsize = crypto_aead_authsize(tfm); + struct aegis_ctx *ctx = crypto_aead_ctx(tfm); + unsigned int cryptlen = req->cryptlen; + struct skcipher_walk walk; + struct aegis_state state; - .ivsize = AEGIS128_NONCE_SIZE, - .maxauthsize = AEGIS128_MAX_AUTH_SIZE, - .chunksize = AEGIS_BLOCK_SIZE, + if (!aegis128_do_simd()) + return crypto_aegis128_encrypt_generic(req); - .base = { - .cra_blocksize = 1, - .cra_ctxsize = sizeof(struct aegis_ctx), - .cra_alignmask = 0, + skcipher_walk_aead_encrypt(&walk, req, false); + crypto_aegis128_init_simd(&state, &ctx->key, req->iv); + crypto_aegis128_process_ad(&state, req->src, req->assoclen, true); + crypto_aegis128_process_crypt(&state, &walk, + crypto_aegis128_encrypt_chunk_simd); + crypto_aegis128_final_simd(&state, &tag, req->assoclen, cryptlen, 0); - .cra_priority = 100, + scatterwalk_map_and_copy(tag.bytes, req->dst, req->assoclen + cryptlen, + authsize, 1); + return 0; +} - .cra_name = "aegis128", - .cra_driver_name = "aegis128-generic", +static int crypto_aegis128_decrypt_simd(struct aead_request *req) +{ + struct crypto_aead *tfm = crypto_aead_reqtfm(req); + union aegis_block tag; + unsigned int authsize = crypto_aead_authsize(tfm); + unsigned int cryptlen = req->cryptlen - authsize; + struct aegis_ctx *ctx = crypto_aead_ctx(tfm); + struct skcipher_walk walk; + struct aegis_state state; - .cra_module = THIS_MODULE, + if (!aegis128_do_simd()) + return crypto_aegis128_decrypt_generic(req); + + scatterwalk_map_and_copy(tag.bytes, req->src, req->assoclen + cryptlen, + authsize, 0); + + skcipher_walk_aead_decrypt(&walk, req, false); + crypto_aegis128_init_simd(&state, &ctx->key, req->iv); + crypto_aegis128_process_ad(&state, req->src, req->assoclen, true); + crypto_aegis128_process_crypt(&state, &walk, + crypto_aegis128_decrypt_chunk_simd); + + if (unlikely(crypto_aegis128_final_simd(&state, &tag, req->assoclen, + cryptlen, authsize))) { + skcipher_walk_aead_decrypt(&walk, req, false); + crypto_aegis128_process_crypt(NULL, &walk, + crypto_aegis128_wipe_chunk); + return -EBADMSG; } + return 0; +} + +static struct aead_alg crypto_aegis128_alg_generic = { + .setkey = crypto_aegis128_setkey, + .setauthsize = crypto_aegis128_setauthsize, + .encrypt = crypto_aegis128_encrypt_generic, + .decrypt = crypto_aegis128_decrypt_generic, + + .ivsize = AEGIS128_NONCE_SIZE, + .maxauthsize = AEGIS128_MAX_AUTH_SIZE, + .chunksize = AEGIS_BLOCK_SIZE, + + .base.cra_blocksize = 1, + .base.cra_ctxsize = sizeof(struct aegis_ctx), + .base.cra_alignmask = 0, + .base.cra_priority = 100, + .base.cra_name = "aegis128", + .base.cra_driver_name = "aegis128-generic", + .base.cra_module = THIS_MODULE, +}; + +static struct aead_alg crypto_aegis128_alg_simd = { + .setkey = crypto_aegis128_setkey, + .setauthsize = crypto_aegis128_setauthsize, + .encrypt = crypto_aegis128_encrypt_simd, + .decrypt = crypto_aegis128_decrypt_simd, + + .ivsize = AEGIS128_NONCE_SIZE, + .maxauthsize = AEGIS128_MAX_AUTH_SIZE, + .chunksize = AEGIS_BLOCK_SIZE, + + .base.cra_blocksize = 1, + .base.cra_ctxsize = sizeof(struct aegis_ctx), + .base.cra_alignmask = 0, + .base.cra_priority = 200, + .base.cra_name = "aegis128", + .base.cra_driver_name = "aegis128-simd", + .base.cra_module = THIS_MODULE, }; static int __init crypto_aegis128_module_init(void) { + int ret; + + ret = crypto_register_aead(&crypto_aegis128_alg_generic); + if (ret) + return ret; + if (IS_ENABLED(CONFIG_CRYPTO_AEGIS128_SIMD) && - crypto_aegis128_have_simd()) + crypto_aegis128_have_simd()) { + ret = crypto_register_aead(&crypto_aegis128_alg_simd); + if (ret) { + crypto_unregister_aead(&crypto_aegis128_alg_generic); + return ret; + } static_branch_enable(&have_simd); - - return crypto_register_aead(&crypto_aegis128_alg); + } + return 0; } static void __exit crypto_aegis128_module_exit(void) { - crypto_unregister_aead(&crypto_aegis128_alg); + if (IS_ENABLED(CONFIG_CRYPTO_AEGIS128_SIMD) && + crypto_aegis128_have_simd()) + crypto_unregister_aead(&crypto_aegis128_alg_simd); + + crypto_unregister_aead(&crypto_aegis128_alg_generic); } subsys_initcall(crypto_aegis128_module_init); @@ -528,3 +593,4 @@ MODULE_AUTHOR("Ondrej Mosnacek "); MODULE_DESCRIPTION("AEGIS-128 AEAD algorithm"); MODULE_ALIAS_CRYPTO("aegis128"); MODULE_ALIAS_CRYPTO("aegis128-generic"); +MODULE_ALIAS_CRYPTO("aegis128-simd"); -- cgit v1.2.3 From dc3517474c0906e5dc11d1affd285f93edefedd0 Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Fri, 20 Nov 2020 12:04:31 +0100 Subject: crypto: tcrypt - don't initialize at subsys_initcall time Commit 23311b0ed61a5 ("crypto: run initcalls for generic implementations earlier") converted tcrypt.ko's module_init() to subsys_initcall(), but this was unintentional: tcrypt.ko currently cannot be built into the core kernel, and so the subsys_initcall() gets converted into module_init() under the hood. Given that tcrypt.ko does not implement a generic version of a crypto algorithm that has to be available early during boot, there is no point in running the tcrypt init code earlier than implied by module_init(). However, for crypto development purposes, we will lift the restriction that tcrypt.ko must be built as a module, and when builtin, it makes sense for tcrypt.ko (which does its work inside the module init function) to run as late as possible. So let's switch to late_initcall() instead. Signed-off-by: Ard Biesheuvel Reviewed-by: Eric Biggers Signed-off-by: Herbert Xu --- crypto/tcrypt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c index eea0f453..fc1f3e51 100644 --- a/crypto/tcrypt.c +++ b/crypto/tcrypt.c @@ -3066,7 +3066,7 @@ err_free_tv: */ static void __exit tcrypt_mod_fini(void) { } -subsys_initcall(tcrypt_mod_init); +late_initcall(tcrypt_mod_init); module_exit(tcrypt_mod_fini); module_param(alg, charp, 0); -- cgit v1.2.3 From 0e7a9e269de5500fc2e6eda8e105b7a89c7164c5 Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Fri, 20 Nov 2020 12:04:32 +0100 Subject: crypto: tcrypt - permit tcrypt.ko to be builtin When working on crypto algorithms, being able to run tcrypt quickly without booting an entire Linux installation can be very useful. For instance, QEMU/kvm can be used to boot a kernel from the command line, and having tcrypt.ko builtin would allow tcrypt to be executed to run benchmarks, or to run tests for algorithms that need to be instantiated from templates, without the need to make it past the point where the rootfs is mounted. So let's relax the requirement that tcrypt can only be built as a module when CONFIG_EXPERT is enabled. Signed-off-by: Ard Biesheuvel Signed-off-by: Herbert Xu --- crypto/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crypto/Kconfig b/crypto/Kconfig index 37de7d00..a367fcfe 100644 --- a/crypto/Kconfig +++ b/crypto/Kconfig @@ -201,7 +201,7 @@ config CRYPTO_AUTHENC config CRYPTO_TEST tristate "Testing module" - depends on m + depends on m || EXPERT select CRYPTO_MANAGER help Quick & dirty crypto test module. -- cgit v1.2.3 From a8f0e3a397cd601adc3844d7f56a1ac53da66bda Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Fri, 20 Nov 2020 12:04:33 +0100 Subject: crypto: tcrypt - include 1420 byte blocks in aead and skcipher benchmarks WireGuard and IPsec both typically operate on input blocks that are ~1420 bytes in size, given the default Ethernet MTU of 1500 bytes and the overhead of the VPN metadata. Many aead and sckipher implementations are optimized for power-of-2 block sizes, and whether they perform well when operating on 1420 byte blocks cannot be easily extrapolated from the performance on power-of-2 block size. So let's add 1420 bytes explicitly, and round it up to the next blocksize multiple of the algo in question if it does not support 1420 byte blocks. Signed-off-by: Ard Biesheuvel Signed-off-by: Herbert Xu --- crypto/tcrypt.c | 81 +++++++++++++++++++++++++++++++-------------------------- 1 file changed, 44 insertions(+), 37 deletions(-) diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c index fc1f3e51..a647bb29 100644 --- a/crypto/tcrypt.c +++ b/crypto/tcrypt.c @@ -77,8 +77,8 @@ static const char *check[] = { NULL }; -static u32 block_sizes[] = { 16, 64, 256, 1024, 1472, 8192, 0 }; -static u32 aead_sizes[] = { 16, 64, 256, 512, 1024, 2048, 4096, 8192, 0 }; +static const int block_sizes[] = { 16, 64, 256, 1024, 1420, 4096, 0 }; +static const int aead_sizes[] = { 16, 64, 256, 512, 1024, 1420, 4096, 8192, 0 }; #define XBUFSIZE 8 #define MAX_IVLEN 32 @@ -256,10 +256,10 @@ static void test_mb_aead_speed(const char *algo, int enc, int secs, struct test_mb_aead_data *data; struct crypto_aead *tfm; unsigned int i, j, iv_len; + const int *b_size; const char *key; const char *e; void *assoc; - u32 *b_size; char *iv; int ret; @@ -337,15 +337,17 @@ static void test_mb_aead_speed(const char *algo, int enc, int secs, do { b_size = aead_sizes; do { - if (*b_size + authsize > XBUFSIZE * PAGE_SIZE) { + int bs = round_up(*b_size, crypto_aead_blocksize(tfm)); + + if (bs + authsize > XBUFSIZE * PAGE_SIZE) { pr_err("template (%u) too big for buffer (%lu)\n", - authsize + *b_size, + authsize + bs, XBUFSIZE * PAGE_SIZE); goto out; } pr_info("test %u (%d bit key, %d byte blocks): ", i, - *keysize * 8, *b_size); + *keysize * 8, bs); /* Set up tfm global state, i.e. the key */ @@ -380,11 +382,11 @@ static void test_mb_aead_speed(const char *algo, int enc, int secs, memset(assoc, 0xff, aad_size); sg_init_aead(cur->sg, cur->xbuf, - *b_size + (enc ? 0 : authsize), + bs + (enc ? 0 : authsize), assoc, aad_size); sg_init_aead(cur->sgout, cur->xoutbuf, - *b_size + (enc ? authsize : 0), + bs + (enc ? authsize : 0), assoc, aad_size); aead_request_set_ad(cur->req, aad_size); @@ -394,7 +396,7 @@ static void test_mb_aead_speed(const char *algo, int enc, int secs, aead_request_set_crypt(cur->req, cur->sgout, cur->sg, - *b_size, iv); + bs, iv); ret = crypto_aead_encrypt(cur->req); ret = do_one_aead_op(cur->req, ret); @@ -406,18 +408,18 @@ static void test_mb_aead_speed(const char *algo, int enc, int secs, } aead_request_set_crypt(cur->req, cur->sg, - cur->sgout, *b_size + + cur->sgout, bs + (enc ? 0 : authsize), iv); } if (secs) { - ret = test_mb_aead_jiffies(data, enc, *b_size, + ret = test_mb_aead_jiffies(data, enc, bs, secs, num_mb); cond_resched(); } else { - ret = test_mb_aead_cycles(data, enc, *b_size, + ret = test_mb_aead_cycles(data, enc, bs, num_mb); } @@ -534,7 +536,7 @@ static void test_aead_speed(const char *algo, int enc, unsigned int secs, char *xbuf[XBUFSIZE]; char *xoutbuf[XBUFSIZE]; char *axbuf[XBUFSIZE]; - unsigned int *b_size; + const int *b_size; unsigned int iv_len; struct crypto_wait wait; @@ -590,12 +592,14 @@ static void test_aead_speed(const char *algo, int enc, unsigned int secs, do { b_size = aead_sizes; do { + u32 bs = round_up(*b_size, crypto_aead_blocksize(tfm)); + assoc = axbuf[0]; memset(assoc, 0xff, aad_size); - if ((*keysize + *b_size) > TVMEMSIZE * PAGE_SIZE) { + if ((*keysize + bs) > TVMEMSIZE * PAGE_SIZE) { pr_err("template (%u) too big for tvmem (%lu)\n", - *keysize + *b_size, + *keysize + bs, TVMEMSIZE * PAGE_SIZE); goto out; } @@ -616,7 +620,7 @@ static void test_aead_speed(const char *algo, int enc, unsigned int secs, crypto_aead_clear_flags(tfm, ~0); printk(KERN_INFO "test %u (%d bit key, %d byte blocks): ", - i, *keysize * 8, *b_size); + i, *keysize * 8, bs); memset(tvmem[0], 0xff, PAGE_SIZE); @@ -627,11 +631,11 @@ static void test_aead_speed(const char *algo, int enc, unsigned int secs, goto out; } - sg_init_aead(sg, xbuf, *b_size + (enc ? 0 : authsize), + sg_init_aead(sg, xbuf, bs + (enc ? 0 : authsize), assoc, aad_size); sg_init_aead(sgout, xoutbuf, - *b_size + (enc ? authsize : 0), assoc, + bs + (enc ? authsize : 0), assoc, aad_size); aead_request_set_ad(req, aad_size); @@ -644,7 +648,7 @@ static void test_aead_speed(const char *algo, int enc, unsigned int secs, * reversed (input <-> output) to calculate it */ aead_request_set_crypt(req, sgout, sg, - *b_size, iv); + bs, iv); ret = do_one_aead_op(req, crypto_aead_encrypt(req)); @@ -656,15 +660,15 @@ static void test_aead_speed(const char *algo, int enc, unsigned int secs, } aead_request_set_crypt(req, sg, sgout, - *b_size + (enc ? 0 : authsize), + bs + (enc ? 0 : authsize), iv); if (secs) { - ret = test_aead_jiffies(req, enc, *b_size, + ret = test_aead_jiffies(req, enc, bs, secs); cond_resched(); } else { - ret = test_aead_cycles(req, enc, *b_size); + ret = test_aead_cycles(req, enc, bs); } if (ret) { @@ -1253,9 +1257,9 @@ static void test_mb_skcipher_speed(const char *algo, int enc, int secs, struct test_mb_skcipher_data *data; struct crypto_skcipher *tfm; unsigned int i, j, iv_len; + const int *b_size; const char *key; const char *e; - u32 *b_size; char iv[128]; int ret; @@ -1316,14 +1320,16 @@ static void test_mb_skcipher_speed(const char *algo, int enc, int secs, do { b_size = block_sizes; do { - if (*b_size > XBUFSIZE * PAGE_SIZE) { + u32 bs = round_up(*b_size, crypto_skcipher_blocksize(tfm)); + + if (bs > XBUFSIZE * PAGE_SIZE) { pr_err("template (%u) too big for buffer (%lu)\n", *b_size, XBUFSIZE * PAGE_SIZE); goto out; } pr_info("test %u (%d bit key, %d byte blocks): ", i, - *keysize * 8, *b_size); + *keysize * 8, bs); /* Set up tfm global state, i.e. the key */ @@ -1353,7 +1359,7 @@ static void test_mb_skcipher_speed(const char *algo, int enc, int secs, for (j = 0; j < num_mb; ++j) { struct test_mb_skcipher_data *cur = &data[j]; - unsigned int k = *b_size; + unsigned int k = bs; unsigned int pages = DIV_ROUND_UP(k, PAGE_SIZE); unsigned int p = 0; @@ -1377,12 +1383,12 @@ static void test_mb_skcipher_speed(const char *algo, int enc, int secs, if (secs) { ret = test_mb_acipher_jiffies(data, enc, - *b_size, secs, + bs, secs, num_mb); cond_resched(); } else { ret = test_mb_acipher_cycles(data, enc, - *b_size, num_mb); + bs, num_mb); } if (ret) { @@ -1497,8 +1503,8 @@ static void test_skcipher_speed(const char *algo, int enc, unsigned int secs, char iv[128]; struct skcipher_request *req; struct crypto_skcipher *tfm; + const int *b_size; const char *e; - u32 *b_size; if (enc == ENCRYPT) e = "encryption"; @@ -1533,17 +1539,18 @@ static void test_skcipher_speed(const char *algo, int enc, unsigned int secs, b_size = block_sizes; do { + u32 bs = round_up(*b_size, crypto_skcipher_blocksize(tfm)); struct scatterlist sg[TVMEMSIZE]; - if ((*keysize + *b_size) > TVMEMSIZE * PAGE_SIZE) { + if ((*keysize + bs) > TVMEMSIZE * PAGE_SIZE) { pr_err("template (%u) too big for " - "tvmem (%lu)\n", *keysize + *b_size, + "tvmem (%lu)\n", *keysize + bs, TVMEMSIZE * PAGE_SIZE); goto out_free_req; } pr_info("test %u (%d bit key, %d byte blocks): ", i, - *keysize * 8, *b_size); + *keysize * 8, bs); memset(tvmem[0], 0xff, PAGE_SIZE); @@ -1565,7 +1572,7 @@ static void test_skcipher_speed(const char *algo, int enc, unsigned int secs, goto out_free_req; } - k = *keysize + *b_size; + k = *keysize + bs; sg_init_table(sg, DIV_ROUND_UP(k, PAGE_SIZE)); if (k > PAGE_SIZE) { @@ -1582,22 +1589,22 @@ static void test_skcipher_speed(const char *algo, int enc, unsigned int secs, sg_set_buf(sg + j, tvmem[j], k); memset(tvmem[j], 0xff, k); } else { - sg_set_buf(sg, tvmem[0] + *keysize, *b_size); + sg_set_buf(sg, tvmem[0] + *keysize, bs); } iv_len = crypto_skcipher_ivsize(tfm); if (iv_len) memset(&iv, 0xff, iv_len); - skcipher_request_set_crypt(req, sg, sg, *b_size, iv); + skcipher_request_set_crypt(req, sg, sg, bs, iv); if (secs) { ret = test_acipher_jiffies(req, enc, - *b_size, secs); + bs, secs); cond_resched(); } else { ret = test_acipher_cycles(req, enc, - *b_size); + bs); } if (ret) { -- cgit v1.2.3 From 8319c80ab523e5735c07bb0da3bae7a93997c61e Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Tue, 24 Nov 2020 11:47:19 +0100 Subject: crypto: ecdh - avoid unaligned accesses in ecdh_set_secret() ecdh_set_secret() casts a void* pointer to a const u64* in order to feed it into ecc_is_key_valid(). This is not generally permitted by the C standard, and leads to actual misalignment faults on ARMv6 cores. In some cases, these are fixed up in software, but this still leads to performance hits that are entirely avoidable. So let's copy the key into the ctx buffer first, which we will do anyway in the common case, and which guarantees correct alignment. Cc: Signed-off-by: Ard Biesheuvel Signed-off-by: Herbert Xu --- crypto/ecdh.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/crypto/ecdh.c b/crypto/ecdh.c index b0232d6a..d56b8603 100644 --- a/crypto/ecdh.c +++ b/crypto/ecdh.c @@ -53,12 +53,13 @@ static int ecdh_set_secret(struct crypto_kpp *tfm, const void *buf, return ecc_gen_privkey(ctx->curve_id, ctx->ndigits, ctx->private_key); - if (ecc_is_key_valid(ctx->curve_id, ctx->ndigits, - (const u64 *)params.key, params.key_size) < 0) - return -EINVAL; - memcpy(ctx->private_key, params.key, params.key_size); + if (ecc_is_key_valid(ctx->curve_id, ctx->ndigits, + ctx->private_key, params.key_size) < 0) { + memzero_explicit(ctx->private_key, params.key_size); + return -EINVAL; + } return 0; } -- cgit v1.2.3 From 910e98c7ca92bb1020f4daa8a4214821dfff9735 Mon Sep 17 00:00:00 2001 From: Tom Rix Date: Fri, 27 Nov 2020 08:23:45 -0800 Subject: crypto: seed - remove trailing semicolon in macro definition The macro use will already have a semicolon. Signed-off-by: Tom Rix Signed-off-by: Herbert Xu --- crypto/seed.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crypto/seed.c b/crypto/seed.c index 5e3bef3a..27720140 100644 --- a/crypto/seed.c +++ b/crypto/seed.c @@ -322,7 +322,7 @@ static const u32 KC[SEED_NUM_KCONSTANTS] = { SS2[byte(t1, 2)] ^ SS3[byte(t1, 3)]; \ t0 += t1; \ X1 ^= t0; \ - X2 ^= t1; + X2 ^= t1 static int seed_set_key(struct crypto_tfm *tfm, const u8 *in_key, unsigned int key_len) -- cgit v1.2.3 From a9c868434b8e5aeaee517c7a86621d5b7c80fbf9 Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Mon, 30 Nov 2020 13:26:20 +0100 Subject: crypto: aegis128 - avoid spurious references crypto_aegis128_update_simd Geert reports that builds where CONFIG_CRYPTO_AEGIS128_SIMD is not set may still emit references to crypto_aegis128_update_simd(), which cannot be satisfied and therefore break the build. These references only exist in functions that can be optimized away, but apparently, the compiler is not always able to prove this. So add some explicit checks for CONFIG_CRYPTO_AEGIS128_SIMD to help the compiler figure this out. Tested-by: Geert Uytterhoeven Signed-off-by: Ard Biesheuvel Signed-off-by: Herbert Xu --- crypto/aegis128-core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crypto/aegis128-core.c b/crypto/aegis128-core.c index 2b05f794..89dc1c55 100644 --- a/crypto/aegis128-core.c +++ b/crypto/aegis128-core.c @@ -89,7 +89,7 @@ static void crypto_aegis128_update_a(struct aegis_state *state, const union aegis_block *msg, bool do_simd) { - if (do_simd) { + if (IS_ENABLED(CONFIG_CRYPTO_AEGIS128_SIMD) && do_simd) { crypto_aegis128_update_simd(state, msg); return; } @@ -101,7 +101,7 @@ static void crypto_aegis128_update_a(struct aegis_state *state, static void crypto_aegis128_update_u(struct aegis_state *state, const void *msg, bool do_simd) { - if (do_simd) { + if (IS_ENABLED(CONFIG_CRYPTO_AEGIS128_SIMD) && do_simd) { crypto_aegis128_update_simd(state, msg); return; } -- cgit v1.2.3