On Fri, 25 Oct 2019 at 14:18, Tero Kristo <t-kristo@xxxxxx> wrote: > > On 25/10/2019 15:05, Ard Biesheuvel wrote: > > On Fri, 25 Oct 2019 at 13:56, Tero Kristo <t-kristo@xxxxxx> wrote: > >> > >> On 25/10/2019 14:55, Tero Kristo wrote: > >>> On 25/10/2019 14:33, Ard Biesheuvel wrote: > >>>> On Thu, 17 Oct 2019 at 14:26, Tero Kristo <t-kristo@xxxxxx> wrote: > >>>>> > >>>>> Hi, > >>>>> > >>>>> This series fixes a number of bugs with omap crypto implementation. > >>>>> These have become evident with the changes to the cryptomanager, where > >>>>> it adds some new test cases and modifies some existing, namely the split > >>>>> update tests. Also, while fixing the cryptomanager induced bugs, some > >>>>> other surfaced with tcrypt/IPSec tests, so fixed them aswell. > >>>>> > >>>>> Patch #9 is against crypto core modifying the crypto_wait_req > >>>>> common API to have a timeout for it also, currently it waits forever > >>>>> and it is kind of difficult to see what test fails with crypto manager. > >>>>> This is not really needed for anything, but it is kind of nice to have > >>>>> (makes debugging easier.) > >>>>> > >>>>> This series has been tested on top of 5.4-rc2, with following setups, > >>>>> on AM57xx-beagle-x15 board: > >>>>> > >>>>> - crypto manager self tests > >>>>> - tcrypt performance test > >>>>> - ipsec test with strongswan > >>>>> > >>>>> This series depends on the skcipher API switch patch from Ard Biesheuvel > >>>>> [1]. > >>>>> > >>>> > >>>> Hi Tero, > >>>> > >>>> On my BeagleBone White, I am hitting the following issues after > >>>> applying these patches: > >>>> > >>>> [ 7.493903] alg: skcipher: ecb-aes-omap encryption unexpectedly > >>>> succeeded on test vector "random: len=531 klen=32"; > >>>> expected_error=-22, cfg="random: inplace may_sleep use_finup > >>>> src_divs=[44.72%@+4028, <flush>14.70%@alignmask+3, 19.45%@+4070, > >>>> 21.13%@+2728]" > >>>> [ 7.651103] alg: skcipher: cbc-aes-omap encryption unexpectedly > >>>> succeeded on test vector "random: len=1118 klen=32"; > >>>> expected_error=-22, cfg="random: may_sleep use_final > >>>> src_divs=[<reimport>41.87%@+31, <flush>58.13%@+2510]" > >>>> > >>>> These are simply a result of the ECB and CBC implementations not > >>>> returning -EINVAL when the input is not a multiple of the block size. > >>>> > >>>> [ 7.845527] alg: skcipher: blocksize for ctr-aes-omap (16) doesn't > >>>> match generic impl (1) > >>>> > >>>> This means cra_blocksize is not set to 1 as it should. If your driver > >>>> uses the skcipher walk API, it should set the walksize to > >>>> AES_BLOCK_SIZE to ensure that the input is handled correctly. If you > >>>> don't, then you can disregard that part. > >>>> > >>>> [ 8.306491] alg: aead: gcm-aes-omap setauthsize unexpectedly > >>>> succeeded on test vector "random: alen=3 plen=31 authsize=6 klen=9"; > >>>> expected_error=-22 > >>>> > >>>> Another missing sanity check. GCM only permits certain authsizes. > >>>> > >>>> [ 9.074703] omap_crypto_copy_sgs: Couldn't allocate pages for > >>>> unaligned cases. > >>>> > >>>> This is not a bug, but I'm not sure if the below is related or not. > >>>> > >>>> I'll preserve the binaries, in case you need me to objdump anything. > >>> > >>> What are these tests you are executing? For me, the testmgr self test > >>> suite is passing just fine. Any extra tests you have enabled somehow? > >>> > > > > I enabled CONFIG_CRYPTO_MANAGER_EXTRA_TESTS, which enables a bunch of > > fuzz tests of the offloaded algorithms against the generic > > implementations. > > Ahha I see, let me give that a shot locally. I have so far only been > testing with the standard suite. > > > > >>> I am also running full test on different board though (am57xx), I > >>> haven't been explicitly running anything on am335x. > >> > >> Oh, and btw, did you try without my series? I think the selftests are > >> failing rather miserably without them... > >> > > > > No, I just tried a branch with mine and your patches applied. > > Could you give it a shot without the CRYPTO_MANAGER_EXTRA_TESTS, that > should pass with my series, and fail without? > The missing output IVs are fixed by this series, but it seems we need some more work to get all the wrinkles ironed out. I sent some patches on top that address a couple of them, but we still need a proper fix for the situation where only assocdata is presented, and cryptlen == 0 Feel free to merge my patches into your series, or take bits and pieces into your own patches where needed.
