G wrote:
> On Tue, Jul 21, 2009 at 1:23 AM, Jim Paris<j...@jtan.com> wrote:
> > Here's a patch to try.  I'm not familiar with the code, but it looks
> > like this buffer might be too small versus the packet lengths that
> > you're seeing, and similar definitions in hw/usb-uhci.c.
> >
> > -jim
> >
> > diff -urN kvm-87-orig/usb-linux.c kvm-87/usb-linux.c
> > --- kvm-87-orig/usb-linux.c     2009-06-23 09:32:38.000000000 -0400
> > +++ kvm-87/usb-linux.c  2009-07-20 19:15:35.000000000 -0400
> > @@ -115,7 +115,7 @@
> >     uint16_t offset;
> >     uint8_t  state;
> >     struct   usb_ctrlrequest req;
> > -    uint8_t  buffer[1024];
> > +    uint8_t  buffer[2048];
> >  };
> >
> >  typedef struct USBHostDevice {
> Yes! Applying this patch makes the crash go away! Thank you!


> In addition to enabling DEBUG and applying your debug printout
> patches, I added a debug printout right above the memcpy()s in
> usb-linux.c, and found that the memcpy() in do_token_in() is called
> multiple time (since do_token_in() is called multiple times for the
> 1993 bytes usb packet I have in my usb sniff dumps), which I guess is
> what's causing a buffer overflow as the offset is pushed beyond 1024
> bytes. But I'm not sure.

Yeah, I think that's it.

> I've looked at the code trying to figure out a better way to solve
> this, now that the problem spot has been found. To me it seems that
> malloc()ing and, when the need arises (the large 1993 bytes packets
> I'm seeing), realloc()ing the buffer, instead of using a statically
> sized buffer, would be the best solution.

Dynamically sizing the buffer might get tricky.  It looks like
hw/usb-uhci.c will go up to 2048, while hw/usb-ohci.c and
hw/usb-musb.c could potentially go up to 8192.  I think bumping it to
8192 and adding an error instead of overflowing would be good enough.
I'll try to understand the code a bit more and then spin a patch.

> One could of course redefine all buffers to be 8192 bytes instead,
> but that would just be a false sense of security, and perhaps some
> buffers need to be of a particular size to conform to the USB
> specification...

USB packets don't get that large, but the host controllers can combine
them, from what I understand.  So it's more a question of what the
host controllers can do.


To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to