On Thu, 21 Aug 2003, Greg KH wrote:
> Does this actually catch anything new? And based on your other email
> messages, I think it's a bit wrong :)
>
> Also, don't fix up improper urbs, if the submitter gets it wrong, it's
> wrong, they should fix their code.
Good points. Here is a revised version (as85b). In more detail, this
patch does three things:
When a device is not in the CONFIGURED state, it disallows
transfers to any endpoint other than ep0 (currently the code allows
transfers to any control endpoint).
For control transfers, it verifies that the direction given by the
pipe is the same as the direction indicated in the setup packet (but only
if the transfer length is > 0).
For isochronous transfers, it verifies that none of the iso. frame
descriptors refers to an offset that is outside the transfer buffer. It
also initializes the error_count field to 0 so the HCD doesn't have to.
So far as I know, there aren't any existing errors this patch will detect.
(For that matter, I don't know that _any_ of the checks in
usb_submit_urb() detects an existing error.) It's intended as a defensive
measure -- as long as we're checking things, we might as well be thorough.
Alan Stern
===== urb.c 1.35 vs edited =====
--- 1.35/drivers/usb/core/urb.c Mon Aug 4 13:47:05 2003
+++ edited/drivers/usb/core/urb.c Fri Aug 22 09:58:54 2003
@@ -232,7 +232,7 @@
temp = usb_pipetype (pipe);
is_out = usb_pipeout (pipe);
- if (!usb_pipecontrol (pipe) && dev->state < USB_STATE_CONFIGURED)
+ if (usb_pipeendpoint(pipe) != 0 && dev->state < USB_STATE_CONFIGURED)
return -ENODEV;
/* (actually HCDs may need to duplicate this, endpoint might yet
@@ -242,6 +242,17 @@
if (usb_endpoint_halted (dev, usb_pipeendpoint (pipe), is_out))
return -EPIPE;
+ /* for control transfers, check the setup packet and verify the
+ * pipe direction
+ */
+ if (temp == PIPE_CONTROL && urb->transfer_buffer_length > 0) {
+ struct usb_ctrlrequest *setup =
+ (struct usb_ctrlrequest *) urb->setup_packet;
+
+ if (!setup || is_out != !(setup->bRequestType & USB_DIR_IN))
+ return -EINVAL;
+ }
+
/* FIXME there should be a sharable lock protecting us against
* config/altsetting changes and disconnects, kicking in here.
* (here == before maxpacket, and eventually endpoint type,
@@ -275,12 +286,20 @@
if (urb->number_of_packets <= 0)
return -EINVAL;
for (n = 0; n < urb->number_of_packets; n++) {
- len = urb->iso_frame_desc [n].length;
+ struct usb_iso_packet_descriptor *isopd =
+ &urb->iso_frame_desc [n];
+
+ len = isopd->length;
if (len < 0 || len > max)
return -EMSGSIZE;
- urb->iso_frame_desc [n].status = -EXDEV;
- urb->iso_frame_desc [n].actual_length = 0;
+ if (isopd->offset > urb->transfer_buffer_length ||
+ len > urb->transfer_buffer_length -
+ isopd->offset)
+ return -EMSGSIZE;
+ isopd->status = -EXDEV;
+ isopd->actual_length = 0;
}
+ urb->error_count = 0;
}
/* the I/O buffer must be mapped/unmapped, except when length=0 */
-------------------------------------------------------
This SF.net email is sponsored by: VM Ware
With VMware you can run multiple operating systems on a single machine.
WITHOUT REBOOTING! Mix Linux / Windows / Novell virtual machines
at the same time. Free trial click here:http://www.vmware.com/wl/offer/358/0
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
