[PATCH 8/8] USB: handle zero-length usbfs submissions correctly

[Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>]

This patch (as1262) fixes a bug in usbfs: It refuses to accept
zero-length transfers, and it insists that the buffer pointer be valid
even if there is no data being transferred.

The patch also consolidates a bunch of repetitive access_ok() checks
into a single check, which incidentally fixes the lack of such a check
for Isochronous URBs.

Signed-off-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>

---

This should go in 2.6.31 and in .stable.  You may find that a little 
fiddling is needed to get it to apply without the 7/8 patch present.


Index: usb-2.6/drivers/usb/core/devio.c
===================================================================
--- usb-2.6.orig/drivers/usb/core/devio.c
+++ usb-2.6/drivers/usb/core/devio.c
@@ -984,7 +984,7 @@ static int proc_do_submiturb(struct dev_
 				USBDEVFS_URB_ZERO_PACKET |
 				USBDEVFS_URB_NO_INTERRUPT))
 		return -EINVAL;
-	if (!uurb->buffer)
+	if (uurb->buffer_length > 0 && !uurb->buffer)
 		return -EINVAL;
 	if (!(uurb->type == USBDEVFS_URB_TYPE_CONTROL &&
 	    (uurb->endpoint & ~USB_ENDPOINT_DIR_MASK) == 0)) {
@@ -1040,11 +1040,6 @@ static int proc_do_submiturb(struct dev_
 			is_in = 0;
 			uurb->endpoint &= ~USB_DIR_IN;
 		}
-		if (!access_ok(is_in ? VERIFY_WRITE : VERIFY_READ,
-				uurb->buffer, uurb->buffer_length)) {
-			kfree(dr);
-			return -EFAULT;
-		}
 		break;
 
 	case USBDEVFS_URB_TYPE_BULK:
@@ -1057,9 +1052,6 @@ static int proc_do_submiturb(struct dev_
 		uurb->number_of_packets = 0;
 		if (uurb->buffer_length > MAX_USBFS_BUFFER_SIZE)
 			return -EINVAL;
-		if (!access_ok(is_in ? VERIFY_WRITE : VERIFY_READ,
-				uurb->buffer, uurb->buffer_length))
-			return -EFAULT;
 		break;
 
 	case USBDEVFS_URB_TYPE_ISO:
@@ -1099,27 +1091,34 @@ static int proc_do_submiturb(struct dev_
 			return -EINVAL;
 		if (uurb->buffer_length > MAX_USBFS_BUFFER_SIZE)
 			return -EINVAL;
-		if (!access_ok(is_in ? VERIFY_WRITE : VERIFY_READ,
-				uurb->buffer, uurb->buffer_length))
-			return -EFAULT;
 		break;
 
 	default:
 		return -EINVAL;
 	}
-	as = alloc_async(uurb->number_of_packets);
-	if (!as) {
+	if (uurb->buffer_length > 0 &&
+			!access_ok(is_in ? VERIFY_WRITE : VERIFY_READ,
+				uurb->buffer, uurb->buffer_length)) {
 		kfree(isopkt);
 		kfree(dr);
-		return -ENOMEM;
+		return -EFAULT;
 	}
-	as->urb->transfer_buffer = kmalloc(uurb->buffer_length, GFP_KERNEL);
-	if (!as->urb->transfer_buffer) {
+	as = alloc_async(uurb->number_of_packets);
+	if (!as) {
 		kfree(isopkt);
 		kfree(dr);
-		free_async(as);
 		return -ENOMEM;
 	}
+	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;
+		}
+	}
 	as->urb->dev = ps->dev;
 	as->urb->pipe = (uurb->type << 30) |
 			__create_pipe(ps->dev, uurb->endpoint & 0xf) |
@@ -1161,7 +1160,7 @@ static int proc_do_submiturb(struct dev_
 	kfree(isopkt);
 	as->ps = ps;
 	as->userurb = arg;
-	if (uurb->endpoint & USB_DIR_IN)
+	if (is_in && uurb->buffer_length > 0)
 		as->userbuffer = uurb->buffer;
 	else
 		as->userbuffer = NULL;
@@ -1171,9 +1170,9 @@ static int proc_do_submiturb(struct dev_
 	as->uid = cred->uid;
 	as->euid = cred->euid;
 	security_task_getsecid(current, &as->secid);
-	if (!is_in) {
+	if (!is_in && uurb->buffer_length > 0) {
 		if (copy_from_user(as->urb->transfer_buffer, uurb->buffer,
-				as->urb->transfer_buffer_length)) {
+				uurb->buffer_length)) {
 			free_async(as);
 			return -EFAULT;
 		}

--
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