Patchwork [14/16] cadence_gem: avoid stack-writing buffer-overrun

login
register
mail settings
Submitter Peter Maydell
Date June 19, 2012, 1:31 p.m.
Message ID <1340112673-14846-15-git-send-email-peter.maydell@linaro.org>
Download mbox | patch
Permalink /patch/165726/
State New
Headers show

Comments

Peter Maydell - June 19, 2012, 1:31 p.m.
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>
Reviewed-by: Peter A.G. Crosthwaite <peter.crosthwaite@petalogix.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/cadence_gem.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Peter A. G. Crosthwaite - June 20, 2012, 1:47 a.m.
I re-sent this yesterday to trivial.

May end up getting queued for merge twice.

On Tue, Jun 19, 2012 at 11:31 PM, Peter Maydell
<peter.maydell@linaro.org> 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>
> Reviewed-by: Peter A.G. Crosthwaite <peter.crosthwaite@petalogix.com>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/cadence_gem.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> 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.1
>
>

Patch

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) {