Skip to content

Commit 749494b

Browse files
kopasiakFelipe Balbi
kopasiak
authored and
Felipe Balbi
committed
usb: gadget: f_hid: fix: Move IN request allocation to set_alt()
Since commit: ba1582f ("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: ba1582f ("usb: gadget: f_hid: use alloc_ep_req()") Cc: stable@vger.kernel.org Tested-by: David Lechner <david@lechnology.com> Signed-off-by: Krzysztof Opasiak <k.opasiak@samsung.com> Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
1 parent 977ac78 commit 749494b

File tree

1 file changed

+67
-22
lines changed

1 file changed

+67
-22
lines changed

‎drivers/usb/gadget/function/f_hid.c

Lines changed: 67 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,7 @@ static ssize_t f_hidg_write(struct file *file, const char __user *buffer,
338338
size_t count, loff_t *offp)
339339
{
340340
struct f_hidg *hidg = file->private_data;
341+
struct usb_request *req;
341342
unsigned long flags;
342343
ssize_t status = -ENOMEM;
343344

@@ -347,7 +348,7 @@ static ssize_t f_hidg_write(struct file *file, const char __user *buffer,
347348
spin_lock_irqsave(&hidg->write_spinlock, flags);
348349

349350
#define WRITE_COND (!hidg->write_pending)
350-
351+
try_again:
351352
/* write queue */
352353
while (!WRITE_COND) {
353354
spin_unlock_irqrestore(&hidg->write_spinlock, flags);
@@ -362,6 +363,7 @@ static ssize_t f_hidg_write(struct file *file, const char __user *buffer,
362363
}
363364

364365
hidg->write_pending = 1;
366+
req = hidg->req;
365367
count = min_t(unsigned, count, hidg->report_length);
366368

367369
spin_unlock_irqrestore(&hidg->write_spinlock, flags);
@@ -374,24 +376,38 @@ static ssize_t f_hidg_write(struct file *file, const char __user *buffer,
374376
goto release_write_pending;
375377
}
376378

377-
hidg->req->status = 0;
378-
hidg->req->zero = 0;
379-
hidg->req->length = count;
380-
hidg->req->complete = f_hidg_req_complete;
381-
hidg->req->context = hidg;
379+
spin_lock_irqsave(&hidg->write_spinlock, flags);
380+
381+
/* we our function has been disabled by host */
382+
if (!hidg->req) {
383+
free_ep_req(hidg->in_ep, hidg->req);
384+
/*
385+
* TODO
386+
* Should we fail with error here?
387+
*/
388+
goto try_again;
389+
}
390+
391+
req->status = 0;
392+
req->zero = 0;
393+
req->length = count;
394+
req->complete = f_hidg_req_complete;
395+
req->context = hidg;
382396

383397
status = usb_ep_queue(hidg->in_ep, hidg->req, GFP_ATOMIC);
384398
if (status < 0) {
385399
ERROR(hidg->func.config->cdev,
386400
"usb_ep_queue error on int endpoint %zd\n", status);
387-
goto release_write_pending;
401+
goto release_write_pending_unlocked;
388402
} else {
389403
status = count;
390404
}
405+
spin_unlock_irqrestore(&hidg->write_spinlock, flags);
391406

392407
return status;
393408
release_write_pending:
394409
spin_lock_irqsave(&hidg->write_spinlock, flags);
410+
release_write_pending_unlocked:
395411
hidg->write_pending = 0;
396412
spin_unlock_irqrestore(&hidg->write_spinlock, flags);
397413

@@ -595,12 +611,23 @@ static void hidg_disable(struct usb_function *f)
595611
kfree(list);
596612
}
597613
spin_unlock_irqrestore(&hidg->read_spinlock, flags);
614+
615+
spin_lock_irqsave(&hidg->write_spinlock, flags);
616+
if (!hidg->write_pending) {
617+
free_ep_req(hidg->in_ep, hidg->req);
618+
hidg->write_pending = 1;
619+
}
620+
621+
hidg->req = NULL;
622+
spin_unlock_irqrestore(&hidg->write_spinlock, flags);
598623
}
599624

600625
static int hidg_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
601626
{
602627
struct usb_composite_dev *cdev = f->config->cdev;
603628
struct f_hidg *hidg = func_to_hidg(f);
629+
struct usb_request *req_in = NULL;
630+
unsigned long flags;
604631
int i, status = 0;
605632

606633
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)
621648
goto fail;
622649
}
623650
hidg->in_ep->driver_data = hidg;
651+
652+
req_in = hidg_alloc_ep_req(hidg->in_ep, hidg->report_length);
653+
if (!req_in) {
654+
status = -ENOMEM;
655+
goto disable_ep_in;
656+
}
624657
}
625658

626659

@@ -632,12 +665,12 @@ static int hidg_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
632665
hidg->out_ep);
633666
if (status) {
634667
ERROR(cdev, "config_ep_by_speed FAILED!\n");
635-
goto fail;
668+
goto free_req_in;
636669
}
637670
status = usb_ep_enable(hidg->out_ep);
638671
if (status < 0) {
639672
ERROR(cdev, "Enable OUT endpoint FAILED!\n");
640-
goto fail;
673+
goto free_req_in;
641674
}
642675
hidg->out_ep->driver_data = hidg;
643676

@@ -653,17 +686,37 @@ static int hidg_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
653686
req->context = hidg;
654687
status = usb_ep_queue(hidg->out_ep, req,
655688
GFP_ATOMIC);
656-
if (status)
689+
if (status) {
657690
ERROR(cdev, "%s queue req --> %d\n",
658691
hidg->out_ep->name, status);
692+
free_ep_req(hidg->out_ep, req);
693+
}
659694
} else {
660-
usb_ep_disable(hidg->out_ep);
661695
status = -ENOMEM;
662-
goto fail;
696+
goto disable_out_ep;
663697
}
664698
}
665699
}
666700

701+
if (hidg->in_ep != NULL) {
702+
spin_lock_irqsave(&hidg->write_spinlock, flags);
703+
hidg->req = req_in;
704+
hidg->write_pending = 0;
705+
spin_unlock_irqrestore(&hidg->write_spinlock, flags);
706+
707+
wake_up(&hidg->write_queue);
708+
}
709+
return 0;
710+
disable_out_ep:
711+
usb_ep_disable(hidg->out_ep);
712+
free_req_in:
713+
if (req_in)
714+
free_ep_req(hidg->in_ep, req_in);
715+
716+
disable_ep_in:
717+
if (hidg->in_ep)
718+
usb_ep_disable(hidg->in_ep);
719+
667720
fail:
668721
return status;
669722
}
@@ -712,12 +765,6 @@ static int hidg_bind(struct usb_configuration *c, struct usb_function *f)
712765
goto fail;
713766
hidg->out_ep = ep;
714767

715-
/* preallocate request and buffer */
716-
status = -ENOMEM;
717-
hidg->req = alloc_ep_req(hidg->in_ep, hidg->report_length);
718-
if (!hidg->req)
719-
goto fail;
720-
721768
/* set descriptor dynamic values */
722769
hidg_interface_desc.bInterfaceSubClass = hidg->bInterfaceSubClass;
723770
hidg_interface_desc.bInterfaceProtocol = hidg->bInterfaceProtocol;
@@ -755,6 +802,8 @@ static int hidg_bind(struct usb_configuration *c, struct usb_function *f)
755802
goto fail;
756803

757804
spin_lock_init(&hidg->write_spinlock);
805+
hidg->write_pending = 1;
806+
hidg->req = NULL;
758807
spin_lock_init(&hidg->read_spinlock);
759808
init_waitqueue_head(&hidg->write_queue);
760809
init_waitqueue_head(&hidg->read_queue);
@@ -1019,10 +1068,6 @@ static void hidg_unbind(struct usb_configuration *c, struct usb_function *f)
10191068
device_destroy(hidg_class, MKDEV(major, hidg->minor));
10201069
cdev_del(&hidg->cdev);
10211070

1022-
/* disable/free request and end point */
1023-
usb_ep_disable(hidg->in_ep);
1024-
free_ep_req(hidg->in_ep, hidg->req);
1025-
10261071
usb_free_all_descriptors(f);
10271072
}
10281073

0 commit comments

Comments
 (0)