Commit 749494b6 authored by Krzysztof Opasiak's avatar Krzysztof Opasiak Committed by Felipe Balbi

usb: gadget: f_hid: fix: Move IN request allocation to set_alt()

Since commit: ba1582f2 ("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: ba1582f2 ("usb: gadget: f_hid: use alloc_ep_req()")
Cc: stable@vger.kernel.org
Tested-by: default avatarDavid Lechner <david@lechnology.com>
Signed-off-by: default avatarKrzysztof Opasiak <k.opasiak@samsung.com>
Signed-off-by: default avatarFelipe Balbi <felipe.balbi@linux.intel.com>
parent 977ac789
...@@ -338,6 +338,7 @@ static ssize_t f_hidg_write(struct file *file, const char __user *buffer, ...@@ -338,6 +338,7 @@ static ssize_t f_hidg_write(struct file *file, const char __user *buffer,
size_t count, loff_t *offp) size_t count, loff_t *offp)
{ {
struct f_hidg *hidg = file->private_data; struct f_hidg *hidg = file->private_data;
struct usb_request *req;
unsigned long flags; unsigned long flags;
ssize_t status = -ENOMEM; ssize_t status = -ENOMEM;
...@@ -347,7 +348,7 @@ static ssize_t f_hidg_write(struct file *file, const char __user *buffer, ...@@ -347,7 +348,7 @@ static ssize_t f_hidg_write(struct file *file, const char __user *buffer,
spin_lock_irqsave(&hidg->write_spinlock, flags); spin_lock_irqsave(&hidg->write_spinlock, flags);
#define WRITE_COND (!hidg->write_pending) #define WRITE_COND (!hidg->write_pending)
try_again:
/* write queue */ /* write queue */
while (!WRITE_COND) { while (!WRITE_COND) {
spin_unlock_irqrestore(&hidg->write_spinlock, flags); spin_unlock_irqrestore(&hidg->write_spinlock, flags);
...@@ -362,6 +363,7 @@ static ssize_t f_hidg_write(struct file *file, const char __user *buffer, ...@@ -362,6 +363,7 @@ static ssize_t f_hidg_write(struct file *file, const char __user *buffer,
} }
hidg->write_pending = 1; hidg->write_pending = 1;
req = hidg->req;
count = min_t(unsigned, count, hidg->report_length); count = min_t(unsigned, count, hidg->report_length);
spin_unlock_irqrestore(&hidg->write_spinlock, flags); spin_unlock_irqrestore(&hidg->write_spinlock, flags);
...@@ -374,24 +376,38 @@ static ssize_t f_hidg_write(struct file *file, const char __user *buffer, ...@@ -374,24 +376,38 @@ static ssize_t f_hidg_write(struct file *file, const char __user *buffer,
goto release_write_pending; goto release_write_pending;
} }
hidg->req->status = 0; spin_lock_irqsave(&hidg->write_spinlock, flags);
hidg->req->zero = 0;
hidg->req->length = count; /* we our function has been disabled by host */
hidg->req->complete = f_hidg_req_complete; if (!hidg->req) {
hidg->req->context = hidg; 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); status = usb_ep_queue(hidg->in_ep, hidg->req, GFP_ATOMIC);
if (status < 0) { if (status < 0) {
ERROR(hidg->func.config->cdev, ERROR(hidg->func.config->cdev,
"usb_ep_queue error on int endpoint %zd\n", status); "usb_ep_queue error on int endpoint %zd\n", status);
goto release_write_pending; goto release_write_pending_unlocked;
} else { } else {
status = count; status = count;
} }
spin_unlock_irqrestore(&hidg->write_spinlock, flags);
return status; return status;
release_write_pending: release_write_pending:
spin_lock_irqsave(&hidg->write_spinlock, flags); spin_lock_irqsave(&hidg->write_spinlock, flags);
release_write_pending_unlocked:
hidg->write_pending = 0; hidg->write_pending = 0;
spin_unlock_irqrestore(&hidg->write_spinlock, flags); spin_unlock_irqrestore(&hidg->write_spinlock, flags);
...@@ -595,12 +611,23 @@ static void hidg_disable(struct usb_function *f) ...@@ -595,12 +611,23 @@ static void hidg_disable(struct usb_function *f)
kfree(list); kfree(list);
} }
spin_unlock_irqrestore(&hidg->read_spinlock, flags); 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) static int hidg_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
{ {
struct usb_composite_dev *cdev = f->config->cdev; struct usb_composite_dev *cdev = f->config->cdev;
struct f_hidg *hidg = func_to_hidg(f); struct f_hidg *hidg = func_to_hidg(f);
struct usb_request *req_in = NULL;
unsigned long flags;
int i, status = 0; int i, status = 0;
VDBG(cdev, "hidg_set_alt intf:%d alt:%d\n", intf, alt); VDBG(cdev, "hidg_set_alt intf:%d alt:%d\n", intf, alt);
...@@ -621,6 +648,12 @@ static int hidg_set_alt(struct usb_function *f, unsigned intf, unsigned alt) ...@@ -621,6 +648,12 @@ static int hidg_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
goto fail; goto fail;
} }
hidg->in_ep->driver_data = hidg; 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_function *f, unsigned intf, unsigned alt) ...@@ -632,12 +665,12 @@ static int hidg_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
hidg->out_ep); hidg->out_ep);
if (status) { if (status) {
ERROR(cdev, "config_ep_by_speed FAILED!\n"); ERROR(cdev, "config_ep_by_speed FAILED!\n");
goto fail; goto free_req_in;
} }
status = usb_ep_enable(hidg->out_ep); status = usb_ep_enable(hidg->out_ep);
if (status < 0) { if (status < 0) {
ERROR(cdev, "Enable OUT endpoint FAILED!\n"); ERROR(cdev, "Enable OUT endpoint FAILED!\n");
goto fail; goto free_req_in;
} }
hidg->out_ep->driver_data = hidg; hidg->out_ep->driver_data = hidg;
...@@ -653,17 +686,37 @@ static int hidg_set_alt(struct usb_function *f, unsigned intf, unsigned alt) ...@@ -653,17 +686,37 @@ static int hidg_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
req->context = hidg; req->context = hidg;
status = usb_ep_queue(hidg->out_ep, req, status = usb_ep_queue(hidg->out_ep, req,
GFP_ATOMIC); GFP_ATOMIC);
if (status) if (status) {
ERROR(cdev, "%s queue req --> %d\n", ERROR(cdev, "%s queue req --> %d\n",
hidg->out_ep->name, status); hidg->out_ep->name, status);
free_ep_req(hidg->out_ep, req);
}
} else { } else {
usb_ep_disable(hidg->out_ep);
status = -ENOMEM; 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: fail:
return status; return status;
} }
...@@ -712,12 +765,6 @@ static int hidg_bind(struct usb_configuration *c, struct usb_function *f) ...@@ -712,12 +765,6 @@ static int hidg_bind(struct usb_configuration *c, struct usb_function *f)
goto fail; goto fail;
hidg->out_ep = ep; 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 */ /* set descriptor dynamic values */
hidg_interface_desc.bInterfaceSubClass = hidg->bInterfaceSubClass; hidg_interface_desc.bInterfaceSubClass = hidg->bInterfaceSubClass;
hidg_interface_desc.bInterfaceProtocol = hidg->bInterfaceProtocol; hidg_interface_desc.bInterfaceProtocol = hidg->bInterfaceProtocol;
...@@ -755,6 +802,8 @@ static int hidg_bind(struct usb_configuration *c, struct usb_function *f) ...@@ -755,6 +802,8 @@ static int hidg_bind(struct usb_configuration *c, struct usb_function *f)
goto fail; goto fail;
spin_lock_init(&hidg->write_spinlock); spin_lock_init(&hidg->write_spinlock);
hidg->write_pending = 1;
hidg->req = NULL;
spin_lock_init(&hidg->read_spinlock); spin_lock_init(&hidg->read_spinlock);
init_waitqueue_head(&hidg->write_queue); init_waitqueue_head(&hidg->write_queue);
init_waitqueue_head(&hidg->read_queue); init_waitqueue_head(&hidg->read_queue);
...@@ -1019,10 +1068,6 @@ static void hidg_unbind(struct usb_configuration *c, struct usb_function *f) ...@@ -1019,10 +1068,6 @@ static void hidg_unbind(struct usb_configuration *c, struct usb_function *f)
device_destroy(hidg_class, MKDEV(major, hidg->minor)); device_destroy(hidg_class, MKDEV(major, hidg->minor));
cdev_del(&hidg->cdev); 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); usb_free_all_descriptors(f);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment