On 16/07/15 13:31, Roger Quadros wrote: > Peter, > > On 16/07/15 11:04, Peter Chen wrote: >> At some unexpected cases, the host may send the non-core control request >> before the configruation has been established, so the cdev->config is still >> NULL, then below NULL pointer dereference issue problem will occur. Although >> the udc driver can handle non-core control request beforhand, we still need >> composite core can handle some exceptions and without system crash. >> >> I meet this issue when I connect one board which supports USB OTG 2.0 >> (SRP & HNP), this board uses an internal bsp code, and another B-device >> uses the latest upstream mode which supports USB OTG not very well, so >> when the host sends the SET_FEATURE for USB_DEVICE_A_HNP_SUPPORT request >> (non-core control req00.03 v0004 i0000 l0), the udc driver does not handle >> it, and the composite driver takes it as a unknown request, it tries to >> get functions within configuration before checking configuration's valid. > > Why are we connecting to the host if we're not yet ready with the configuration? > Doesn't this indicate an error somewhere else and we just mask the > cause by this patch? Answering myself :). I understood now that this situation happens before host issues set_config() and that's why config->cdev is NULL. > >> >> root@imx6sxsabresd:~# modprobe g_mass_storage file=/dev/mmcblk0p1 removable=1 >> [ 41.994328] Number of LUNs=8 >> [ 41.997260] Mass Storage Function, version: 2009/09/11 >> [ 42.004301] LUN: removable file: (no medium) >> [ 42.012441] Number of LUNs=1 >> [ 42.016179] LUN: removable file: /dev/mmcblk0p1 >> [ 42.020855] Number of LUNs=1 >> [ 42.028315] g_mass_storage gadget: Mass Storage Gadget, version: 2009/09/11 >> [ 42.035395] g_mass_storage gadget: userspace failed to provide iSerialNumber >> [ 42.042559] g_mass_storage gadget: g_mass_storage ready >> root@imx6sxsabresd:~# >> root@imx6sxsabresd:~# [ 43.735411] Unable to handle kernel NULL pointer dereference at virtual address 00000028 >> [ 43.743523] pgd = 80004000 >> [ 43.746237] [00000028] *pgd=00000000 >> [ 43.749840] Internal error: Oops: 17 [#1] SMP ARM >> [ 43.754551] Modules linked in: g_mass_storage usb_f_mass_storage libcomposite configfs evbug >> [ 43.763096] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.2.0-rc1-00007-ga577f1b-dirty #358 >> [ 43.771278] Hardware name: Freescale i.MX6 SoloX (Device Tree) >> [ 43.777118] task: 80c9a9f8 ti: 80c94000 task.ti: 80c94000 >> [ 43.782558] PC is at composite_setup+0xe4/0x18d4 [libcomposite] >> [ 43.788484] LR is at 0x1 >> [ 43.791025] pc : [<7f0120e4>] lr : [<00000001>] psr: 600b0193 >> [ 43.791025] sp : 80c95d30 ip : 00000000 fp : 80c95d94 >> [ 43.802507] r10: 80c95dc8 r9 : 00000004 r8 : 00000000 >> [ 43.807738] r7 : 00000000 r6 : bd0c82d0 r5 : bd3fdb00 r4 : bd3fd080 >> [ 43.814269] r3 : 00000000 r2 : 00000000 r1 : 00000000 r0 : 00000003 >> [ 43.820803] Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment kernel >> [ 43.828205] Control: 10c5387d Table: bbc0c04a DAC: 00000015 >> [ 43.833958] Process swapper/0 (pid: 0, stack limit = 0x80c94210) >> [ 43.839970] Stack: (0x80c95d30 to 0x80c96000) >> [ 43.844336] 5d20: 808bc300 8007bf24 00000001 00000000 >> [ 43.852523] 5d40: 8056b280 80c95d50 00000000 600b0193 80c95d94 bd0c8014 bd0c8010 00000000 >> [ 43.860710] 5d60: 00082001 808bc5c8 600b0193 bd0c8014 bd0c8010 00080001 00082001 bd0c8568 >> [ 43.868896] 5d80: bd0c9010 c0876140 80c95dfc 80c95d98 8056c0ec 7f01200c bd0c8568 bd0c8014 >> [ 43.877083] 5da0: 00000001 00000000 bd0c9010 00000000 800787b8 bd0c8010 00000000 80075e2c >> [ 43.885270] 5dc0: 00000001 00000080 00040300 00000000 80c95dfc bd0c8010 0b242f20 bd0c9010 >> [ 43.893456] 5de0: 00000000 00000000 80d42428 80d4243c 80c95e1c 80c95e00 805684e4 8056b8ec >> [ 43.901643] 5e00: 8056847c bd064bc0 be1cf264 00000116 80c95e5c 80c95e20 800849ec 80568488 >> [ 43.909829] 5e20: be1cf264 bd064bc0 be1cf200 00000000 600b0193 be1cf200 be1cf264 bd064bc0 >> [ 43.918015] 5e40: 00000000 00000001 be01e000 808c0640 80c95e7c 80c95e60 80084bec 800849a4 >> [ 43.926202] 5e60: 00000000 be1cf200 be1cf264 80ca3798 80c95e9c 80c95e80 800881a0 80084ba8 >> [ 43.934389] 5e80: 800880c4 00000116 00000116 80c972d4 80c95eb4 80c95ea0 80083f4c 800880d0 >> [ 43.942576] 5ea0: 00000125 80c90654 80c95edc 80c95eb8 800842a4 80083f20 80c95f00 c080e10c >> [ 43.950762] 5ec0: 80c974bc c080e100 80c969c4 80c60278 80c95efc 80c95ee0 800095a8 8008423c >> [ 43.958949] 5ee0: 800115d4 200b0013 ffffffff 80c95f34 80c95f54 80c95f00 80015be4 80009584 >> [ 43.967135] 5f00: 00000001 00000001 00000000 80025fc0 80c94000 80c96a10 00000001 80d429c8 >> [ 43.975322] 5f20: 80c969c4 80c60278 808c0640 80c95f54 80c95f18 80c95f48 80075a14 800115d4 >> [ 43.983509] 5f40: 200b0013 ffffffff 80c95f64 80c95f58 8007010c 800115b0 80c95f84 80c95f68 >> [ 43.991696] 5f60: 80070264 800700e8 80c95f84 80c8e3e4 808b7854 80c96900 80c95fac 80c95f88 >> [ 43.999883] 5f80: 808ac968 80070128 00000000 00000000 808ac834 ffffffff 80d5c050 80d5c000 >> [ 44.008070] 5fa0: 80c95ff4 80c95fb0 80be5cd0 808ac840 ffffffff ffffffff 00000000 80be56ec >> [ 44.016255] 5fc0: 00000000 80c60278 00000000 80d5c294 80c969ac 80c60274 80c9c420 8000406a >> [ 44.024441] 5fe0: 412fc09a 00000000 00000000 80c95ff8 8000807c 80be596c 00000000 00000000 >> [ 44.032621] Backtrace: >> [ 44.035122] [<7f012000>] (composite_setup [libcomposite]) from [<8056c0ec>] (udc_irq+0x80c/0xe68) >> [ 44.044000] r10:c0876140 r9:bd0c9010 r8:bd0c8568 r7:00082001 r6:00080001 r5:bd0c8010 >> [ 44.051916] r4:bd0c8014 >> [ 44.054483] [<8056b8e0>] (udc_irq) from [<805684e4>] (ci_irq+0x68/0x160) >> [ 44.061189] r10:80d4243c r9:80d42428 r8:00000000 r7:00000000 r6:bd0c9010 r5:0b242f20 >> [ 44.069106] r4:bd0c8010 >> [ 44.071672] [<8056847c>] (ci_irq) from [<800849ec>] (handle_irq_event_percpu+0x54/0x204) >> [ 44.079765] r6:00000116 r5:be1cf264 r4:bd064bc0 r3:8056847c >> [ 44.085500] [<80084998>] (handle_irq_event_percpu) from [<80084bec>] (handle_irq_event+0x50/0x74) >> [ 44.094376] r10:808c0640 r9:be01e000 r8:00000001 r7:00000000 r6:bd064bc0 r5:be1cf264 >> [ 44.102291] r4:be1cf200 >> [ 44.104854] [<80084b9c>] (handle_irq_event) from [<800881a0>] (handle_fasteoi_irq+0xdc/0x1c4) >> [ 44.113383] r6:80ca3798 r5:be1cf264 r4:be1cf200 r3:00000000 >> [ 44.119117] [<800880c4>] (handle_fasteoi_irq) from [<80083f4c>] (generic_handle_irq+0x38/0x4c) >> [ 44.127732] r6:80c972d4 r5:00000116 r4:00000116 r3:800880c4 >> [ 44.133465] [<80083f14>] (generic_handle_irq) from [<800842a4>] (__handle_domain_irq+0x74/0xf0) >> [ 44.142167] r4:80c90654 r3:00000125 >> [ 44.145788] [<80084230>] (__handle_domain_irq) from [<800095a8>] (gic_handle_irq+0x30/0x70) >> [ 44.154142] r9:80c60278 r8:80c969c4 r7:c080e100 r6:80c974bc r5:c080e10c r4:80c95f00 >> [ 44.161977] [<80009578>] (gic_handle_irq) from [<80015be4>] (__irq_svc+0x44/0x5c) >> [ 44.169465] Exception stack(0x80c95f00 to 0x80c95f48) >> [ 44.174527] 5f00: 00000001 00000001 00000000 80025fc0 80c94000 80c96a10 00000001 80d429c8 >> [ 44.182714] 5f20: 80c969c4 80c60278 808c0640 80c95f54 80c95f18 80c95f48 80075a14 800115d4 >> [ 44.190895] 5f40: 200b0013 ffffffff >> [ 44.194388] r7:80c95f34 r6:ffffffff r5:200b0013 r4:800115d4 >> [ 44.200137] [<800115a4>] (arch_cpu_idle) from [<8007010c>] (default_idle_call+0x30/0x40) >> [ 44.208243] [<800700dc>] (default_idle_call) from [<80070264>] (cpu_startup_entry+0x148/0x270) >> [ 44.216869] [<8007011c>] (cpu_startup_entry) from [<808ac968>] (rest_init+0x134/0x170) >> [ 44.224790] r7:80c96900 >> [ 44.227354] [<808ac834>] (rest_init) from [<80be5cd0>] (start_kernel+0x370/0x3e8) >> [ 44.234842] r5:80d5c000 r4:80d5c050 >> [ 44.238458] [<80be5960>] (start_kernel) from [<8000807c>] (0x8000807c) >> [ 44.244995] Code: e3130001 1a000039 e594200c e1a03002 (e5b35028) >> [ 44.251100] ---[ end trace 48ab8610ac76d0a2 ]--- >> [ 44.255725] Kernel panic - not syncing: Fatal exception in interrupt >> [ 44.262092] ---[ end Kernel panic - not syncing: Fatal exception in interrupt >> >> Cc: <stable@xxxxxxxxxxxxxxx> #v3.14+ >> Cc: Jun Li <jun.li@xxxxxxxxxxxxx> >> Cc: Roger Quadros <rogerq@xxxxxx> >> Signed-off-by: Peter Chen <peter.chen@xxxxxxxxxxxxx> Acked-by: Roger Quadros <rogerq@xxxxxx> cheers, -roger >> --- >> drivers/usb/gadget/composite.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c >> index 4e3447b..dc836b3 100644 >> --- a/drivers/usb/gadget/composite.c >> +++ b/drivers/usb/gadget/composite.c >> @@ -1758,6 +1758,8 @@ unknown: >> * take such requests too, if that's ever needed: to work >> * in config 0, etc. >> */ >> + if (!cdev->config) >> + break; >> list_for_each_entry(f, &cdev->config->functions, list) >> if (f->req_match && f->req_match(f, ctrl)) >> goto try_fun_setup; >> > -- > 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 > -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html