--- /dev/null
+From 520b72fc64debf8a86c3853b8e486aa5982188f0 Mon Sep 17 00:00:00 2001
+From: Alan Stern <stern@rowland.harvard.edu>
+Date: Thu, 21 Sep 2017 13:23:58 -0400
+Subject: USB: gadgetfs: Fix crash caused by inadequate synchronization
+
+From: Alan Stern <stern@rowland.harvard.edu>
+
+commit 520b72fc64debf8a86c3853b8e486aa5982188f0 upstream.
+
+The gadgetfs driver (drivers/usb/gadget/legacy/inode.c) was written
+before the UDC and composite frameworks were adopted; it is a legacy
+driver. As such, it expects that once bound to a UDC controller, it
+will not be unbound until it unregisters itself.
+
+However, the UDC framework does unbind function drivers while they are
+still registered. When this happens, it can cause the gadgetfs driver
+to misbehave or crash. For example, userspace can cause a crash by
+opening the device file and doing an ioctl call before setting up a
+configuration (found by Andrey Konovalov using the syzkaller fuzzer).
+
+This patch adds checks and synchronization to prevent these bad
+behaviors. It adds a udc_usage counter that the driver increments at
+times when it is using a gadget interface without holding the private
+spinlock. The unbind routine waits for this counter to go to 0 before
+returning, thereby ensuring that the UDC is no longer in use.
+
+The patch also adds a check in the dev_ioctl() routine to make sure
+the driver is bound to a UDC before dereferencing the gadget pointer,
+and it makes destroy_ep_files() synchronize with the endpoint I/O
+routines, to prevent the user from accessing an endpoint data
+structure after it has been removed.
+
+Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
+Reported-by: Andrey Konovalov <andreyknvl@google.com>
+Tested-by: Andrey Konovalov <andreyknvl@google.com>
+Acked-by: Felipe Balbi <felipe.balbi@linux.intel.com>
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+
+---
+ drivers/usb/gadget/legacy/inode.c | 41 +++++++++++++++++++++++++++++++++-----
+ 1 file changed, 36 insertions(+), 5 deletions(-)
+
+--- a/drivers/usb/gadget/legacy/inode.c
++++ b/drivers/usb/gadget/legacy/inode.c
+@@ -28,7 +28,7 @@
+ #include <linux/aio.h>
+ #include <linux/uio.h>
+ #include <linux/refcount.h>
+-
++#include <linux/delay.h>
+ #include <linux/device.h>
+ #include <linux/moduleparam.h>
+
+@@ -116,6 +116,7 @@ enum ep0_state {
+ struct dev_data {
+ spinlock_t lock;
+ refcount_t count;
++ int udc_usage;
+ enum ep0_state state; /* P: lock */
+ struct usb_gadgetfs_event event [N_EVENT];
+ unsigned ev_next;
+@@ -513,9 +514,9 @@ static void ep_aio_complete(struct usb_e
+ INIT_WORK(&priv->work, ep_user_copy_worker);
+ schedule_work(&priv->work);
+ }
+- spin_unlock(&epdata->dev->lock);
+
+ usb_ep_free_request(ep, req);
++ spin_unlock(&epdata->dev->lock);
+ put_ep(epdata);
+ }
+
+@@ -939,9 +940,11 @@ ep0_read (struct file *fd, char __user *
+ struct usb_request *req = dev->req;
+
+ if ((retval = setup_req (ep, req, 0)) == 0) {
++ ++dev->udc_usage;
+ spin_unlock_irq (&dev->lock);
+ retval = usb_ep_queue (ep, req, GFP_KERNEL);
+ spin_lock_irq (&dev->lock);
++ --dev->udc_usage;
+ }
+ dev->state = STATE_DEV_CONNECTED;
+
+@@ -1131,6 +1134,7 @@ ep0_write (struct file *fd, const char _
+ retval = setup_req (dev->gadget->ep0, dev->req, len);
+ if (retval == 0) {
+ dev->state = STATE_DEV_CONNECTED;
++ ++dev->udc_usage;
+ spin_unlock_irq (&dev->lock);
+ if (copy_from_user (dev->req->buf, buf, len))
+ retval = -EFAULT;
+@@ -1142,6 +1146,7 @@ ep0_write (struct file *fd, const char _
+ GFP_KERNEL);
+ }
+ spin_lock_irq(&dev->lock);
++ --dev->udc_usage;
+ if (retval < 0) {
+ clean_req (dev->gadget->ep0, dev->req);
+ } else
+@@ -1243,9 +1248,21 @@ static long dev_ioctl (struct file *fd,
+ struct usb_gadget *gadget = dev->gadget;
+ long ret = -ENOTTY;
+
+- if (gadget->ops->ioctl)
++ spin_lock_irq(&dev->lock);
++ if (dev->state == STATE_DEV_OPENED ||
++ dev->state == STATE_DEV_UNBOUND) {
++ /* Not bound to a UDC */
++ } else if (gadget->ops->ioctl) {
++ ++dev->udc_usage;
++ spin_unlock_irq(&dev->lock);
++
+ ret = gadget->ops->ioctl (gadget, code, value);
+
++ spin_lock_irq(&dev->lock);
++ --dev->udc_usage;
++ }
++ spin_unlock_irq(&dev->lock);
++
+ return ret;
+ }
+
+@@ -1463,10 +1480,12 @@ delegate:
+ if (value < 0)
+ break;
+
++ ++dev->udc_usage;
+ spin_unlock (&dev->lock);
+ value = usb_ep_queue (gadget->ep0, dev->req,
+ GFP_KERNEL);
+ spin_lock (&dev->lock);
++ --dev->udc_usage;
+ if (value < 0) {
+ clean_req (gadget->ep0, dev->req);
+ break;
+@@ -1490,8 +1509,12 @@ delegate:
+ req->length = value;
+ req->zero = value < w_length;
+
++ ++dev->udc_usage;
+ spin_unlock (&dev->lock);
+ value = usb_ep_queue (gadget->ep0, req, GFP_KERNEL);
++ spin_lock(&dev->lock);
++ --dev->udc_usage;
++ spin_unlock(&dev->lock);
+ if (value < 0) {
+ DBG (dev, "ep_queue --> %d\n", value);
+ req->status = 0;
+@@ -1518,21 +1541,24 @@ static void destroy_ep_files (struct dev
+ /* break link to FS */
+ ep = list_first_entry (&dev->epfiles, struct ep_data, epfiles);
+ list_del_init (&ep->epfiles);
++ spin_unlock_irq (&dev->lock);
++
+ dentry = ep->dentry;
+ ep->dentry = NULL;
+ parent = d_inode(dentry->d_parent);
+
+ /* break link to controller */
++ mutex_lock(&ep->lock);
+ if (ep->state == STATE_EP_ENABLED)
+ (void) usb_ep_disable (ep->ep);
+ ep->state = STATE_EP_UNBOUND;
+ usb_ep_free_request (ep->ep, ep->req);
+ ep->ep = NULL;
++ mutex_unlock(&ep->lock);
++
+ wake_up (&ep->wait);
+ put_ep (ep);
+
+- spin_unlock_irq (&dev->lock);
+-
+ /* break link to dcache */
+ inode_lock(parent);
+ d_delete (dentry);
+@@ -1603,6 +1629,11 @@ gadgetfs_unbind (struct usb_gadget *gadg
+
+ spin_lock_irq (&dev->lock);
+ dev->state = STATE_DEV_UNBOUND;
++ while (dev->udc_usage > 0) {
++ spin_unlock_irq(&dev->lock);
++ usleep_range(1000, 2000);
++ spin_lock_irq(&dev->lock);
++ }
+ spin_unlock_irq (&dev->lock);
+
+ destroy_ep_files (dev);
projects / thirdparty / kernel / stable-queue.git / commitdiff
