lkml.org 
[lkml]   [2019]   [Aug]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v2 13/15] crypto: testmgr - convert hash testing to use testvec_configs
    From
    Date
    Hi Eric,


    Le 01/02/2019 à 08:51, Eric Biggers a écrit :
    > From: Eric Biggers <ebiggers@google.com>
    >
    > 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@google.com>
    > ---
    > 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

    \
     
     \ /
      Last update: 2019-08-29 17:33    [W:3.891 / U:0.024 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site