On Tue, Jul 21, 2009 at 1:23 AM, Jim Paris<jim@xxxxxxxx> wrote:
> G wrote:
>> And thanks for your help and suggestions so far, btw.
>
> 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.
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. However, I cannot find a
suitable place to do this, so in the meantime I'll use your patch,
although I do hope the kvm developers will implement a more
stable/reliable malloc()/realloc() solution in the future. 1993 bytes
isn't far from the 2048 bytes limit, and it seems to me that there are
more places in the usb code where statically sized buffer are used
which could lead to more problems of this kind. 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...
The differences between the usb code in kvm-72 (which works without a
patch) and kvm-87 are too big for me to try to find out why it works
in kvm-72.
Anyways, I'm happy. Once again, thanks.
--
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
