usb: raw-gadget: fix raw_event_queue_fetch locking

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@oracle.com>
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
Signed-off-by: Felipe Balbi <balbi@kernel.org>

diff --git a/drivers/usb/gadget/legacy/raw_gadget.c b/drivers/usb/gadget/legacy/raw_gadget.c
index 7640634..21d3fee 100644 (file)
--- 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)
 {
 static struct usb_raw_event *raw_event_queue_fetch(
                                struct raw_event_queue *queue)
 {

+       int ret;

        unsigned long flags;
        struct usb_raw_event *event;
 
        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.
         */
         * 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);
        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],
        event = queue->events[0];
        queue->size--;
        memmove(&queue->events[0], &queue->events[1],


@@ -525,10 +533,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);
        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;
        }
                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);
        ret = copy_to_user((void __user *)value, event,
                                sizeof(*event) + length);
        length = min(arg.length, event->length);
        ret = copy_to_user((void __user *)value, event,
                                sizeof(*event) + length);