mirror of
https://github.com/Divested-Mobile/DivestOS-Build.git
synced 2024-10-01 01:35:54 -04:00
187 lines
6.1 KiB
Diff
187 lines
6.1 KiB
Diff
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: [PATCH] USB: gadgetfs: Fix crash caused by inadequate synchronization
|
|
|
|
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>
|
|
CC: <stable@vger.kernel.org>
|
|
Acked-by: Felipe Balbi <felipe.balbi@linux.intel.com>
|
|
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
---
|
|
drivers/usb/gadget/legacy/inode.c | 41 ++++++++++++++++++++++++++++++++++-----
|
|
1 file changed, 36 insertions(+), 5 deletions(-)
|
|
|
|
diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c
|
|
index 956b3dc7c3a4d..5c28bee327e15 100644
|
|
--- 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_ep *ep, struct usb_request *req)
|
|
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 *buf, size_t len, loff_t *ptr)
|
|
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;
|
|
|
|
@@ -1134,6 +1137,7 @@ ep0_write (struct file *fd, const char __user *buf, size_t len, loff_t *ptr)
|
|
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;
|
|
@@ -1145,6 +1149,7 @@ ep0_write (struct file *fd, const char __user *buf, size_t len, loff_t *ptr)
|
|
GFP_KERNEL);
|
|
}
|
|
spin_lock_irq(&dev->lock);
|
|
+ --dev->udc_usage;
|
|
if (retval < 0) {
|
|
clean_req (dev->gadget->ep0, dev->req);
|
|
} else
|
|
@@ -1246,9 +1251,21 @@ static long dev_ioctl (struct file *fd, unsigned code, unsigned long value)
|
|
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;
|
|
}
|
|
|
|
@@ -1466,10 +1483,12 @@ gadgetfs_setup (struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
|
|
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;
|
|
@@ -1493,8 +1512,12 @@ gadgetfs_setup (struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
|
|
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;
|
|
@@ -1521,21 +1544,24 @@ static void destroy_ep_files (struct dev_data *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);
|
|
@@ -1606,6 +1632,11 @@ gadgetfs_unbind (struct usb_gadget *gadget)
|
|
|
|
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);
|