Hello,
I compiled and installed linux-4.6-rc3 on my beagle bone black and
noticed that when I unload a gadget using f_sourcesink (namely
g_zero), a kernel panic occurs:
[ 12.531504] Unable to handle kernel NULL pointer dereference at virtual address 00000005
[ 12.540100] pgd = de6a4000
[ 12.542984] [00000005] *pgd=9e702831, *pte=00000000, *ppte=00000000
[ 12.549713] Internal error: Oops: 17 [#1] SMP ARM
[ 12.554713] Modules linked in: usb_f_ss_lb g_zero(-) libcomposite musb_dsps musb_hdrc cppi41 udc_core usbcore omap_rng rng_core musb_am335x rtc_omap omap_wdt cpufreq_dt thermal_sys leds_gpio hwmon led_class
[ 12.574519] CPU: 0 PID: 139 Comm: modprobe Not tainted 4.6.0-rc3 #3
[ 12.581165] Hardware name: Generic AM33XX (Flattened Device Tree)
[ 12.587632] task: de65e400 ti: de6ce000 task.ti: de6ce000
[ 12.593391] PC is at source_sink_free_instance+0x24/0xfc [usb_f_ss_lb]
[ 12.600327] LR is at source_sink_complete+0x174/0x480 [usb_f_ss_lb]
[ 12.606980] pc : [<bf0d699c>] lr : [<bf0d6d2c>] psr: 80000093
[ 12.606980] sp : de6cfe40 ip : bf0a4074 fp : 00000000
[ 12.619141] r10: de685d80 r9 : dd00a000 r8 : 20000093
[ 12.624688] r7 : de685d80 r6 : dd123000 r5 : 00000000 r4 : dd1804e8
[ 12.631609] r3 : 00000000 r2 : bf0d2900 r1 : de5a4280 r0 : dd123000
[ 12.638536] Flags: Nzcv IRQs off FIQs on Mode SVC_32 ISA ARM Segment none
[ 12.646196] Control: 10c5387d Table: 9e6a4019 DAC: 00000051
[ 12.652292] Process modprobe (pid: 139, stack limit = 0xde6ce218)
[ 12.658758] Stack: (0xde6cfe40 to 0xde6d0000)
[ 12.663409] fe40: dd1804e8 dd1804e8 de5a4280 dd123000 de685d80 20000093 dd00a000 e0b72410
[ 12.672096] fe60: 00000000 bf0d6d2c 60000093 dd180010 dd180010 dd1804e8 00000001 de5a4280
[ 12.680781] fe80: dd180010 dd1804e8 00000001 bf09900c c0c0965c bf0ad7ec e0b72410 dd180010
[ 12.689467] fea0: dd1804e8 dd180538 dd180010 bf0b2c84 20000093 bf0adf28 dd1804e8 de685d80
[ 12.698152] fec0: dd180f0c dd1804e8 c0107984 de6ce000 00000000 bf0d68f8 dd102740 de6cff00
[ 12.706838] fee0: c0107984 de685d80 dd180fec bf0d7aa8 dd180f0c dd123000 de685d80 00000000
[ 12.715523] ff00: 00000081 bf0d7b00 dd180fec bf0c4bec bf0ca000 bf0c4c18 de685d80 de685ddc
[ 12.724209] ff20: 60000013 bf0c51d4 dd175800 dd1811b0 00000080 bf09974c bf0d2a10 dd175800
[ 12.732895] ff40: bf0d2a10 bf099814 bf0d247c bf0d2ac0 000af6d0 c01c5350 00000000 657a5f67
[ 12.741579] ff60: 00006f72 c0cc5d9c de65e400 00000000 de6ce000 00000000 00000000 c0155754
[ 12.750264] ff80: 00000000 de6ce000 b6f4348c 000af6b0 00000001 001078bc b6f4348c 000af6b0
[ 12.758949] ffa0: 00000001 c01077e0 b6f4348c 000af6b0 000af6d0 00000080 00000001 00000000
[ 12.767633] ffc0: b6f4348c 000af6b0 00000001 00000081 000af638 00000000 000af6b0 00000000
[ 12.776319] ffe0: b6eed3e8 beb1bb70 0001b160 b6eed3f4 a0000010 000af6d0 9fdf6861 9fdf6c61
[ 12.785029] [<bf0d699c>] (source_sink_free_instance [usb_f_ss_lb]) from [<bf0d6d2c>] (source_sink_complete+0x174/0x480 [usb_f_ss_lb])
[ 12.797779] [<bf0d6d2c>] (source_sink_complete [usb_f_ss_lb]) from [<bf09900c>] (usb_gadget_giveback_request+0xc/0x10 [udc_core])
[ 12.810188] [<bf09900c>] (usb_gadget_giveback_request [udc_core]) from [<bf0ad7ec>] (musb_g_giveback+0x118/0x614 [musb_hdrc])
[ 12.822218] [<bf0ad7ec>] (musb_g_giveback [musb_hdrc]) from [<bf0adf28>] (musb_gadget_disable+0x130/0x1d8 [musb_hdrc])
[ 12.833590] [<bf0adf28>] (musb_gadget_disable [musb_hdrc]) from [<bf0d68f8>] (sourcesink_get_alt+0x3c/0x78 [usb_f_ss_lb])
[ 12.845227] [<bf0d68f8>] (sourcesink_get_alt [usb_f_ss_lb]) from [<bf0d7aa8>] (disable_endpoints+0x24/0x84 [usb_f_ss_lb])
[ 12.856864] [<bf0d7aa8>] (disable_endpoints [usb_f_ss_lb]) from [<bf0d7b00>] (disable_endpoints+0x7c/0x84 [usb_f_ss_lb])
[ 12.868447] [<bf0d7b00>] (disable_endpoints [usb_f_ss_lb]) from [<bf0c4c18>] (config_ep_by_speed+0x28c/0x3ac [libcomposite])
[ 12.880379] [<bf0c4c18>] (config_ep_by_speed [libcomposite]) from [<bf0c51d4>] (composite_disconnect+0x2c/0x54 [libcomposite])
[ 12.892484] [<bf0c51d4>] (composite_disconnect [libcomposite]) from [<bf09974c>] (usb_gadget_state_work+0x90/0xf4 [udc_core])
[ 12.904488] [<bf09974c>] (usb_gadget_state_work [udc_core]) from [<bf099814>] (usb_gadget_unregister_driver+0x64/0xc4 [udc_core])
[ 12.916868] [<bf099814>] (usb_gadget_unregister_driver [udc_core]) from [<c01c5350>] (SyS_delete_module+0x11c/0x1e4)
[ 12.928048] [<c01c5350>] (SyS_delete_module) from [<c01077e0>] (ret_fast_syscall+0x0/0x1c)
[ 12.936829] Code: e5905080 e5933024 e3550002 e592a01c (e5d39005)
[ 12.943314] ---[ end trace 87c865532163a167 ]---
I traced the issue to f_sourcesink trying to use a struct usb_ep's
dest field after it's set to NULL by musb_gadget.c
This patch fixes this problem by moving the clearing of ep->desc to
occur after calling the complete() callbacks for all requests.
Tal Shorer (1):
usb: musb: gadget: nuke endpoint before setting its descriptor to NULL
drivers/usb/musb/musb_gadget.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
--
2.5.0
Some functions, such as f_sourcesink, rely on an endpoint's desc
field during their requests' complete() callback, so clear it only
_after_ nuking all requests to avoid NULL pointer dereference.
Signed-off-by: Tal Shorer <[email protected]>
---
drivers/usb/musb/musb_gadget.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index 87bd578..152865b 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -1164,12 +1164,12 @@ static int musb_gadget_disable(struct usb_ep *ep)
musb_writew(epio, MUSB_RXMAXP, 0);
}
- musb_ep->desc = NULL;
- musb_ep->end_point.desc = NULL;
-
/* abort all pending DMA and requests */
nuke(musb_ep, -ESHUTDOWN);
+ musb_ep->desc = NULL;
+ musb_ep->end_point.desc = NULL;
+
schedule_work(&musb->irq_work);
spin_unlock_irqrestore(&(musb->lock), flags);
--
2.5.0
Hi,
On Thu, Apr 14, 2016 at 07:33:43PM +0300, Tal Shorer wrote:
> Some functions, such as f_sourcesink, rely on an endpoint's desc
> field during their requests' complete() callback, so clear it only
> _after_ nuking all requests to avoid NULL pointer dereference.
>
> Signed-off-by: Tal Shorer <[email protected]>
Signed-off-by: Bin Liu <[email protected]>
Regards,
-Bin.
> ---
> drivers/usb/musb/musb_gadget.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
> index 87bd578..152865b 100644
> --- a/drivers/usb/musb/musb_gadget.c
> +++ b/drivers/usb/musb/musb_gadget.c
> @@ -1164,12 +1164,12 @@ static int musb_gadget_disable(struct usb_ep *ep)
> musb_writew(epio, MUSB_RXMAXP, 0);
> }
>
> - musb_ep->desc = NULL;
> - musb_ep->end_point.desc = NULL;
> -
> /* abort all pending DMA and requests */
> nuke(musb_ep, -ESHUTDOWN);
>
> + musb_ep->desc = NULL;
> + musb_ep->end_point.desc = NULL;
> +
> schedule_work(&musb->irq_work);
>
> spin_unlock_irqrestore(&(musb->lock), flags);
> --
> 2.5.0
>