Re: [PATCH] usb: cdns3: fix role switching during resume

[Peter Chen (CIX)]

On 26-01-30 11:05:45, Thomas Richard (TI) wrote:
> If the role change while we are suspended, the cdns3 driver switches to the
> new mode during resume. However, switching to host mode in this context
> causes a NULL pointer dereference.
> 
> The host role's start() operation registers a xhci-hcd device, but its
> probe is deferred while we are in the resume path. The host role's resume()
> operation assumes the xhci-hcd device is already probed, which is not the
> case, leading to the dereference. Since the start() operation of the new
> role is already called, the resume operation can be skipped.
> 
> So skip the resume operation for the new role if a role switch occurs
> during resume. Once the resume sequence is complete, the xhci-hcd device
> can be probed in case of host mode.
> 
> Unable to handle kernel NULL pointer dereference at virtual address 0000000000000208
> Mem abort info:
> ...
> Data abort info:
> ...
> [0000000000000208] pgd=0000000000000000, p4d=0000000000000000
> Internal error: Oops: 0000000096000004 [#1]  SMP
> Modules linked in:
> CPU: 0 UID: 0 PID: 146 Comm: sh Not tainted
> 6.19.0-rc7-00013-g6e64f4aabfae-dirty #135 PREEMPT
> Hardware name: Texas Instruments J7200 EVM (DT)
> pstate: 20000005 (nzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : usb_hcd_is_primary_hcd+0x0/0x1c
> lr : cdns_host_resume+0x24/0x5c
> ...
> Call trace:
>  usb_hcd_is_primary_hcd+0x0/0x1c (P)
>  cdns_resume+0x6c/0xbc
>  cdns3_controller_resume.isra.0+0xe8/0x17c
>  cdns3_plat_resume+0x18/0x24
>  platform_pm_resume+0x2c/0x68
>  dpm_run_callback+0x90/0x248
>  device_resume+0x100/0x24c
>  dpm_resume+0x190/0x2ec
>  dpm_resume_end+0x18/0x34
>  suspend_devices_and_enter+0x2b0/0xa44
>  pm_suspend+0x16c/0x5fc
>  state_store+0x80/0xec
>  kobj_attr_store+0x18/0x2c
>  sysfs_kf_write+0x7c/0x94
>  kernfs_fop_write_iter+0x130/0x1dc
>  vfs_write+0x240/0x370
>  ksys_write+0x70/0x108
>  __arm64_sys_write+0x1c/0x28
>  invoke_syscall+0x48/0x10c
>  el0_svc_common.constprop.0+0x40/0xe0
>  do_el0_svc+0x1c/0x28
>  el0_svc+0x34/0x108
>  el0t_64_sync_handler+0xa0/0xe4
>  el0t_64_sync+0x198/0x19c
> Code: 52800003 f9407ca5 d63f00a0 17ffffe4 (f9410401)
> ---[ end trace 0000000000000000 ]---
> 
> Cc: stable@xxxxxxxxxxxxxxx
> Fixes: 2cf2581cd229 ("usb: cdns3: add power lost support for system resume")
> Signed-off-by: Thomas Richard (TI) <thomas.richard@xxxxxxxxxxx>

Acked-by: Peter Chen <peter.chen@xxxxxxxxxx>

Peter
> ---
> This patch is related to the following discussion:
> https://lore.kernel.org/all/8743fec1-301d-46e1-89bf-7952c73faa86@xxxxxxxxxxx/
> ---
>  drivers/usb/cdns3/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
> index 1243a5cea91b..f0e32227c0b7 100644
> --- a/drivers/usb/cdns3/core.c
> +++ b/drivers/usb/cdns3/core.c
> @@ -551,7 +551,7 @@ int cdns_resume(struct cdns *cdns)
>  		}
>  	}
>  
> -	if (cdns->roles[cdns->role]->resume)
> +	if (!role_changed && cdns->roles[cdns->role]->resume)
>  		cdns->roles[cdns->role]->resume(cdns, power_lost);
>  
>  	return 0;
> 

-- 

Best regards,
Peter