[PATCH] crypto: fix copy size causing overruns/crashing

[PATCH] crypto: fix copy size causing overruns/crashing

[Will Dietz]

[-- Attachment #1: Type: text/plain, Size: 1053 bytes --]

num_ad is already accounted for in `sizeof(iov)`
as iov has size `sizeof(struct iovec) * (num_ad+1)`.
---
 src/crypto.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/crypto.c b/src/crypto.c
index 632117d..62edd44 100644
--- a/src/crypto.c
+++ b/src/crypto.c
@@ -311,7 +311,7 @@ bool aes_siv_encrypt(const uint8_t *key, size_t key_len, const uint8_t *in,
        struct iovec iov[num_ad + 1];
        uint8_t v[16];
 
-       memcpy(iov, ad, sizeof(iov) * num_ad);
+       memcpy(iov, ad, sizeof(struct iovec) * num_ad);
        iov[num_ad].iov_base = (void *)in;
        iov[num_ad].iov_len = in_len;
        num_ad++;
@@ -368,7 +368,7 @@ bool aes_siv_decrypt(const uint8_t *key, size_t key_len, const uint8_t *in,
        if (in_len < 16)
                return false;
 
-       memcpy(iov, ad, sizeof(iov) * num_ad);
+       memcpy(iov, ad, sizeof(struct iovec) * num_ad);
        iov[num_ad].iov_base = (void *)out;
        iov[num_ad].iov_len = in_len - 16;
        num_ad++;
-- 
2.23.GIT

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] crypto: fix copy size causing overruns/crashing

[Prestwood, James]

[-- Attachment #1: Type: text/plain, Size: 982 bytes --]

Hi Will,

On Thu, 2019-10-03 at 11:35 -0500, Will Dietz wrote:
> num_ad is already accounted for in `sizeof(iov)`
> as iov has size `sizeof(struct iovec) * (num_ad+1)`.
> ---
>  src/crypto.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/crypto.c b/src/crypto.c
> index 632117d..62edd44 100644
> --- a/src/crypto.c
> +++ b/src/crypto.c
> @@ -311,7 +311,7 @@ bool aes_siv_encrypt(const uint8_t *key, size_t
> key_len, const uint8_t *in,
>         struct iovec iov[num_ad + 1];
>         uint8_t v[16];
>  
> -       memcpy(iov, ad, sizeof(iov) * num_ad);
> +       memcpy(iov, ad, sizeof(struct iovec) * num_ad);

Good catch! I'm fine with this way, but also fine with simply removing
* num_ad and leaving sizeof(iov) as is. Lets see what Denis prefers.

Thanks,
James
> _______________________________________________
> iwd mailing list -- iwd(a)lists.01.org
> To unsubscribe send an email to iwd-leave(a)lists.01.org

Re: [PATCH] crypto: fix copy size causing overruns/crashing

[Prestwood, James]

[-- Attachment #1: Type: text/plain, Size: 1353 bytes --]

On Thu, 2019-10-03 at 16:44 +0000, Prestwood, James wrote:
> Hi Will,
> 
> On Thu, 2019-10-03 at 11:35 -0500, Will Dietz wrote:
> > num_ad is already accounted for in `sizeof(iov)`
> > as iov has size `sizeof(struct iovec) * (num_ad+1)`.
> > ---
> >  src/crypto.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/crypto.c b/src/crypto.c
> > index 632117d..62edd44 100644
> > --- a/src/crypto.c
> > +++ b/src/crypto.c
> > @@ -311,7 +311,7 @@ bool aes_siv_encrypt(const uint8_t *key, size_t
> > key_len, const uint8_t *in,
> >         struct iovec iov[num_ad + 1];
> >         uint8_t v[16];
> >  
> > -       memcpy(iov, ad, sizeof(iov) * num_ad);
> > +       memcpy(iov, ad, sizeof(struct iovec) * num_ad);
> 
> Good catch! I'm fine with this way, but also fine with simply
> removing
> * num_ad and leaving sizeof(iov) as is. Lets see what Denis prefers.

Oh sorry, this wont work. That would copy past 'ad'. Nevermind, lets do
it your way ;)

> 
> Thanks,
> James
> > _______________________________________________
> > iwd mailing list -- iwd(a)lists.01.org
> > To unsubscribe send an email to iwd-leave(a)lists.01.org
> 
> _______________________________________________
> iwd mailing list -- iwd(a)lists.01.org
> To unsubscribe send an email to iwd-leave(a)lists.01.org

Re: [PATCH] crypto: fix copy size causing overruns/crashing

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 1010 bytes --]

Hi James,

On 10/3/19 11:44 AM, Prestwood, James wrote:
> Hi Will,
> 
> On Thu, 2019-10-03 at 11:35 -0500, Will Dietz wrote:
>> num_ad is already accounted for in `sizeof(iov)`
>> as iov has size `sizeof(struct iovec) * (num_ad+1)`.
>> ---
>>   src/crypto.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/crypto.c b/src/crypto.c
>> index 632117d..62edd44 100644
>> --- a/src/crypto.c
>> +++ b/src/crypto.c
>> @@ -311,7 +311,7 @@ bool aes_siv_encrypt(const uint8_t *key, size_t
>> key_len, const uint8_t *in,
>>          struct iovec iov[num_ad + 1];
>>          uint8_t v[16];
>>   
>> -       memcpy(iov, ad, sizeof(iov) * num_ad);
>> +       memcpy(iov, ad, sizeof(struct iovec) * num_ad);
> 
> Good catch! I'm fine with this way, but also fine with simply removing
> * num_ad and leaving sizeof(iov) as is. Lets see what Denis prefers.
> 

Not sure that will work since you define iov to be an array of size 
num_ad + 1?

Regards,
-Denis

Re: [PATCH] crypto: fix copy size causing overruns/crashing

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 431 bytes --]

Hi Will,

On 10/3/19 11:35 AM, Will Dietz wrote:
> num_ad is already accounted for in `sizeof(iov)`
> as iov has size `sizeof(struct iovec) * (num_ad+1)`.
> ---
>   src/crypto.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 

Can you use git send-email to submit this? When I save this message it 
comes in as multipart with the signature, etc included and seems to 
confuse git am.

Regards,
-Denis

[PATCH] crypto: fix copy size causing overruns/crashing

[Will Dietz]

[-- Attachment #1: Type: text/plain, Size: 960 bytes --]

num_ad is already accounted for in `sizeof(iov)`
as iov has size `sizeof(struct iovec) * (num_ad+1)`.
---
 src/crypto.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/crypto.c b/src/crypto.c
index 632117d..62edd44 100644
--- a/src/crypto.c
+++ b/src/crypto.c
@@ -311,7 +311,7 @@ bool aes_siv_encrypt(const uint8_t *key, size_t key_len, const uint8_t *in,
 	struct iovec iov[num_ad + 1];
 	uint8_t v[16];
 
-	memcpy(iov, ad, sizeof(iov) * num_ad);
+	memcpy(iov, ad, sizeof(struct iovec) * num_ad);
 	iov[num_ad].iov_base = (void *)in;
 	iov[num_ad].iov_len = in_len;
 	num_ad++;
@@ -368,7 +368,7 @@ bool aes_siv_decrypt(const uint8_t *key, size_t key_len, const uint8_t *in,
 	if (in_len < 16)
 		return false;
 
-	memcpy(iov, ad, sizeof(iov) * num_ad);
+	memcpy(iov, ad, sizeof(struct iovec) * num_ad);
 	iov[num_ad].iov_base = (void *)out;
 	iov[num_ad].iov_len = in_len - 16;
 	num_ad++;
-- 
2.23.0

Re: [PATCH] crypto: fix copy size causing overruns/crashing

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 291 bytes --]

Hi Will,

On 10/3/19 12:18 PM, Will Dietz wrote:
> num_ad is already accounted for in `sizeof(iov)`
> as iov has size `sizeof(struct iovec) * (num_ad+1)`.
> ---
>   src/crypto.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 

Applied, thanks.

Regards,
-Denis