usb: cdns3: host: Add null check for host_device before accessing its members

[PATCH] usb: cdns3: host: Add null check for host_device before accessing its members

Posted by Abhash Kumar Jha 2 weeks, 5 days ago

In cases when the xhci device is not populated, we see a nullptr
exception when resuming.

Add a nullptr check for the host_dev before accessing its private data.

Fixes: 3a85c1011540 ("usb: host: cdns3: forward lost power information to xhci")
Signed-off-by: Abhash Kumar Jha <a-kumar2@ti.com>
---
 drivers/usb/cdns3/host.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/cdns3/host.c b/drivers/usb/cdns3/host.c
index f0df114c2b53b..8cc7fc1de6193 100644
--- a/drivers/usb/cdns3/host.c
+++ b/drivers/usb/cdns3/host.c
@@ -141,9 +141,12 @@ static void cdns_host_exit(struct cdns *cdns)
 static int cdns_host_resume(struct cdns *cdns, bool power_lost)
 {
 	struct usb_hcd *hcd = platform_get_drvdata(cdns->host_dev);
-	struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
+	struct xhci_plat_priv *priv;
 
-	priv->power_lost = power_lost;
+	if (hcd) {
+		priv = hcd_to_xhci_priv(hcd);
+		priv->power_lost = power_lost;
+	}
 
 	return 0;
 }
-- 
2.34.1

Re: [PATCH] usb: cdns3: host: Add null check for host_device before accessing its members

Posted by Thomas Richard 1 week, 2 days ago

Hello,

On 1/19/26 9:22 AM, Abhash Kumar Jha wrote:
> In cases when the xhci device is not populated, we see a nullptr
> exception when resuming.
> 
> Add a nullptr check for the host_dev before accessing its private data.
> 

I was able to reproduce Abhash's issue on J784s4 and J7200 EVM board
with 6.19-rc7 kernel.

Unable to handle kernel NULL pointer dereference at virtual address
0000000000000208
Mem abort info:
  ESR = 0x0000000096000004
  EC = 0x25: DABT (current EL), IL = 32 bits
  SET = 0, FnV = 0
  EA = 0, S1PTW = 0
  FSC = 0x04: level 0 translation fault
Data abort info:
  ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
  CM = 0, WnR = 0, TnD = 0, TagAccess = 0
  GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
user pgtable: 4k pages, 48-bit VAs, pgdp=00000008856e6000
[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
sp : ffff800084fc39d0
x29: ffff800084fc39d0 x28: ffff8000840ac098 x27: ffff8000842e6ee0
x26: 0000000000000000 x25: ffff800084247958 x24: ffff80008427d000
x23: 000000048f2f5605 x22: ffff000801937218 x21: 0000000000000010
x20: 0000000000000001 x19: 0000000000000000 x18: 0000000000000006
x17: 0000000000000000 x16: 0000000000000000 x15: ffff800084fc3390
x14: 0000000000000000 x13: 383331203a74696e x12: 695f74736f685f73
x11: ffff800083c48828 x10: 0000000000000018 x9 : ffff800083c48828
x8 : 00000000000006aa x7 : ffff800083ca0828 x6 : 00000000fffff000
x5 : 0000000000017fe8 x4 : ffff000801935dc0 x3 : 0000000000000000
x2 : ffff800080e3b388 x1 : 0000000000000001 x0 : 0000000000000000
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 ]---

So I investigated and I identified two distinct issues.

1- First issue is hardware / board related issue, for an unknown reason
VBUS change after a suspend-to-ram or suspend-to-idle which triggers a
mode change of the controller (from idle to host). This issue shall be
fixed but it is not the right place for such a discussion.

2- Second issue is: switching from idle to host mode during resume
causes a NULL pointer dereference. I think it is a valid use case so it
should be fixed.

During resume cdns3 detects that the real role changed [1]. In this case
it switches to its new role [2]. Behind the scene, it runs the stop()
operation of the former role [3][4], and then run the start() operation
of the new role [5][6].

In host mode, the start() operation creates a xhci-hcd device [7]. But
as we are in the resume path, the probe() of this xhci-hcd device is not
run. Later in the resume sequence of cdns3 driver we call the resume()
operation [8] of the current role (which is host right now). But in the
host resume operation we assume the xhci-hcd device has been probed
which is not true in this case, so we try to use an uninitialized
pointer which causes a NULL pointer dereference.

I think we can skip the resume() operation in case the role changed
during resume as we already did the start() operation for the new role.
Then resume ends successfully, and xhci-hcd device is probed after.

I will send a patch.

[1]
https://elixir.bootlin.com/linux/v6.19-rc5/source/drivers/usb/cdns3/core.c#L534
[2]
https://elixir.bootlin.com/linux/v6.19-rc5/source/drivers/usb/cdns3/core.c#L536
[3]
https://elixir.bootlin.com/linux/v6.19-rc5/source/drivers/usb/cdns3/core.c#L316
[4]
https://elixir.bootlin.com/linux/v6.19-rc5/source/drivers/usb/cdns3/core.c#L66
[5]
https://elixir.bootlin.com/linux/v6.19-rc5/source/drivers/usb/cdns3/core.c#L320
[6]
https://elixir.bootlin.com/linux/v6.19-rc5/source/drivers/usb/cdns3/core.c#L47
[7]
https://elixir.bootlin.com/linux/v6.19-rc5/source/drivers/usb/cdns3/host.c#L69
[8]
https://elixir.bootlin.com/linux/v6.19-rc5/source/drivers/usb/cdns3/core.c#L555

Best Regards,
Thomas

Re: [PATCH] usb: cdns3: host: Add null check for host_device before accessing its members

Posted by Théo Lebrun 2 weeks, 5 days ago

Hello Abhash,

On Mon Jan 19, 2026 at 9:22 AM CET, Abhash Kumar Jha wrote:
> In cases when the xhci device is not populated, we see a nullptr
> exception when resuming.
>
> Add a nullptr check for the host_dev before accessing its private data.
>
> Fixes: 3a85c1011540 ("usb: host: cdns3: forward lost power information to xhci")
> Signed-off-by: Abhash Kumar Jha <a-kumar2@ti.com>
> ---
>  drivers/usb/cdns3/host.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/cdns3/host.c b/drivers/usb/cdns3/host.c
> index f0df114c2b53b..8cc7fc1de6193 100644
> --- a/drivers/usb/cdns3/host.c
> +++ b/drivers/usb/cdns3/host.c
> @@ -141,9 +141,12 @@ static void cdns_host_exit(struct cdns *cdns)
>  static int cdns_host_resume(struct cdns *cdns, bool power_lost)
>  {
>  	struct usb_hcd *hcd = platform_get_drvdata(cdns->host_dev);
> -	struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
> +	struct xhci_plat_priv *priv;
>  
> -	priv->power_lost = power_lost;
> +	if (hcd) {
> +		priv = hcd_to_xhci_priv(hcd);
> +		priv->power_lost = power_lost;
> +	}

I am not seeing a codepath making it possible for drvdata to be NULL.
Can you point it out? What HW, what config, etc.

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH] usb: cdns3: host: Add null check for host_device before accessing its members

Posted by Greg KH 2 weeks, 5 days ago

On Mon, Jan 19, 2026 at 01:52:42PM +0530, Abhash Kumar Jha wrote:
> In cases when the xhci device is not populated, we see a nullptr
> exception when resuming.

How can that happen?  What changed to cause this?

> Add a nullptr check for the host_dev before accessing its private data.
> 
> Fixes: 3a85c1011540 ("usb: host: cdns3: forward lost power information to xhci")

No cc: stable?

> Signed-off-by: Abhash Kumar Jha <a-kumar2@ti.com>
> ---
>  drivers/usb/cdns3/host.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/cdns3/host.c b/drivers/usb/cdns3/host.c
> index f0df114c2b53b..8cc7fc1de6193 100644
> --- a/drivers/usb/cdns3/host.c
> +++ b/drivers/usb/cdns3/host.c
> @@ -141,9 +141,12 @@ static void cdns_host_exit(struct cdns *cdns)
>  static int cdns_host_resume(struct cdns *cdns, bool power_lost)
>  {
>  	struct usb_hcd *hcd = platform_get_drvdata(cdns->host_dev);
> -	struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
> +	struct xhci_plat_priv *priv;
>  
> -	priv->power_lost = power_lost;
> +	if (hcd) {
> +		priv = hcd_to_xhci_priv(hcd);
> +		priv->power_lost = power_lost;
> +	}

Shouldn't you be returning an error if something went wrong?

And what protects hcd from changing right after you tested it?

thanks,

greg k-h