Those of you making heavy use of usbfs, please see if this hurts.
It should just filter out some bogus requests that were previously
passed through, and replace some cryptic indirect logic with code
that's more obviously correct.
I've done basic sanity testing with it; gphoto2 still seems not
to know how to recover from certain PTP states, but it can talk
with a camera, as can 'lsusb'.
- Dave
Some of the usbdevfs proc_submiturb() logic has long embedded bogus
assumptions, notably these three which are at least partly corrected
by this patch:
- All eight endpoint bits mean something ... when only four do,
the others must be zero;
- Endpoints other than ep0 can support control transfers ... in
theory that's true, but no hardware seems to support it. Since
there are places contrary assumptions are made, so that such
transfers would need real testing, this rejects such tranfers.
- Requests from userspace are internally consistent ... hah! Well,
this takes the control request's direction from the SETUP packet,
which is where the device will find it.
This came up from looking at that rc5 patch ... it wasn't the only
dubious masking in that area.
--- 1.75/drivers/usb/core/devio.c 2005-02-28 15:34:03 -08:00
+++ edited/drivers/usb/core/devio.c 2005-02-28 15:40:00 -08:00
@@ -824,7 +824,7 @@
struct async *as;
struct usb_ctrlrequest *dr = NULL;
unsigned int u, totlen, isofrmlen;
- int ret, interval = 0, ifnum = -1;
+ int ret, interval = 0, ifnum = -1, is_in;
if (copy_from_user(&uurb, arg, sizeof(uurb)))
return -EFAULT;
@@ -835,13 +835,17 @@
return -EINVAL;
if (uurb.signr != 0 && (uurb.signr < SIGRTMIN || uurb.signr > SIGRTMAX))
return -EINVAL;
- if (!(uurb.type == USBDEVFS_URB_TYPE_CONTROL && (uurb.endpoint & ~USB_ENDPOINT_DIR_MASK) == 0)) {
+ if (uurb.endpoint & ~(USB_ENDPOINT_DIR_MASK|USB_ENDPOINT_NUMBER_MASK))
+ return -EINVAL;
+ is_in = (uurb.endpoint & USB_ENDPOINT_DIR_MASK) != 0;
+ if (uurb.type != USBDEVFS_URB_TYPE_CONTROL) {
if ((ifnum = findintfep(ps->dev, uurb.endpoint)) < 0)
return ifnum;
if ((ret = checkintf(ps, ifnum)))
return ret;
- }
- if ((uurb.endpoint & USB_ENDPOINT_DIR_MASK) != 0)
+ } else if ((uurb.endpoint & USB_ENDPOINT_NUMBER_MASK) != 0)
+ return -EINVAL;
+ if (is_in)
ep = ps->dev->ep_in [uurb.endpoint & USB_ENDPOINT_NUMBER_MASK];
else
ep = ps->dev->ep_out [uurb.endpoint & USB_ENDPOINT_NUMBER_MASK];
@@ -869,7 +873,7 @@
kfree(dr);
return ret;
}
- uurb.endpoint = (uurb.endpoint & ~USB_ENDPOINT_DIR_MASK) | (dr->bRequestType & USB_ENDPOINT_DIR_MASK);
+ uurb.endpoint = dr->bRequestType & USB_ENDPOINT_DIR_MASK;
uurb.number_of_packets = 0;
uurb.buffer_length = le16_to_cpup(&dr->wLength);
uurb.buffer += 8;
