[PATCH V1] USB: check sg buffer size in usb_submit_urb

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



USB spec stats that short packet can only appear at the end
of transfer. Because lost of HC(EHCI/UHCI/OHCI/...) can't
build a full packet from discontinuous buffers, we introduce
the limit in usb_submit_urb() to avoid such kind of bad sg
coming from driver.

The limit might be a bit strict:
	- platform has iommu to do sg list mapping
	- some host controllers may support to build full packet from
	discontinuous buffers.

But considered that most of HCs don't support that, and driver
need work well or keep consistent on different HCs or ARCHs, we
have to introduce the limit.

Currently, only usbtest is reported to pass such sg buffers to HC,
and other users(mass storage, usbfs) don't have the problem.

Reported-by: Konstantin Filatov <kfilatov@xxxxxxxxxxxxx>
Reported-by: Denis V. Lunev <den@xxxxxxxxxx>
Cc: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
Cc: Felipe Balbi <balbi@xxxxxx>
Signed-off-by: Ming Lei <ming.lei@xxxxxxxxxxxxx>
---
V1:
	- avoid checking the last element in if(...) as Alan suggested
	- rewrite doc according to Alan's suggestion

Thomas, please let us know if you think wireless USB needn't the check.

 drivers/usb/core/urb.c |   11 +++++++++++
 include/linux/usb.h    |    4 +++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c
index 16927fa..3b95821 100644
--- a/drivers/usb/core/urb.c
+++ b/drivers/usb/core/urb.c
@@ -7,6 +7,7 @@
 #include <linux/usb.h>
 #include <linux/wait.h>
 #include <linux/usb/hcd.h>
+#include <linux/scatterlist.h>
 
 #define to_urb(d) container_of(d, struct urb, kref)
 
@@ -413,6 +414,16 @@ int usb_submit_urb(struct urb *urb, gfp_t mem_flags)
 			urb->iso_frame_desc[n].status = -EXDEV;
 			urb->iso_frame_desc[n].actual_length = 0;
 		}
+	} else {
+		/* check sg buffer size */
+		if (urb->num_sgs) {
+			struct scatterlist *sg;
+			int i;
+
+			for_each_sg(urb->sg, sg, urb->num_sgs - 1, i)
+				if (sg->length % max)
+					return -EINVAL;
+		}
 	}
 
 	/* the I/O buffer must be mapped/unmapped, except when length=0 */
diff --git a/include/linux/usb.h b/include/linux/usb.h
index a232b7e..e99b2a1 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -1258,7 +1258,9 @@ typedef void (*usb_complete_t)(struct urb *);
  *	the device driver is saying that it provided this DMA address,
  *	which the host controller driver should use in preference to the
  *	transfer_buffer.
- * @sg: scatter gather buffer list
+ * @sg: scatter gather buffer list, the buffer size of each element in
+ * 	the list (except the last) must be divisible by the endpoint's
+ * 	max packet size
  * @num_mapped_sgs: (internal) number of mapped sg entries
  * @num_sgs: number of entries in the sg list
  * @transfer_buffer_length: How big is transfer_buffer.  The transfer may
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux