* [PATCH] crypto/testmgr: don't copy from source IV too much
@ 2015-09-03 11:32 Andrey Ryabinin
2015-09-03 11:32 ` [PATCH] x86/crypto/ghash-intel: specify context size for ghash async algorithm Andrey Ryabinin
2015-09-03 13:20 ` [PATCH] crypto/testmgr: don't copy from source IV too much Herbert Xu
0 siblings, 2 replies; 8+ messages in thread
From: Andrey Ryabinin @ 2015-09-03 11:32 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, linux-crypto; +Cc: Andrey Ryabinin, linux-kernel
While the destination buffer 'iv' is MAX_IVLEN size,
the source 'template[i].iv' could be smaller. Thus
copying it via memcpy() leads to invalid memory access.
Use strlcpy() instead.
Signed-off-by: Andrey Ryabinin <aryabinin@odin.com>
---
crypto/testmgr.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index d0a42bd..d85221c 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -974,7 +974,7 @@ static int __test_skcipher(struct crypto_ablkcipher *tfm, int enc,
continue;
if (template[i].iv)
- memcpy(iv, template[i].iv, MAX_IVLEN);
+ strlcpy(iv, template[i].iv, MAX_IVLEN);
else
memset(iv, 0, MAX_IVLEN);
@@ -1049,7 +1049,7 @@ static int __test_skcipher(struct crypto_ablkcipher *tfm, int enc,
continue;
if (template[i].iv)
- memcpy(iv, template[i].iv, MAX_IVLEN);
+ strlcpy(iv, template[i].iv, MAX_IVLEN);
else
memset(iv, 0, MAX_IVLEN);
--
2.4.6
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH] x86/crypto/ghash-intel: specify context size for ghash async algorithm
2015-09-03 11:32 [PATCH] crypto/testmgr: don't copy from source IV too much Andrey Ryabinin
@ 2015-09-03 11:32 ` Andrey Ryabinin
2015-09-04 15:24 ` Herbert Xu
2015-09-03 13:20 ` [PATCH] crypto/testmgr: don't copy from source IV too much Herbert Xu
1 sibling, 1 reply; 8+ messages in thread
From: Andrey Ryabinin @ 2015-09-03 11:32 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, linux-crypto
Cc: Andrey Ryabinin, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
x86, linux-kernel
Currently context size (cra_ctxsize) doesn't specified for
ghash_async_alg. Which means it's zero. Thus crypto_create_tfm()
doesn't allocate needed space for ghash_async_ctx, so any
read/write to ctx (e.g. in ghash_async_init_tfm()) is not valid.
Signed-off-by: Andrey Ryabinin <aryabinin@odin.com>
---
arch/x86/crypto/ghash-clmulni-intel_glue.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/x86/crypto/ghash-clmulni-intel_glue.c b/arch/x86/crypto/ghash-clmulni-intel_glue.c
index 64d7cf1..440df0c 100644
--- a/arch/x86/crypto/ghash-clmulni-intel_glue.c
+++ b/arch/x86/crypto/ghash-clmulni-intel_glue.c
@@ -294,6 +294,7 @@ static struct ahash_alg ghash_async_alg = {
.cra_name = "ghash",
.cra_driver_name = "ghash-clmulni",
.cra_priority = 400,
+ .cra_ctxsize = sizeof(struct ghash_async_ctx),
.cra_flags = CRYPTO_ALG_TYPE_AHASH | CRYPTO_ALG_ASYNC,
.cra_blocksize = GHASH_BLOCK_SIZE,
.cra_type = &crypto_ahash_type,
--
2.4.6
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] crypto/testmgr: don't copy from source IV too much
2015-09-03 11:32 [PATCH] crypto/testmgr: don't copy from source IV too much Andrey Ryabinin
2015-09-03 11:32 ` [PATCH] x86/crypto/ghash-intel: specify context size for ghash async algorithm Andrey Ryabinin
@ 2015-09-03 13:20 ` Herbert Xu
2015-09-04 16:42 ` Andrey Ryabinin
2015-09-10 10:11 ` [PATCH v2] " Andrey Ryabinin
1 sibling, 2 replies; 8+ messages in thread
From: Herbert Xu @ 2015-09-03 13:20 UTC (permalink / raw)
To: Andrey Ryabinin; +Cc: David S. Miller, linux-crypto, linux-kernel
On Thu, Sep 03, 2015 at 02:32:00PM +0300, Andrey Ryabinin wrote:
> While the destination buffer 'iv' is MAX_IVLEN size,
> the source 'template[i].iv' could be smaller. Thus
> copying it via memcpy() leads to invalid memory access.
> Use strlcpy() instead.
>
> Signed-off-by: Andrey Ryabinin <aryabinin@odin.com>
Thanks for the patch. Unfortunately it's broken because the IV
is not a string and can contain NULs. So either fix it by using
the real ivsize, or change template[i].iv to a char array.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] x86/crypto/ghash-intel: specify context size for ghash async algorithm
2015-09-03 11:32 ` [PATCH] x86/crypto/ghash-intel: specify context size for ghash async algorithm Andrey Ryabinin
@ 2015-09-04 15:24 ` Herbert Xu
0 siblings, 0 replies; 8+ messages in thread
From: Herbert Xu @ 2015-09-04 15:24 UTC (permalink / raw)
To: Andrey Ryabinin
Cc: David S. Miller, linux-crypto, Thomas Gleixner, Ingo Molnar,
H. Peter Anvin, x86, linux-kernel
On Thu, Sep 03, 2015 at 02:32:01PM +0300, Andrey Ryabinin wrote:
> Currently context size (cra_ctxsize) doesn't specified for
> ghash_async_alg. Which means it's zero. Thus crypto_create_tfm()
> doesn't allocate needed space for ghash_async_ctx, so any
> read/write to ctx (e.g. in ghash_async_init_tfm()) is not valid.
>
> Signed-off-by: Andrey Ryabinin <aryabinin@odin.com>
Patch applied. Thanks!
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] crypto/testmgr: don't copy from source IV too much
2015-09-03 13:20 ` [PATCH] crypto/testmgr: don't copy from source IV too much Herbert Xu
@ 2015-09-04 16:42 ` Andrey Ryabinin
2015-09-05 2:04 ` Herbert Xu
2015-09-10 10:11 ` [PATCH v2] " Andrey Ryabinin
1 sibling, 1 reply; 8+ messages in thread
From: Andrey Ryabinin @ 2015-09-04 16:42 UTC (permalink / raw)
To: Herbert Xu; +Cc: David S. Miller, linux-crypto, linux-kernel
On 09/03/2015 04:20 PM, Herbert Xu wrote:
> On Thu, Sep 03, 2015 at 02:32:00PM +0300, Andrey Ryabinin wrote:
>> While the destination buffer 'iv' is MAX_IVLEN size,
>> the source 'template[i].iv' could be smaller. Thus
>> copying it via memcpy() leads to invalid memory access.
>> Use strlcpy() instead.
>>
>> Signed-off-by: Andrey Ryabinin <aryabinin@odin.com>
>
> Thanks for the patch. Unfortunately it's broken because the IV
> is not a string and can contain NULs. So either fix it by using
> the real ivsize,
So I've tried to use crypto_ablkcipher_ivsize(tfm) for that.
But noticed that some algs don't set ivsize (which makes it zero).
E.g. "ecb-cast6-avx" doesn't set it, but test vectors (cast6_enc_tv_template[], cast6_dec_tv_template[])
have .iv of 16bytes.
So I'm not sure what part is wrong here.
Is it wrong to use crypto_ablkcipher_ivsize(tfm) to get ivsize here?
Is it bug in 'ecb-cast6-avx'?
Or maybe something else?
> or change template[i].iv to a char array.
>
> Cheers,
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] crypto/testmgr: don't copy from source IV too much
2015-09-04 16:42 ` Andrey Ryabinin
@ 2015-09-05 2:04 ` Herbert Xu
0 siblings, 0 replies; 8+ messages in thread
From: Herbert Xu @ 2015-09-05 2:04 UTC (permalink / raw)
To: Andrey Ryabinin; +Cc: David S. Miller, linux-crypto, linux-kernel
On Fri, Sep 04, 2015 at 07:42:26PM +0300, Andrey Ryabinin wrote:
>
> But noticed that some algs don't set ivsize (which makes it zero).
> E.g. "ecb-cast6-avx" doesn't set it, but test vectors (cast6_enc_tv_template[], cast6_dec_tv_template[])
> have .iv of 16bytes.
ECB should always have an IV size of zero so this is correct.
If the test vectors contain an IV it'll simply be ignored.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] crypto/testmgr: don't copy from source IV too much
2015-09-03 13:20 ` [PATCH] crypto/testmgr: don't copy from source IV too much Herbert Xu
2015-09-04 16:42 ` Andrey Ryabinin
@ 2015-09-10 10:11 ` Andrey Ryabinin
2015-09-11 14:14 ` Herbert Xu
1 sibling, 1 reply; 8+ messages in thread
From: Andrey Ryabinin @ 2015-09-10 10:11 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, linux-crypto; +Cc: linux-kernel, Andrey Ryabinin
While the destination buffer 'iv' is MAX_IVLEN size,
the source 'template[i].iv' could be smaller, thus
memcpy may read read invalid memory.
Use crypto_skcipher_ivsize() to get real ivsize
and pass it to memcpy.
Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---
crypto/testmgr.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 35c2de1..fa18753 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -940,6 +940,7 @@ static int __test_skcipher(struct crypto_skcipher *tfm, int enc,
char *xbuf[XBUFSIZE];
char *xoutbuf[XBUFSIZE];
int ret = -ENOMEM;
+ unsigned int ivsize = crypto_skcipher_ivsize(tfm);
if (testmgr_alloc_buf(xbuf))
goto out_nobuf;
@@ -975,7 +976,7 @@ static int __test_skcipher(struct crypto_skcipher *tfm, int enc,
continue;
if (template[i].iv)
- memcpy(iv, template[i].iv, MAX_IVLEN);
+ memcpy(iv, template[i].iv, ivsize);
else
memset(iv, 0, MAX_IVLEN);
@@ -1051,7 +1052,7 @@ static int __test_skcipher(struct crypto_skcipher *tfm, int enc,
continue;
if (template[i].iv)
- memcpy(iv, template[i].iv, MAX_IVLEN);
+ memcpy(iv, template[i].iv, ivsize);
else
memset(iv, 0, MAX_IVLEN);
--
2.4.6
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] crypto/testmgr: don't copy from source IV too much
2015-09-10 10:11 ` [PATCH v2] " Andrey Ryabinin
@ 2015-09-11 14:14 ` Herbert Xu
0 siblings, 0 replies; 8+ messages in thread
From: Herbert Xu @ 2015-09-11 14:14 UTC (permalink / raw)
To: Andrey Ryabinin; +Cc: David S. Miller, linux-crypto, linux-kernel
On Thu, Sep 10, 2015 at 01:11:55PM +0300, Andrey Ryabinin wrote:
> While the destination buffer 'iv' is MAX_IVLEN size,
> the source 'template[i].iv' could be smaller, thus
> memcpy may read read invalid memory.
> Use crypto_skcipher_ivsize() to get real ivsize
> and pass it to memcpy.
>
> Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
Patch applied. Thanks!
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-09-11 14:14 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-03 11:32 [PATCH] crypto/testmgr: don't copy from source IV too much Andrey Ryabinin
2015-09-03 11:32 ` [PATCH] x86/crypto/ghash-intel: specify context size for ghash async algorithm Andrey Ryabinin
2015-09-04 15:24 ` Herbert Xu
2015-09-03 13:20 ` [PATCH] crypto/testmgr: don't copy from source IV too much Herbert Xu
2015-09-04 16:42 ` Andrey Ryabinin
2015-09-05 2:04 ` Herbert Xu
2015-09-10 10:11 ` [PATCH v2] " Andrey Ryabinin
2015-09-11 14:14 ` Herbert Xu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.