usblp: Fix a double kfree

If submit fails, slab hits a BUG() because of a double kfree.
The today's lesson is, you cannot just slap USB_FREE_BUFFER on code
without adjusting the error paths.

The patch is made bigger by opportunistic refactoring.

Signed-Off-By: Pete Zaitcev <zaitcev@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

1 files changed, 23 insertions, 12 deletions

diff --git a/drivers/usb/class/usblp.c b/drivers/usb/class/usblp.c
index 3a0f8186d4b..ad632f2d6f9 100644
--- a/drivers/usb/class/usblp.c
+++ b/drivers/usb/class/usblp.c
@@ -686,10 +686,30 @@ done:
 	return retval;
 }
 
+static struct urb *usblp_new_writeurb(struct usblp *usblp, int transfer_length)
+{
+	struct urb *urb;
+	char *writebuf;
+
+	if ((writebuf = kmalloc(transfer_length, GFP_KERNEL)) == NULL)
+		return NULL;
+	if ((urb = usb_alloc_urb(0, GFP_KERNEL)) == NULL) {
+		kfree(writebuf);
+		return NULL;
+	}
+
+	usb_fill_bulk_urb(urb, usblp->dev,
+		usb_sndbulkpipe(usblp->dev,
+		 usblp->protocol[usblp->current_protocol].epwrite->bEndpointAddress),
+		writebuf, transfer_length, usblp_bulk_write, usblp);
+	urb->transfer_flags |= URB_FREE_BUFFER;
+
+	return urb;
+}
+
 static ssize_t usblp_write(struct file *file, const char __user *buffer, size_t count, loff_t *ppos)
 {
 	struct usblp *usblp = file->private_data;
-	char *writebuf;
 	struct urb *writeurb;
 	int rv;
 	int transfer_length;
@@ -710,18 +730,11 @@ static ssize_t usblp_write(struct file *file, const char __user *buffer, size_t
 			transfer_length = USBLP_BUF_SIZE;
 
 		rv = -ENOMEM;
-		if ((writebuf = kmalloc(USBLP_BUF_SIZE, GFP_KERNEL)) == NULL)
-			goto raise_buf;
-		if ((writeurb = usb_alloc_urb(0, GFP_KERNEL)) == NULL)
+		if ((writeurb = usblp_new_writeurb(usblp, transfer_length)) == NULL)
 			goto raise_urb;
-		usb_fill_bulk_urb(writeurb, usblp->dev,
-			usb_sndbulkpipe(usblp->dev,
-			  usblp->protocol[usblp->current_protocol].epwrite->bEndpointAddress),
-			writebuf, transfer_length, usblp_bulk_write, usblp);
-		writeurb->transfer_flags |= URB_FREE_BUFFER;
 		usb_anchor_urb(writeurb, &usblp->urbs);
 
-		if (copy_from_user(writebuf,
+		if (copy_from_user(writeurb->transfer_buffer,
 				   buffer + writecount, transfer_length)) {
 			rv = -EFAULT;
 			goto raise_badaddr;
@@ -780,8 +793,6 @@ raise_badaddr:
 	usb_unanchor_urb(writeurb);
 	usb_free_urb(writeurb);
 raise_urb:
-	kfree(writebuf);
-raise_buf:
 raise_wait:
 collect_error:		/* Out of raise sequence */
 	mutex_unlock(&usblp->wmut);