[PATCH v3] usb: raw-gadget: fix raw_event_queue_fetch locking

From: Andrey Konovalov
Date: Tue Apr 07 2020 - 10:48:04 EST


If queue->size check in raw_event_queue_fetch() fails (which normally
shouldn't happen, that check is a fail-safe), the function returns
without reenabling interrupts. This patch fixes that issue, along with
propagating the cause of failure to the function caller.

Fixes: f2c2e717642c ("usb: gadget: add raw-gadget interface"
Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
Signed-off-by: Andrey Konovalov <andreyknvl@xxxxxxxxxx>
---

Changes in v3:
- Don't return NULL from raw_event_queue_fetch() in case on an error.

Changes in v2:
- Added a comment before the WARN_ON() call.

Greg, this should apply cleanly on top of Dan's "usb: raw-gadget: Fix
copy_to/from_user() checks" patch.

---
drivers/usb/gadget/legacy/raw_gadget.c | 25 ++++++++++++++++++++-----
1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/gadget/legacy/raw_gadget.c b/drivers/usb/gadget/legacy/raw_gadget.c
index e490ffa1f58b..ca7d95bf7397 100644
--- a/drivers/usb/gadget/legacy/raw_gadget.c
+++ b/drivers/usb/gadget/legacy/raw_gadget.c
@@ -81,6 +81,7 @@ static int raw_event_queue_add(struct raw_event_queue *queue,
static struct usb_raw_event *raw_event_queue_fetch(
struct raw_event_queue *queue)
{
+ int ret;
unsigned long flags;
struct usb_raw_event *event;

@@ -89,11 +90,18 @@ static struct usb_raw_event *raw_event_queue_fetch(
* there's at least one event queued by decrementing the semaphore,
* and then take the lock to protect queue struct fields.
*/
- if (down_interruptible(&queue->sema))
- return NULL;
+ ret = down_interruptible(&queue->sema);
+ if (ret)
+ return ERR_PTR(ret);
spin_lock_irqsave(&queue->lock, flags);
- if (WARN_ON(!queue->size))
- return NULL;
+ /*
+ * queue->size must have the same value as queue->sema counter (before
+ * the down_interruptible() call above), so this check is a fail-safe.
+ */
+ if (WARN_ON(!queue->size)) {
+ spin_unlock_irqrestore(&queue->lock, flags);
+ return ERR_PTR(-ENODEV);
+ }
event = queue->events[0];
queue->size--;
memmove(&queue->events[0], &queue->events[1],
@@ -522,10 +530,17 @@ static int raw_ioctl_event_fetch(struct raw_dev *dev, unsigned long value)
spin_unlock_irqrestore(&dev->lock, flags);

event = raw_event_queue_fetch(&dev->queue);
- if (!event) {
+ if (PTR_ERR(event) == -EINTR) {
dev_dbg(&dev->gadget->dev, "event fetching interrupted\n");
return -EINTR;
}
+ if (IS_ERR(event)) {
+ dev_err(&dev->gadget->dev, "failed to fetch event\n");
+ spin_lock_irqsave(&dev->lock, flags);
+ dev->state = STATE_DEV_FAILED;
+ spin_unlock_irqrestore(&dev->lock, flags);
+ return -ENODEV;
+ }
length = min(arg.length, event->length);
if (copy_to_user((void __user *)value, event, sizeof(*event) + length))
return -EFAULT;
--
2.26.0.292.g33ef6b2f38-goog