Re: [Qemu-devel] [Qemu-arm] [PATCH] cadence_gem: fix buffer overflow

[Michael S. Tsirkin]

On Thu, Jan 14, 2016 at 10:03:15AM +0000, Peter Maydell wrote:
> On 14 January 2016 at 09:43, Michael S. Tsirkin <address@hidden> wrote:
> > gem_receive copies a packet received from network into an rxbuf[2048]
> > array on stack, with size limited by descriptor length set by guest.  If
> > guest is malicious and specifies a descriptor length that is too large,
> > and should packet size exceed array size, this results in a buffer
> > overflow.
> >
> > Reported-by: 刘令 <address@hidden>
> > Signed-off-by: Michael S. Tsirkin <address@hidden>
> > ---
> >  hw/net/cadence_gem.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> > index 3639fc1..15a0786 100644
> > --- a/hw/net/cadence_gem.c
> > +++ b/hw/net/cadence_gem.c
> > @@ -862,6 +862,14 @@ static void gem_transmit(CadenceGEMState *s)
> >              break;
> >          }
> >
> > +        if (tx_desc_get_length(desc) > sizeof(tx_packet) - (p - 
> > tx_packet)) {
> > +            DB_PRINT("TX descriptor @ 0x%x too large: size 0x%x space 
> > 0x%x\n",
> > +                     (unsigned)packet_desc_addr,
> > +                     (unsigned)tx_desc_get_length(desc),
> > +                     sizeof(tx_packet) - (p - tx_packet));
> > +            break;
> > +        }
> 
> Is this what the real hardware does in this situation?
> Should we log this as a guest error?

I don't really know.  This is just consistent with what this device does
for other descriptor errors.

> > +
> >          /* Gather this fragment of the packet from "dma memory" to our 
> > contig.
> >           * buffer.
> >           */
> > --
> > MST
> >
> 
> thanks
> -- PMM