Hi All, I've just noticed this patch has been applied to linux-next. It breaks DWC2 driver initialization on Samsung Exynos-based boards I've use for kernel daily tests. Here is an example of the call stack: Unable to handle kernel NULL pointer dereference at virtual address 0000000c pgd = (ptrval) [0000000c] *pgd=00000000 Internal error: Oops: 5 [#1] PREEMPT SMP ARM Modules linked in: CPU: 0 PID: 25 Comm: kworker/0:1 Not tainted 4.17.0-rc5-next-20180517 #782 Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) Workqueue: events deferred_probe_work_func PC is at usb_ep_alloc_request+0x1c/0x200 LR is at composite_dev_prepare+0x20/0xec pc : [<c06976a8>] lr : [<c0694744>] psr: 60000153 sp : d95f1d10 ip : 00000000 fp : 00000002 r10: c1072d58 r9 : d82443d0 r8 : 00000000 r7 : c1074180 r6 : c10ad460 r5 : c100746c r4 : d8253980 r3 : 00000000 r2 : d82539b8 r1 : 014000c0 r0 : d82443d0 Flags: nZCv IRQs on FIQs off Mode SVC_32 ISA ARM Segment none Control: 10c5387d Table: 4000406a DAC: 00000051 Process kworker/0:1 (pid: 25, stack limit = 0x(ptrval)) Stack: (0xd95f1d10 to 0xd95f2000) ... [<c06976a8>] (usb_ep_alloc_request) from [<c0694744>] (composite_dev_prepare+0x20/0xec) [<c0694744>] (composite_dev_prepare) from [<c0694b4c>] (composite_bind+0x7c/0x1c8) [<c0694b4c>] (composite_bind) from [<c069a8e0>] (udc_bind_to_driver+0x74/0x134) [<c069a8e0>] (udc_bind_to_driver) from [<c069aa14>] (check_pending_gadget_drivers+0x74/0xac) [<c069aa14>] (check_pending_gadget_drivers) from [<c069abdc>] (usb_add_gadget_udc_release+0x190/0x1ec) [<c069abdc>] (usb_add_gadget_udc_release) from [<c0655a84>] (dwc2_gadget_init+0x274/0x464) [<c0655a84>] (dwc2_gadget_init) from [<c0642fb0>] (dwc2_driver_probe+0x488/0x50c) [<c0642fb0>] (dwc2_driver_probe) from [<c057a1e4>] (platform_drv_probe+0x48/0x9c) [<c057a1e4>] (platform_drv_probe) from [<c0577d54>] (driver_probe_device+0x2dc/0x4ac) [<c0577d54>] (driver_probe_device) from [<c0575d54>] (bus_for_each_drv+0x74/0xb8) [<c0575d54>] (bus_for_each_drv) from [<c057792c>] (__device_attach+0xd4/0x168) [<c057792c>] (__device_attach) from [<c0576bf8>] (bus_probe_device+0x88/0x90) [<c0576bf8>] (bus_probe_device) from [<c05771b0>] (deferred_probe_work_func+0x60/0x180) [<c05771b0>] (deferred_probe_work_func) from ad+0x28c/0x58c) [<c0147f2c>] (worker_thread) from [<c014dc5c>] (kthread+0x138/0x168) [<c014dc5c>] (kthread) from [<c01010b4>] (ret_from_fork+0x14/0x20) A proper fix for the potential memory leak is to add free to error path instead of reordering usb_add_gadget_udc() call. The issue happens if DWC2 driver is initialized from deferred probe (in such case the gadget driver is already registered). Felipe: please drop or revert this patch in your -next branch. Best regards Marek Szyprowski, PhD Samsung R&D Institute Poland On 2018-04-16 12:16, Grigor Tovmasyan wrote: > In dwc2_gadget_init() we allocate EP0 request via > dwc2_hsotg_ep_alloc_request(). After that there are > usb_add_gadget_udc() call and if it failed, then > ctrl_req will not be freed and will cause memory leak. > > Reordered function calls in gadget_init: moved up usb_add_gadget_udc() > before dwc2_hsotg_ep_alloc_request(). > > Tested using kmemleak. > > Cc: Stefan Wahren <stefan.wahren@xxxxxxxx> > Signed-off-by: Grigor Tovmasyan <tovmasya@xxxxxxxxxxxx> > Acked-by: Minas Harutyunyan <hminas@xxxxxxxxxxxx> > --- > drivers/usb/dwc2/gadget.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c > index 6c32bf26e48e..24000bda5c20 100644 > --- a/drivers/usb/dwc2/gadget.c > +++ b/drivers/usb/dwc2/gadget.c > @@ -4679,6 +4679,10 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg) > INIT_LIST_HEAD(&hsotg->gadget.ep_list); > hsotg->gadget.ep0 = &hsotg->eps_out[0]->ep; > > + ret = usb_add_gadget_udc(dev, &hsotg->gadget); > + if (ret) > + return ret; > + > /* allocate EP0 request */ > > hsotg->ctrl_req = dwc2_hsotg_ep_alloc_request(&hsotg->eps_out[0]->ep, > @@ -4698,10 +4702,6 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg) > epnum, 0); > } > > - ret = usb_add_gadget_udc(dev, &hsotg->gadget); > - if (ret) > - return ret; > - > dwc2_hsotg_dump(hsotg); > > return 0; > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
