Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1218179 > unrolled thread

[PATCH] crypto/testmgr: don't copy from source IV too much

Started byAndrey Ryabinin <aryabinin@odin.com>
First post2015-09-03 13:40 +0200
Last post2015-09-05 04:10 +0200
Articles 4 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH] crypto/testmgr: don't copy from source IV too much Andrey Ryabinin <aryabinin@odin.com> - 2015-09-03 13:40 +0200
    Re: [PATCH] crypto/testmgr: don't copy from source IV too much Herbert Xu <herbert@gondor.apana.org.au> - 2015-09-03 15:30 +0200
      Re: [PATCH] crypto/testmgr: don't copy from source IV too much Andrey Ryabinin <aryabinin@odin.com> - 2015-09-04 18:50 +0200
        Re: [PATCH] crypto/testmgr: don't copy from source IV too much Herbert Xu <herbert@gondor.apana.org.au> - 2015-09-05 04:10 +0200

#1218179 — [PATCH] crypto/testmgr: don't copy from source IV too much

FromAndrey Ryabinin <aryabinin@odin.com>
Date2015-09-03 13:40 +0200
Subject[PATCH] crypto/testmgr: don't copy from source IV too much
Message-ID<q4B22-2jJ-15@gated-at.bofh.it>
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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1218247

FromHerbert Xu <herbert@gondor.apana.org.au>
Date2015-09-03 15:30 +0200
Message-ID<q4CKt-4Os-7@gated-at.bofh.it>
In reply to#1218179
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1219169

FromAndrey Ryabinin <aryabinin@odin.com>
Date2015-09-04 18:50 +0200
Message-ID<q52lB-7Jv-37@gated-at.bofh.it>
In reply to#1218247
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,
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1219415

FromHerbert Xu <herbert@gondor.apana.org.au>
Date2015-09-05 04:10 +0200
Message-ID<q5b5v-3pQ-9@gated-at.bofh.it>
In reply to#1219169
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web