[PATCH 4.10 120/167] usb: gadget: f_hid: fix: Move IN request allocation to set_alt()
From: Greg Kroah-Hartman
Date: Fri Mar 10 2017 - 04:55:09 EST
4.10-stable review patch. If anyone has any objections, please let me know.
------------------
From: Krzysztof Opasiak <kopasiak90@xxxxxxxxx>
commit 749494b6bdbbaf0899aa1c62a1ad74cd747bce47 upstream.
Since commit: ba1582f22231 ("usb: gadget: f_hid: use alloc_ep_req()")
we cannot allocate any requests in bind() as we check if we should
align request buffer based on endpoint descriptor which is assigned
in set_alt().
Allocating request in bind() function causes a NULL pointer
dereference.
This commit moves allocation of IN request from bind() to set_alt()
to prevent this issue.
Fixes: ba1582f22231 ("usb: gadget: f_hid: use alloc_ep_req()")
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 | 89 +++++++++++++++++++++++++++---------
1 file changed, 67 insertions(+), 22 deletions(-)
--- a/drivers/usb/gadget/function/f_hid.c
+++ b/drivers/usb/gadget/function/f_hid.c
@@ -338,6 +338,7 @@ static ssize_t f_hidg_write(struct file
size_t count, loff_t *offp)
{
struct f_hidg *hidg = file->private_data;
+ struct usb_request *req;
unsigned long flags;
ssize_t status = -ENOMEM;
@@ -347,7 +348,7 @@ static ssize_t f_hidg_write(struct file
spin_lock_irqsave(&hidg->write_spinlock, flags);
#define WRITE_COND (!hidg->write_pending)
-
+try_again:
/* write queue */
while (!WRITE_COND) {
spin_unlock_irqrestore(&hidg->write_spinlock, flags);
@@ -362,6 +363,7 @@ static ssize_t f_hidg_write(struct file
}
hidg->write_pending = 1;
+ req = hidg->req;
count = min_t(unsigned, count, hidg->report_length);
spin_unlock_irqrestore(&hidg->write_spinlock, flags);
@@ -374,24 +376,38 @@ static ssize_t f_hidg_write(struct file
goto release_write_pending;
}
- hidg->req->status = 0;
- hidg->req->zero = 0;
- hidg->req->length = count;
- hidg->req->complete = f_hidg_req_complete;
- hidg->req->context = hidg;
+ spin_lock_irqsave(&hidg->write_spinlock, flags);
+
+ /* we our function has been disabled by host */
+ if (!hidg->req) {
+ free_ep_req(hidg->in_ep, hidg->req);
+ /*
+ * TODO
+ * Should we fail with error here?
+ */
+ goto try_again;
+ }
+
+ req->status = 0;
+ req->zero = 0;
+ req->length = count;
+ req->complete = f_hidg_req_complete;
+ req->context = hidg;
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);
- goto release_write_pending;
+ goto release_write_pending_unlocked;
} else {
status = count;
}
+ spin_unlock_irqrestore(&hidg->write_spinlock, flags);
return status;
release_write_pending:
spin_lock_irqsave(&hidg->write_spinlock, flags);
+release_write_pending_unlocked:
hidg->write_pending = 0;
spin_unlock_irqrestore(&hidg->write_spinlock, flags);
@@ -595,12 +611,23 @@ static void hidg_disable(struct usb_func
kfree(list);
}
spin_unlock_irqrestore(&hidg->read_spinlock, flags);
+
+ spin_lock_irqsave(&hidg->write_spinlock, flags);
+ if (!hidg->write_pending) {
+ free_ep_req(hidg->in_ep, hidg->req);
+ hidg->write_pending = 1;
+ }
+
+ hidg->req = NULL;
+ spin_unlock_irqrestore(&hidg->write_spinlock, flags);
}
static int hidg_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
{
struct usb_composite_dev *cdev = f->config->cdev;
struct f_hidg *hidg = func_to_hidg(f);
+ struct usb_request *req_in = NULL;
+ unsigned long flags;
int i, status = 0;
VDBG(cdev, "hidg_set_alt intf:%d alt:%d\n", intf, alt);
@@ -621,6 +648,12 @@ static int hidg_set_alt(struct usb_funct
goto fail;
}
hidg->in_ep->driver_data = hidg;
+
+ req_in = hidg_alloc_ep_req(hidg->in_ep, hidg->report_length);
+ if (!req_in) {
+ status = -ENOMEM;
+ goto disable_ep_in;
+ }
}
@@ -632,12 +665,12 @@ static int hidg_set_alt(struct usb_funct
hidg->out_ep);
if (status) {
ERROR(cdev, "config_ep_by_speed FAILED!\n");
- goto fail;
+ goto free_req_in;
}
status = usb_ep_enable(hidg->out_ep);
if (status < 0) {
ERROR(cdev, "Enable OUT endpoint FAILED!\n");
- goto fail;
+ goto free_req_in;
}
hidg->out_ep->driver_data = hidg;
@@ -653,17 +686,37 @@ static int hidg_set_alt(struct usb_funct
req->context = hidg;
status = usb_ep_queue(hidg->out_ep, req,
GFP_ATOMIC);
- if (status)
+ if (status) {
ERROR(cdev, "%s queue req --> %d\n",
hidg->out_ep->name, status);
+ free_ep_req(hidg->out_ep, req);
+ }
} else {
- usb_ep_disable(hidg->out_ep);
status = -ENOMEM;
- goto fail;
+ goto disable_out_ep;
}
}
}
+ if (hidg->in_ep != NULL) {
+ spin_lock_irqsave(&hidg->write_spinlock, flags);
+ hidg->req = req_in;
+ hidg->write_pending = 0;
+ spin_unlock_irqrestore(&hidg->write_spinlock, flags);
+
+ wake_up(&hidg->write_queue);
+ }
+ return 0;
+disable_out_ep:
+ usb_ep_disable(hidg->out_ep);
+free_req_in:
+ if (req_in)
+ free_ep_req(hidg->in_ep, req_in);
+
+disable_ep_in:
+ if (hidg->in_ep)
+ usb_ep_disable(hidg->in_ep);
+
fail:
return status;
}
@@ -712,12 +765,6 @@ static int hidg_bind(struct usb_configur
goto fail;
hidg->out_ep = ep;
- /* preallocate request and buffer */
- status = -ENOMEM;
- hidg->req = alloc_ep_req(hidg->in_ep, hidg->report_length);
- if (!hidg->req)
- goto fail;
-
/* set descriptor dynamic values */
hidg_interface_desc.bInterfaceSubClass = hidg->bInterfaceSubClass;
hidg_interface_desc.bInterfaceProtocol = hidg->bInterfaceProtocol;
@@ -755,6 +802,8 @@ static int hidg_bind(struct usb_configur
goto fail;
spin_lock_init(&hidg->write_spinlock);
+ hidg->write_pending = 1;
+ hidg->req = NULL;
spin_lock_init(&hidg->read_spinlock);
init_waitqueue_head(&hidg->write_queue);
init_waitqueue_head(&hidg->read_queue);
@@ -1019,10 +1068,6 @@ static void hidg_unbind(struct usb_confi
device_destroy(hidg_class, MKDEV(major, hidg->minor));
cdev_del(&hidg->cdev);
- /* disable/free request and end point */
- usb_ep_disable(hidg->in_ep);
- free_ep_req(hidg->in_ep, hidg->req);
-
usb_free_all_descriptors(f);
}