[Qemu-devel] [PATCH] cadence_gem: Avoid stack-writing buffer-overrun

[Qemu-devel] [PATCH] cadence_gem: Avoid stack-writing buffer-overrun

[Peter A. G. Crosthwaite]

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>
Signed-off-by: Peter A. G. Crosthwaite <peter.croshtwaite@petalogix.com>
---
 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.3.2

Re: [Qemu-devel] [Qemu-trivial] [PATCH] cadence_gem: Avoid stack-writing buffer-overrun

[Stefan Hajnoczi]

On Tue, Jun 19, 2012 at 04:44:38PM +1000, Peter A. G. Crosthwaite 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>
> Signed-off-by: Peter A. G. Crosthwaite <peter.croshtwaite@petalogix.com>
> ---
>  hw/cadence_gem.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

Thanks, applied to the trivial patches tree:
https://github.com/stefanha/qemu/commits/trivial-patches

Stefan

Re: [Qemu-devel] [Qemu-trivial] [PATCH] cadence_gem: Avoid stack-writing buffer-overrun

[Peter Maydell]

On 22 June 2012 10:03, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Tue, Jun 19, 2012 at 04:44:38PM +1000, Peter A. G. Crosthwaite wrote:
>> From: Jim Meyering <meyering@redhat.com>
>>
>> Use sizeof(rxbuf)-size (not sizeof(rxbuf-size)) as the number
>> of bytes to clear.

> Thanks, applied to the trivial patches tree:
> https://github.com/stefanha/qemu/commits/trivial-patches

This patch is already in an outstanding arm-devs pullreq (possibly
due to confusion on my part); it would probably be better not to
put it in the trivial-patches tree I guess.

thanks and sorry for the confusion
-- PMM

Re: [Qemu-devel] [Qemu-trivial] [PATCH] cadence_gem: Avoid stack-writing buffer-overrun

[Stefan Hajnoczi]

On Fri, Jun 22, 2012 at 10:09 AM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 22 June 2012 10:03, Stefan Hajnoczi <stefanha@gmail.com> wrote:
>> On Tue, Jun 19, 2012 at 04:44:38PM +1000, Peter A. G. Crosthwaite wrote:
>>> From: Jim Meyering <meyering@redhat.com>
>>>
>>> Use sizeof(rxbuf)-size (not sizeof(rxbuf-size)) as the number
>>> of bytes to clear.
>
>> Thanks, applied to the trivial patches tree:
>> https://github.com/stefanha/qemu/commits/trivial-patches
>
> This patch is already in an outstanding arm-devs pullreq (possibly
> due to confusion on my part); it would probably be better not to
> put it in the trivial-patches tree I guess.

Okay, I've dropped it.

Stefan