This is a note to let you know that I've just added the patch titled usb: gadget: f_hid: Use spinlock instead of mutex to the 4.10-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary The filename of the patch is: usb-gadget-f_hid-use-spinlock-instead-of-mutex.patch and it can be found in the queue-4.10 subdirectory. If you, or anyone else, feels it should not be added to the stable tree, please let <stable@xxxxxxxxxxxxxxx> know about it. >From 33e4c1a9987a1fc3b42c3b534100b5b006d55c61 Mon Sep 17 00:00:00 2001 From: Krzysztof Opasiak <kopasiak90@xxxxxxxxx> Date: Thu, 19 Jan 2017 18:55:29 +0100 Subject: usb: gadget: f_hid: Use spinlock instead of mutex From: Krzysztof Opasiak <kopasiak90@xxxxxxxxx> commit 33e4c1a9987a1fc3b42c3b534100b5b006d55c61 upstream. As IN request has to be allocated in set_alt() and released in disable() we cannot use mutex to protect it as we cannot sleep in those funcitons. Let's replace this mutex with a spinlock. Tested-by: David Lechner <david@xxxxxxxxxxxxxx> Signed-off-by: Krzysztof Opasiak <k.opasiak@xxxxxxxxxxx> Signed-off-by: Felipe Balbi <felipe.balbi@xxxxxxxxxxxxxxx> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> --- drivers/usb/gadget/function/f_hid.c | 57 +++++++++++++++++++++--------------- 1 file changed, 34 insertions(+), 23 deletions(-) --- a/drivers/usb/gadget/function/f_hid.c +++ b/drivers/usb/gadget/function/f_hid.c @@ -50,12 +50,12 @@ struct f_hidg { /* recv report */ struct list_head completed_out_req; - spinlock_t spinlock; + spinlock_t read_spinlock; wait_queue_head_t read_queue; unsigned int qlen; /* send report */ - struct mutex lock; + spinlock_t write_spinlock; bool write_pending; wait_queue_head_t write_queue; struct usb_request *req; @@ -258,20 +258,20 @@ static ssize_t f_hidg_read(struct file * if (!access_ok(VERIFY_WRITE, buffer, count)) return -EFAULT; - spin_lock_irqsave(&hidg->spinlock, flags); + spin_lock_irqsave(&hidg->read_spinlock, flags); #define READ_COND (!list_empty(&hidg->completed_out_req)) /* wait for at least one buffer to complete */ while (!READ_COND) { - spin_unlock_irqrestore(&hidg->spinlock, flags); + spin_unlock_irqrestore(&hidg->read_spinlock, flags); if (file->f_flags & O_NONBLOCK) return -EAGAIN; if (wait_event_interruptible(hidg->read_queue, READ_COND)) return -ERESTARTSYS; - spin_lock_irqsave(&hidg->spinlock, flags); + spin_lock_irqsave(&hidg->read_spinlock, flags); } /* pick the first one */ @@ -286,7 +286,7 @@ static ssize_t f_hidg_read(struct file * req = list->req; count = min_t(unsigned int, count, req->actual - list->pos); - spin_unlock_irqrestore(&hidg->spinlock, flags); + spin_unlock_irqrestore(&hidg->read_spinlock, flags); /* copy to user outside spinlock */ count -= copy_to_user(buffer, req->buf + list->pos, count); @@ -308,9 +308,9 @@ static ssize_t f_hidg_read(struct file * return ret; } } else { - spin_lock_irqsave(&hidg->spinlock, flags); + spin_lock_irqsave(&hidg->read_spinlock, flags); list_add(&list->list, &hidg->completed_out_req); - spin_unlock_irqrestore(&hidg->spinlock, flags); + spin_unlock_irqrestore(&hidg->read_spinlock, flags); wake_up(&hidg->read_queue); } @@ -321,13 +321,16 @@ static ssize_t f_hidg_read(struct file * static void f_hidg_req_complete(struct usb_ep *ep, struct usb_request *req) { struct f_hidg *hidg = (struct f_hidg *)ep->driver_data; + unsigned long flags; if (req->status != 0) { ERROR(hidg->func.config->cdev, "End Point Request ERROR: %d\n", req->status); } + spin_lock_irqsave(&hidg->write_spinlock, flags); hidg->write_pending = 0; + spin_unlock_irqrestore(&hidg->write_spinlock, flags); wake_up(&hidg->write_queue); } @@ -335,18 +338,19 @@ static ssize_t f_hidg_write(struct file size_t count, loff_t *offp) { struct f_hidg *hidg = file->private_data; + unsigned long flags; ssize_t status = -ENOMEM; if (!access_ok(VERIFY_READ, buffer, count)) return -EFAULT; - mutex_lock(&hidg->lock); + spin_lock_irqsave(&hidg->write_spinlock, flags); #define WRITE_COND (!hidg->write_pending) /* write queue */ while (!WRITE_COND) { - mutex_unlock(&hidg->lock); + spin_unlock_irqrestore(&hidg->write_spinlock, flags); if (file->f_flags & O_NONBLOCK) return -EAGAIN; @@ -354,17 +358,20 @@ static ssize_t f_hidg_write(struct file hidg->write_queue, WRITE_COND)) return -ERESTARTSYS; - mutex_lock(&hidg->lock); + spin_lock_irqsave(&hidg->write_spinlock, flags); } + hidg->write_pending = 1; count = min_t(unsigned, count, hidg->report_length); + + spin_unlock_irqrestore(&hidg->write_spinlock, flags); status = copy_from_user(hidg->req->buf, buffer, count); if (status != 0) { ERROR(hidg->func.config->cdev, "copy_from_user error\n"); - mutex_unlock(&hidg->lock); - return -EINVAL; + status = -EINVAL; + goto release_write_pending; } hidg->req->status = 0; @@ -372,19 +379,23 @@ static ssize_t f_hidg_write(struct file hidg->req->length = count; hidg->req->complete = f_hidg_req_complete; hidg->req->context = hidg; - hidg->write_pending = 1; status = usb_ep_queue(hidg->in_ep, hidg->req, GFP_ATOMIC); if (status < 0) { ERROR(hidg->func.config->cdev, "usb_ep_queue error on int endpoint %zd\n", status); - hidg->write_pending = 0; - wake_up(&hidg->write_queue); + goto release_write_pending; } else { status = count; } - mutex_unlock(&hidg->lock); + return status; +release_write_pending: + spin_lock_irqsave(&hidg->write_spinlock, flags); + hidg->write_pending = 0; + spin_unlock_irqrestore(&hidg->write_spinlock, flags); + + wake_up(&hidg->write_queue); return status; } @@ -451,9 +462,9 @@ static void hidg_set_report_complete(str req_list->req = req; - spin_lock_irqsave(&hidg->spinlock, flags); + spin_lock_irqsave(&hidg->read_spinlock, flags); list_add_tail(&req_list->list, &hidg->completed_out_req); - spin_unlock_irqrestore(&hidg->spinlock, flags); + spin_unlock_irqrestore(&hidg->read_spinlock, flags); wake_up(&hidg->read_queue); break; @@ -577,13 +588,13 @@ static void hidg_disable(struct usb_func usb_ep_disable(hidg->in_ep); usb_ep_disable(hidg->out_ep); - spin_lock_irqsave(&hidg->spinlock, flags); + spin_lock_irqsave(&hidg->read_spinlock, flags); list_for_each_entry_safe(list, next, &hidg->completed_out_req, list) { free_ep_req(hidg->out_ep, list->req); list_del(&list->list); kfree(list); } - spin_unlock_irqrestore(&hidg->spinlock, flags); + spin_unlock_irqrestore(&hidg->read_spinlock, flags); } static int hidg_set_alt(struct usb_function *f, unsigned intf, unsigned alt) @@ -743,8 +754,8 @@ static int hidg_bind(struct usb_configur if (status) goto fail; - mutex_init(&hidg->lock); - spin_lock_init(&hidg->spinlock); + spin_lock_init(&hidg->write_spinlock); + spin_lock_init(&hidg->read_spinlock); init_waitqueue_head(&hidg->write_queue); init_waitqueue_head(&hidg->read_queue); INIT_LIST_HEAD(&hidg->completed_out_req); Patches currently in stable-queue which might be from kopasiak90@xxxxxxxxx are queue-4.10/usb-gadget-f_hid-fix-move-in-request-allocation-to-set_alt.patch queue-4.10/usb-gadget-f_hid-use-spinlock-instead-of-mutex.patch queue-4.10/usb-gadget-f_hid-fix-free-out-requests.patch queue-4.10/usb-gadget-f_hid-fix-prevent-accessing-released-memory.patch