2012-07-09 08:18:03

by Horia Geantă

[permalink] [raw]
Subject: [PATCH] crypto: testmgr - make the assoc data and iv contiguous for aead tests

In case of AEAD, some crypto engines expect assoc data and iv to be contiguous.
This is how native IPsec works; make testmgr's behaviour the same.
(Alternative would be to fix this in the crypto engine drivers, but this is
pricy since it would involve memory allocation and copy in the hot path.)

Signed-off-by: Horia Geanta <[email protected]>
---
crypto/testmgr.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index ee62d5c..af2a1a3 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -374,7 +374,7 @@ static int test_aead(struct crypto_aead *tfm, int enc,
unsigned int authsize;
void *input;
void *assoc;
- char iv[MAX_IVLEN];
+ char *iv;
char *xbuf[XBUFSIZE];
char *axbuf[XBUFSIZE];

@@ -412,11 +412,12 @@ static int test_aead(struct crypto_aead *tfm, int enc,

ret = -EINVAL;
if (WARN_ON(template[i].ilen > PAGE_SIZE ||
- template[i].alen > PAGE_SIZE))
+ template[i].alen + MAX_IVLEN > PAGE_SIZE))
goto out;

memcpy(input, template[i].input, template[i].ilen);
memcpy(assoc, template[i].assoc, template[i].alen);
+ iv = (char*)assoc + template[i].alen;
if (template[i].iv)
memcpy(iv, template[i].iv, MAX_IVLEN);
else
--
1.7.3.4


2012-07-09 08:19:40

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH] crypto: testmgr - make the assoc data and iv contiguous for aead tests

On Mon, Jul 09, 2012 at 11:17:43AM +0300, Horia Geanta wrote:
> In case of AEAD, some crypto engines expect assoc data and iv to be contiguous.
> This is how native IPsec works; make testmgr's behaviour the same.
> (Alternative would be to fix this in the crypto engine drivers, but this is
> pricy since it would involve memory allocation and copy in the hot path.)
>
> Signed-off-by: Horia Geanta <[email protected]>

I think we should fix the buggy driver instead.

Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Subject: RE: [PATCH] crypto: testmgr - make the assoc data and iv contiguous for aead tests

On Mon, 9 Jul 2012 11:19:35 +0300, Herbert Xu <[email protected]> wrote:
> On Mon, Jul 09, 2012 at 11:17:43AM +0300, Horia Geanta wrote:
>> In case of AEAD, some crypto engines expect assoc data and iv to be
>> contiguous. This is how native IPsec works; make testmgr's behaviour
>> the same. (Alternative would be to fix this in the crypto engine
>> drivers, but this is pricy since it would involve memory allocation and
>> copy in the hot path.)
>>
>> Signed-off-by: Horia Geanta <[email protected]>
>
> I think we should fix the buggy driver instead.
>
> Thanks,

Ok, then we'll see how to work around the HW limitation in the driver,
without affecting performance too much.

Cheers,
Horia

2012-07-09 16:28:37

by Kim Phillips

[permalink] [raw]
Subject: Re: [PATCH] crypto: testmgr - make the assoc data and iv contiguous for aead tests

On Mon, 9 Jul 2012 03:38:54 -0500
Geanta Neag Horia Ioan-B05471 <[email protected]> wrote:

> On Mon, 9 Jul 2012 11:19:35 +0300, Herbert Xu <[email protected]> wrote:
> > On Mon, Jul 09, 2012 at 11:17:43AM +0300, Horia Geanta wrote:
> >> In case of AEAD, some crypto engines expect assoc data and iv to be
> >> contiguous. This is how native IPsec works; make testmgr's behaviour
> >> the same. (Alternative would be to fix this in the crypto engine
> >> drivers, but this is pricy since it would involve memory allocation and
> >> copy in the hot path.)
> >>
> >> Signed-off-by: Horia Geanta <[email protected]>
> >
> > I think we should fix the buggy driver instead.

technically it's buggy h/w. Even though SEC 2/3 descriptors have
separate fields for assoc and iv data, the h/w produces incorrect
results if they are disjoint.

> Ok, then we'll see how to work around the HW limitation in the driver,
> without affecting performance too much.

If it comes down to more than a simple unlikely(assoc_addr +
assoclen == iv_addr) check, then we may want to introduce a
TALITOS_STRICT config, which may be further made dependent on
!CRYPTO_MANAGER_DISABLE_TESTS.

Kim