Re: [Patch] Increase USBFS Bulk Transfer size

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

 



Markus:

Attached is a sequence of two patches for you to try out.  The second
is meant to be applied on top of the first.  I based them on 3.1-rc4
(more or less) so they may need a little tweaking for your kernel, but
hopefully not very much.

The first patch simplifies the error pathways for the USB submission
routines in usbfs.  The second patch removes the limits on the sizes of
individual URBs and replaces them with a global limit on the total
amount of memory allocated by usbfs.

They work okay on my system but I don't have any good way to give them
a thorough test.  Please try them out and see if they allow your webcam
programs to work properly.

Alan Stern
 drivers/usb/core/devio.c |   96 ++++++++++++++++++++++++-----------------------
 1 file changed, 50 insertions(+), 46 deletions(-)

Index: usb-3.1/drivers/usb/core/devio.c
===================================================================
--- usb-3.1.orig/drivers/usb/core/devio.c
+++ usb-3.1/drivers/usb/core/devio.c
@@ -796,8 +796,8 @@ static int proc_control(struct dev_state
 	if (ctrl.bRequestType & 0x80) {
 		if (ctrl.wLength && !access_ok(VERIFY_WRITE, ctrl.data,
 					       ctrl.wLength)) {
-			free_page((unsigned long)tbuf);
-			return -EINVAL;
+			ret = -EINVAL;
+			goto done;
 		}
 		pipe = usb_rcvctrlpipe(dev, 0);
 		snoop_urb(dev, NULL, pipe, ctrl.wLength, tmo, SUBMIT, NULL, 0);
@@ -811,15 +811,15 @@ static int proc_control(struct dev_state
 			  tbuf, max(i, 0));
 		if ((i > 0) && ctrl.wLength) {
 			if (copy_to_user(ctrl.data, tbuf, i)) {
-				free_page((unsigned long)tbuf);
-				return -EFAULT;
+				ret = -EFAULT;
+				goto done;
 			}
 		}
 	} else {
 		if (ctrl.wLength) {
 			if (copy_from_user(tbuf, ctrl.data, ctrl.wLength)) {
-				free_page((unsigned long)tbuf);
-				return -EFAULT;
+				ret = -EFAULT;
+				goto done;
 			}
 		}
 		pipe = usb_sndctrlpipe(dev, 0);
@@ -833,14 +833,16 @@ static int proc_control(struct dev_state
 		usb_lock_device(dev);
 		snoop_urb(dev, NULL, pipe, max(i, 0), min(i, 0), COMPLETE, NULL, 0);
 	}
-	free_page((unsigned long)tbuf);
 	if (i < 0 && i != -EPIPE) {
 		dev_printk(KERN_DEBUG, &dev->dev, "usbfs: USBDEVFS_CONTROL "
 			   "failed cmd %s rqt %u rq %u len %u ret %d\n",
 			   current->comm, ctrl.bRequestType, ctrl.bRequest,
 			   ctrl.wLength, i);
 	}
-	return i;
+	ret = i;
+ done:
+	free_page((unsigned long) tbuf);
+	return ret;
 }
 
 static int proc_bulk(struct dev_state *ps, void __user *arg)
@@ -874,8 +876,8 @@ static int proc_bulk(struct dev_state *p
 	tmo = bulk.timeout;
 	if (bulk.ep & 0x80) {
 		if (len1 && !access_ok(VERIFY_WRITE, bulk.data, len1)) {
-			kfree(tbuf);
-			return -EINVAL;
+			ret = -EINVAL;
+			goto done;
 		}
 		snoop_urb(dev, NULL, pipe, len1, tmo, SUBMIT, NULL, 0);
 
@@ -886,15 +888,15 @@ static int proc_bulk(struct dev_state *p
 
 		if (!i && len2) {
 			if (copy_to_user(bulk.data, tbuf, len2)) {
-				kfree(tbuf);
-				return -EFAULT;
+				ret = -EFAULT;
+				goto done;
 			}
 		}
 	} else {
 		if (len1) {
 			if (copy_from_user(tbuf, bulk.data, len1)) {
-				kfree(tbuf);
-				return -EFAULT;
+				ret = -EFAULT;
+				goto done;
 			}
 		}
 		snoop_urb(dev, NULL, pipe, len1, tmo, SUBMIT, tbuf, len1);
@@ -904,10 +906,10 @@ static int proc_bulk(struct dev_state *p
 		usb_lock_device(dev);
 		snoop_urb(dev, NULL, pipe, len2, i, COMPLETE, NULL, 0);
 	}
+	ret = (i < 0 ? i : len2);
+ done:
 	kfree(tbuf);
-	if (i < 0)
-		return i;
-	return len2;
+	return ret;
 }
 
 static int proc_resetep(struct dev_state *ps, void __user *arg)
@@ -1052,7 +1054,7 @@ static int proc_do_submiturb(struct dev_
 {
 	struct usbdevfs_iso_packet_desc *isopkt = NULL;
 	struct usb_host_endpoint *ep;
-	struct async *as;
+	struct async *as = NULL;
 	struct usb_ctrlrequest *dr = NULL;
 	const struct cred *cred = current_cred();
 	unsigned int u, totlen, isofrmlen;
@@ -1099,19 +1101,17 @@ static int proc_do_submiturb(struct dev_
 		if (!dr)
 			return -ENOMEM;
 		if (copy_from_user(dr, uurb->buffer, 8)) {
-			kfree(dr);
-			return -EFAULT;
+			ret = -EFAULT;
+			goto error;
 		}
 		if (uurb->buffer_length < (le16_to_cpup(&dr->wLength) + 8)) {
-			kfree(dr);
-			return -EINVAL;
+			ret = -EINVAL;
+			goto error;
 		}
 		ret = check_ctrlrecip(ps, dr->bRequestType,
 				      le16_to_cpup(&dr->wIndex));
-		if (ret) {
-			kfree(dr);
-			return ret;
-		}
+		if (ret)
+			goto error;
 		uurb->number_of_packets = 0;
 		uurb->buffer_length = le16_to_cpup(&dr->wLength);
 		uurb->buffer += 8;
@@ -1167,22 +1167,22 @@ static int proc_do_submiturb(struct dev_
 		if (!(isopkt = kmalloc(isofrmlen, GFP_KERNEL)))
 			return -ENOMEM;
 		if (copy_from_user(isopkt, iso_frame_desc, isofrmlen)) {
-			kfree(isopkt);
-			return -EFAULT;
+			ret = -EFAULT;
+			goto error;
 		}
 		for (totlen = u = 0; u < uurb->number_of_packets; u++) {
 			/* arbitrary limit,
 			 * sufficient for USB 2.0 high-bandwidth iso */
 			if (isopkt[u].length > 8192) {
-				kfree(isopkt);
-				return -EINVAL;
+				ret = -EINVAL;
+				goto error;
 			}
 			totlen += isopkt[u].length;
 		}
 		/* 3072 * 64 microframes */
 		if (totlen > 196608) {
-			kfree(isopkt);
-			return -EINVAL;
+			ret = -EINVAL;
+			goto error;
 		}
 		uurb->buffer_length = totlen;
 		break;
@@ -1193,24 +1193,20 @@ static int proc_do_submiturb(struct dev_
 	if (uurb->buffer_length > 0 &&
 			!access_ok(is_in ? VERIFY_WRITE : VERIFY_READ,
 				uurb->buffer, uurb->buffer_length)) {
-		kfree(isopkt);
-		kfree(dr);
-		return -EFAULT;
+		ret = -EFAULT;
+		goto error;
 	}
 	as = alloc_async(uurb->number_of_packets);
 	if (!as) {
-		kfree(isopkt);
-		kfree(dr);
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto error;
 	}
 	if (uurb->buffer_length > 0) {
 		as->urb->transfer_buffer = kmalloc(uurb->buffer_length,
 				GFP_KERNEL);
 		if (!as->urb->transfer_buffer) {
-			kfree(isopkt);
-			kfree(dr);
-			free_async(as);
-			return -ENOMEM;
+			ret = -ENOMEM;
+			goto error;
 		}
 		/* Isochronous input data may end up being discontiguous
 		 * if some of the packets are short.  Clear the buffer so
@@ -1244,6 +1240,7 @@ static int proc_do_submiturb(struct dev_
 
 	as->urb->transfer_buffer_length = uurb->buffer_length;
 	as->urb->setup_packet = (unsigned char *)dr;
+	dr = NULL;
 	as->urb->start_frame = uurb->start_frame;
 	as->urb->number_of_packets = uurb->number_of_packets;
 	if (uurb->type == USBDEVFS_URB_TYPE_ISO ||
@@ -1259,6 +1256,7 @@ static int proc_do_submiturb(struct dev_
 		totlen += isopkt[u].length;
 	}
 	kfree(isopkt);
+	isopkt = NULL;
 	as->ps = ps;
 	as->userurb = arg;
 	if (is_in && uurb->buffer_length > 0)
@@ -1274,8 +1272,8 @@ static int proc_do_submiturb(struct dev_
 	if (!is_in && uurb->buffer_length > 0) {
 		if (copy_from_user(as->urb->transfer_buffer, uurb->buffer,
 				uurb->buffer_length)) {
-			free_async(as);
-			return -EFAULT;
+			ret = -EFAULT;
+			goto error;
 		}
 	}
 	snoop_urb(ps->dev, as->userurb, as->urb->pipe,
@@ -1321,10 +1319,16 @@ static int proc_do_submiturb(struct dev_
 		snoop_urb(ps->dev, as->userurb, as->urb->pipe,
 				0, ret, COMPLETE, NULL, 0);
 		async_removepending(as);
-		free_async(as);
-		return ret;
+		goto error;
 	}
 	return 0;
+
+ error:
+	kfree(isopkt);
+	kfree(dr);
+	if (as)
+		free_async(as);
+	return ret;
 }
 
 static int proc_submiturb(struct dev_state *ps, void __user *arg)
 drivers/usb/core/devio.c |   76 +++++++++++++++++++++++++++++++++++------------
 1 file changed, 57 insertions(+), 19 deletions(-)

Index: usb-3.1/drivers/usb/core/devio.c
===================================================================
--- usb-3.1.orig/drivers/usb/core/devio.c
+++ usb-3.1/drivers/usb/core/devio.c
@@ -85,6 +85,7 @@ struct async {
 	void __user *userbuffer;
 	void __user *userurb;
 	struct urb *urb;
+	unsigned int mem_usage;
 	int status;
 	u32 secid;
 	u8 bulk_addr;
@@ -107,8 +108,26 @@ enum snoop_when {
 
 #define USB_DEVICE_DEV		MKDEV(USB_DEVICE_MAJOR, 0)
 
-#define	MAX_USBFS_BUFFER_SIZE	16384
+/* Limit on the total amount of memory we can allocate for transfers */
+#define MAX_USBFS_MEMORY_USAGE	16777216	/* 16 MB */
 
+static atomic_t usbfs_memory_usage;	/* Total memory currently allocated */
+
+/* Check whether it's okay to allocate more memory for a transfer */
+static int usbfs_increase_memory_usage(unsigned amount)
+{
+	atomic_add(amount, &usbfs_memory_usage);
+	if (atomic_read(&usbfs_memory_usage) <= MAX_USBFS_MEMORY_USAGE)
+		return 0;
+	atomic_sub(amount, &usbfs_memory_usage);
+	return -ENOMEM;
+}
+
+/* Memory for a transfer is being deallocated */
+static void usbfs_decrease_memory_usage(unsigned amount)
+{
+	atomic_sub(amount, &usbfs_memory_usage);
+}
 
 static int connected(struct dev_state *ps)
 {
@@ -251,6 +270,7 @@ static void free_async(struct async *as)
 	kfree(as->urb->transfer_buffer);
 	kfree(as->urb->setup_packet);
 	usb_free_urb(as->urb);
+	usbfs_decrease_memory_usage(as->mem_usage);
 	kfree(as);
 }
 
@@ -782,9 +802,15 @@ static int proc_control(struct dev_state
 	wLength = ctrl.wLength;		/* To suppress 64k PAGE_SIZE warning */
 	if (wLength > PAGE_SIZE)
 		return -EINVAL;
+	ret = usbfs_increase_memory_usage(PAGE_SIZE + sizeof(struct urb) +
+			sizeof(struct usb_ctrlrequest));
+	if (ret)
+		return ret;
 	tbuf = (unsigned char *)__get_free_page(GFP_KERNEL);
-	if (!tbuf)
-		return -ENOMEM;
+	if (!tbuf) {
+		ret = -ENOMEM;
+		goto done;
+	}
 	tmo = ctrl.timeout;
 	snoop(&dev->dev, "control urb: bRequestType=%02x "
 		"bRequest=%02x wValue=%04x "
@@ -842,6 +868,8 @@ static int proc_control(struct dev_state
 	ret = i;
  done:
 	free_page((unsigned long) tbuf);
+	usbfs_decrease_memory_usage(PAGE_SIZE + sizeof(struct urb) +
+			sizeof(struct usb_ctrlrequest));
 	return ret;
 }
 
@@ -869,10 +897,15 @@ static int proc_bulk(struct dev_state *p
 	if (!usb_maxpacket(dev, pipe, !(bulk.ep & USB_DIR_IN)))
 		return -EINVAL;
 	len1 = bulk.len;
-	if (len1 > MAX_USBFS_BUFFER_SIZE)
+	if (len1 > MAX_USBFS_MEMORY_USAGE)
 		return -EINVAL;
-	if (!(tbuf = kmalloc(len1, GFP_KERNEL)))
-		return -ENOMEM;
+	ret = usbfs_increase_memory_usage(len1 + sizeof(struct urb));
+	if (ret)
+		return ret;
+	if (!(tbuf = kmalloc(len1, GFP_KERNEL))) {
+		ret = -ENOMEM;
+		goto done;
+	}
 	tmo = bulk.timeout;
 	if (bulk.ep & 0x80) {
 		if (len1 && !access_ok(VERIFY_WRITE, bulk.data, len1)) {
@@ -909,6 +942,7 @@ static int proc_bulk(struct dev_state *p
 	ret = (i < 0 ? i : len2);
  done:
 	kfree(tbuf);
+	usbfs_decrease_memory_usage(len1 + sizeof(struct urb));
 	return ret;
 }
 
@@ -1088,14 +1122,14 @@ static int proc_do_submiturb(struct dev_
 	}
 	if (!ep)
 		return -ENOENT;
+
+	u = 0;
 	switch(uurb->type) {
 	case USBDEVFS_URB_TYPE_CONTROL:
 		if (!usb_endpoint_xfer_control(&ep->desc))
 			return -EINVAL;
-		/* min 8 byte setup packet,
-		 * max 8 byte setup plus an arbitrary data stage */
-		if (uurb->buffer_length < 8 ||
-		    uurb->buffer_length > (8 + MAX_USBFS_BUFFER_SIZE))
+		/* min 8 byte setup packet */
+		if (uurb->buffer_length < 8)
 			return -EINVAL;
 		dr = kmalloc(sizeof(struct usb_ctrlrequest), GFP_KERNEL);
 		if (!dr)
@@ -1129,6 +1163,7 @@ static int proc_do_submiturb(struct dev_
 			__le16_to_cpup(&dr->wValue),
 			__le16_to_cpup(&dr->wIndex),
 			__le16_to_cpup(&dr->wLength));
+		u = sizeof(struct usb_ctrlrequest);
 		break;
 
 	case USBDEVFS_URB_TYPE_BULK:
@@ -1142,8 +1177,6 @@ static int proc_do_submiturb(struct dev_
 			goto interrupt_urb;
 		}
 		uurb->number_of_packets = 0;
-		if (uurb->buffer_length > MAX_USBFS_BUFFER_SIZE)
-			return -EINVAL;
 		break;
 
 	case USBDEVFS_URB_TYPE_INTERRUPT:
@@ -1151,8 +1184,6 @@ static int proc_do_submiturb(struct dev_
 			return -EINVAL;
  interrupt_urb:
 		uurb->number_of_packets = 0;
-		if (uurb->buffer_length > MAX_USBFS_BUFFER_SIZE)
-			return -EINVAL;
 		break;
 
 	case USBDEVFS_URB_TYPE_ISO:
@@ -1179,17 +1210,18 @@ static int proc_do_submiturb(struct dev_
 			}
 			totlen += isopkt[u].length;
 		}
-		/* 3072 * 64 microframes */
-		if (totlen > 196608) {
-			ret = -EINVAL;
-			goto error;
-		}
+		u *= sizeof(struct usb_iso_packet_descriptor);
 		uurb->buffer_length = totlen;
 		break;
 
 	default:
 		return -EINVAL;
 	}
+
+	if (uurb->buffer_length > MAX_USBFS_MEMORY_USAGE) {
+		ret = -EINVAL;
+		goto error;
+	}
 	if (uurb->buffer_length > 0 &&
 			!access_ok(is_in ? VERIFY_WRITE : VERIFY_READ,
 				uurb->buffer, uurb->buffer_length)) {
@@ -1201,6 +1233,12 @@ static int proc_do_submiturb(struct dev_
 		ret = -ENOMEM;
 		goto error;
 	}
+	u += sizeof(struct async) + sizeof(struct urb) + uurb->buffer_length;
+	ret = usbfs_increase_memory_usage(u);
+	if (ret)
+		goto error;
+	as->mem_usage = u;
+
 	if (uurb->buffer_length > 0) {
 		as->urb->transfer_buffer = kmalloc(uurb->buffer_length,
 				GFP_KERNEL);

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

  Powered by Linux