[2/2] cadence_gem: avoid stack-writing buffer-overrun

From: Jim Meyering <meyering@redhat.com>

Use sizeof(rxbuf)-size (not sizeof(rxbuf-size)) as the number
of bytes to clear.  The latter would always clear 4 or 8
bytes, possibly writing beyond the end of that stack buffer.
Alternatively, depending on the value of the "size" parameter,
it could fail to initialize the end of "rxbuf".
Spotted by coverity.

Signed-off-by: Jim Meyering <meyering@redhat.com>
---
 hw/cadence_gem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

ACK and Thanks Jim,

Reviewed-by: Peter A.G. Crosthwaite <peter.crosthwaite@petalogix.com>

On Fri, May 11, 2012 at 2:19 AM, Jim Meyering <jim@meyering.net> wrote:
> From: Jim Meyering <meyering@redhat.com>
>
> Use sizeof(rxbuf)-size (not sizeof(rxbuf-size)) as the number
> of bytes to clear.  The latter would always clear 4 or 8
> bytes, possibly writing beyond the end of that stack buffer.
> Alternatively, depending on the value of the "size" parameter,
> it could fail to initialize the end of "rxbuf".
> Spotted by coverity.
>
> Signed-off-by: Jim Meyering <meyering@redhat.com>
> ---
>  hw/cadence_gem.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/cadence_gem.c b/hw/cadence_gem.c
> index e2140ae..dbde392 100644
> --- a/hw/cadence_gem.c
> +++ b/hw/cadence_gem.c
> @@ -664,7 +664,7 @@ static ssize_t gem_receive(VLANClientState *nc, const uint8_t *buf, size_t size)
>          */
>
>         memcpy(rxbuf, buf, size);
> -        memset(rxbuf + size, 0, sizeof(rxbuf - size));
> +        memset(rxbuf + size, 0, sizeof(rxbuf) - size);
>         rxbuf_ptr = rxbuf;
>         crc_val = cpu_to_le32(crc32(0, rxbuf, MAX(size, 60)));
>         if (size < 60) {
> --
> 1.7.10.1.487.ga3935e6
>

Am 14.05.2012 06:57, schrieb Peter Crosthwaite:
> ACK and Thanks Jim,
>
> Reviewed-by: Peter A.G. Crosthwaite<peter.crosthwaite@petalogix.com>
>
> On Fri, May 11, 2012 at 2:19 AM, Jim Meyering<jim@meyering.net>  wrote:
>    
>> From: Jim Meyering<meyering@redhat.com>
>>
>> Use sizeof(rxbuf)-size (not sizeof(rxbuf-size)) as the number
>> of bytes to clear.  The latter would always clear 4 or 8
>> bytes, possibly writing beyond the end of that stack buffer.
>> Alternatively, depending on the value of the "size" parameter,
>> it could fail to initialize the end of "rxbuf".
>> Spotted by coverity.
>>
>> Signed-off-by: Jim Meyering<meyering@redhat.com>
>> ---
>>   hw/cadence_gem.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/cadence_gem.c b/hw/cadence_gem.c
>> index e2140ae..dbde392 100644
>> --- a/hw/cadence_gem.c
>> +++ b/hw/cadence_gem.c
>> @@ -664,7 +664,7 @@ static ssize_t gem_receive(VLANClientState *nc, const uint8_t *buf, size_t size)
>>           */
>>
>>          memcpy(rxbuf, buf, size);
>> -        memset(rxbuf + size, 0, sizeof(rxbuf - size));
>> +        memset(rxbuf + size, 0, sizeof(rxbuf) - size);
>>          rxbuf_ptr = rxbuf;
>>          crc_val = cpu_to_le32(crc32(0, rxbuf, MAX(size, 60)));
>>          if (size<  60) {
>> --
>> 1.7.10.1.487.ga3935e6
>>      


Ping. This patch is still missing in 1.1 and master.