From 7bb29174b77c3a521ad1ecfedbe67c7775144016 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Mon, 9 Oct 2017 11:59:16 +0200 Subject: [PATCH] 4.9-stable patches added patches: usb-gadgetfs-fix-crash-caused-by-inadequate-synchronization.patch --- queue-4.9/series | 1 + ...caused-by-inadequate-synchronization.patch | 189 ++++++++++++++++++ 2 files changed, 190 insertions(+) create mode 100644 queue-4.9/usb-gadgetfs-fix-crash-caused-by-inadequate-synchronization.patch diff --git a/queue-4.9/series b/queue-4.9/series index e69de29bb2d..6233714d867 100644 --- a/queue-4.9/series +++ b/queue-4.9/series @@ -0,0 +1 @@ +usb-gadgetfs-fix-crash-caused-by-inadequate-synchronization.patch diff --git a/queue-4.9/usb-gadgetfs-fix-crash-caused-by-inadequate-synchronization.patch b/queue-4.9/usb-gadgetfs-fix-crash-caused-by-inadequate-synchronization.patch new file mode 100644 index 00000000000..8a2e11d4b90 --- /dev/null +++ b/queue-4.9/usb-gadgetfs-fix-crash-caused-by-inadequate-synchronization.patch @@ -0,0 +1,189 @@ +From 520b72fc64debf8a86c3853b8e486aa5982188f0 Mon Sep 17 00:00:00 2001 +From: Alan Stern +Date: Thu, 21 Sep 2017 13:23:58 -0400 +Subject: USB: gadgetfs: Fix crash caused by inadequate synchronization + +From: Alan Stern + +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 +Reported-by: Andrey Konovalov +Tested-by: Andrey Konovalov +Acked-by: Felipe Balbi +Signed-off-by: Greg Kroah-Hartman +Signed-off-by: Greg Kroah-Hartman + +--- + 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 +@@ -27,7 +27,7 @@ + #include + #include + #include +- ++#include + #include + #include + +@@ -116,6 +116,7 @@ enum ep0_state { + struct dev_data { + spinlock_t lock; + atomic_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); -- 2.47.3