G wrote: > On Tue, Jul 21, 2009 at 1:23 AM, Jim Paris<jim@xxxxxxxx> 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! Great! > 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. -jim -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html