Hello Thomas,
On 02/02/14(Sun) 15:39, Thomas Pfaff wrote:
> Hi.
>
> I was playing with libusb when my machine suddenly crashed.
>
> The code below, run twice, should reproduce the problem on
> i386 and amd64 with any ugen device attached, GENERIC and
> GENERIC.MP.
Thanks for the bug report!
> int
> main(void)
> {
> int n = 0;
> struct libusb_device_handle *h;
>
> libusb_init(NULL);
> h = libusb_open_device_with_vid_pid(NULL, VENDOR, PRODUCT);
> libusb_get_configuration(h, &n);
> printf("libusb_get_configuration -> %d\n", n);
> if ((n = libusb_set_configuration(h, 0)) < 0)
> printf("libusb_set_configuration -> %s\n", libusb_error_name(n));
The value '0' here is special, since there's no configuration number
with such value, our stack interprets it as "unconfigure the device".
If you look at our code, it is referenced by the magic USB_UNCONFIG_NO
define.
> libusb_close(h);
> libusb_exit(NULL);
> return 0;
> }
> $ ./bug
> libusb_get_configuration -> 1
> libusb_set_configuration -> LIBUSB_ERROR_IO
Trying to set the current configuration of a device to a configuration
number that it doesn't support should result in an I/O error. So far so
good.
> $ ./bug
> uvm_fault(0xffffffff81d47f00, 0x2, 0, 1) -> e
> kernel: page fault trap, code=0
> Stopped at usbd_get_cdesc+0x35: movzwl 0x2(%rax),%eax
> ddb{3}> trace
> usbd_get_cdesc() at usbd_get_cdesc+0x35
> usb_fill_udc_task() at usb_fill_udc_task+0x4c
> usb_task_thread() at usb_task_thread+0xb2
> end trace frame: 0x0, count: -3
> ddb{3}>
But it shouldn't leave the device descriptor with NULL pointers when the
whole stack expect them to always be valid!
> I don't have a serial console on this machine so photos
> (and code) is available at http://tp76.info/ugen-crash
> for GENERIC and GENERIC.MP on amd64.
>
> I was hoping I could narrow it down further, using ioctl's
> rather than libusb, but my kernel debugging "skills" suck,
> sorry.
Don't be sorry, no problem with that! To debug USB related problems,
you can start by compiling a new kernel with USB_DEBUG define then set
the usbdebug to something different than 0 (depending on the verbosity
you want/need). In this case since it's an ugen(4) related bug,
defining UGEN_DEBUG also helps ;)
Finally here's a diff that fixes this problem. It's not limited to
libusb and can be triggered by doing an USB_SET_CONFIG ioctl on a ugen
node.
This diff simply remove the possibility to "unconfigure" a device by
passing the magic value to usbd_set_config_no(). There's no code in
our three that does that so it should be enough for the moment.
ok?
Index: usb_subr.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/usb_subr.c,v
retrieving revision 1.96
diff -u -p -r1.96 usb_subr.c
--- usb_subr.c 15 Jan 2014 11:10:40 -0000 1.96
+++ usb_subr.c 2 Feb 2014 17:05:43 -0000
@@ -604,9 +604,6 @@ usbd_set_config_no(struct usbd_device *d
usb_config_descriptor_t cd;
usbd_status err;
- if (no == USB_UNCONFIG_NO)
- return (usbd_set_config_index(dev, USB_UNCONFIG_INDEX, msg));
-
DPRINTFN(5,("usbd_set_config_no: %d\n", no));
/* Figure out what config index to use. */
for (index = 0; index < dev->ddesc.bNumConfigurations; index++) {
