Re: [PATCH v2 13/15] crypto: testmgr - convert hash testing to use testvec_configs

From: Christophe Leroy
Date: Thu Aug 29 2019 - 11:32:52 EST


Hi Eric,


Le 01/02/2019 Ã 08:51, Eric Biggers a ÃcritÂ:
From: Eric Biggers <ebiggers@xxxxxxxxxx>

Convert alg_test_hash() to use the new test framework, adding a list of
testvec_configs to test by default. When the extra self-tests are
enabled, randomly generated testvec_configs are tested as well.

This improves hash test coverage mainly because now all algorithms have
a variety of data layouts tested, whereas before each algorithm was
responsible for declaring its own chunked test cases which were often
missing or provided poor test coverage. The new code also tests both
the MAY_SLEEP and !MAY_SLEEP cases and buffers that cross pages.

This already found bugs in the hash walk code and in the arm32 and arm64
implementations of crct10dif.

I removed the hash chunked test vectors that were the same as
non-chunked ones, but left the ones that were unique.

Signed-off-by: Eric Biggers <ebiggers@xxxxxxxxxx>
---
crypto/testmgr.c | 795 ++++++++++++++++++++---------------------------
crypto/testmgr.h | 107 +------
2 files changed, 352 insertions(+), 550 deletions(-)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 7638090ff1b0a..96aa268ff4184 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c

[...]


-static int __test_hash(struct crypto_ahash *tfm,
- const struct hash_testvec *template, unsigned int tcount,
- enum hash_test test_type, const int align_offset)
+static int test_hash_vec_cfg(const char *driver,
+ const struct hash_testvec *vec,
+ unsigned int vec_num,
+ const struct testvec_config *cfg,
+ struct ahash_request *req,
+ struct test_sglist *tsgl,
+ u8 *hashstate)
{
- const char *algo = crypto_tfm_alg_driver_name(crypto_ahash_tfm(tfm));
- size_t digest_size = crypto_ahash_digestsize(tfm);
- unsigned int i, j, k, temp;
- struct scatterlist sg[8];
- char *result;
- char *key;
- struct ahash_request *req;
- struct crypto_wait wait;
- void *hash_buff;
- char *xbuf[XBUFSIZE];
- int ret = -ENOMEM;
-
- result = kmalloc(digest_size, GFP_KERNEL);
- if (!result)
- return ret;
- key = kmalloc(MAX_KEYLEN, GFP_KERNEL);
- if (!key)
- goto out_nobuf;
- if (testmgr_alloc_buf(xbuf))
- goto out_nobuf;
+ struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
+ 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 u32 req_flags = CRYPTO_TFM_REQ_MAY_BACKLOG | cfg->req_flags;
+ const struct test_sg_division *divs[XBUFSIZE];
+ DECLARE_CRYPTO_WAIT(wait);
+ struct kvec _input;
+ struct iov_iter input;
+ unsigned int i;
+ struct scatterlist *pending_sgl;
+ unsigned int pending_len;
+ u8 result[HASH_MAX_DIGESTSIZE + TESTMGR_POISON_LEN];

Before this patch, result was allocated with kmalloc().
Now, result is in the stack. Is there a reason for this change ?

Due to this change, the talitos driver fails when using CONFIG_VMAP_STACK, because result is not dma-able anymore.

CONFIG_DEBUG_VIRTUAL warns on:

[ 4.276401] WARNING: CPU: 0 PID: 72 at ./arch/powerpc/include/asm/io.h:804 dma_direct_map_page+0x50/0x178
[ 4.285725] CPU: 0 PID: 72 Comm: cryptomgr_test Tainted: G W 5.3.0-rc6-s3k-dev-00897-g03e8e9014403-dirty #2182
[snip registers]
[ 4.353542] NIP [c0066eac] dma_direct_map_page+0x50/0x178
[ 4.358872] LR [c0066eac] dma_direct_map_page+0x50/0x178
[ 4.364074] Call Trace:
[ 4.366533] [c9d0fc18] [c0066eac] dma_direct_map_page+0x50/0x178 (unreliable)
[ 4.373587] [c9d0fc38] [c033ac38] __map_single_talitos_ptr+0x54/0x9c
[ 4.379869] [c9d0fc48] [c033e878] ahash_process_req+0x318/0x7a8
[ 4.385739] [c9d0fca8] [c0210b68] do_ahash_op.isra.0+0x24/0x70
[ 4.391494] [c9d0fcb8] [c02130e8] test_ahash_vec_cfg+0x478/0x5a8
[ 4.397432] [c9d0fda8] [c0213b40] __alg_test_hash.isra.13+0x16c/0x334
[ 4.403797] [c9d0fe08] [c0213d84] alg_test_hash+0x7c/0x164
[ 4.409218] [c9d0fe28] [c0213f88] alg_test+0xc0/0x384
[ 4.414209] [c9d0fef8] [c020f0ec] cryptomgr_test+0x48/0x50
[ 4.419623] [c9d0ff08] [c003a818] kthread+0xe0/0x10c
[ 4.424539] [c9d0ff38] [c000e1d0] ret_from_kernel_thread+0x14/0x1c

How should this be fixed ?

Christophe