[PATCH v2] usb: dwc3: qcom: check for hcd in dwc3_qcom_read_usb2_speed

[PATCH v2] usb: dwc3: qcom: check for hcd in dwc3_qcom_read_usb2_speed

[Prashanth K]

Currently dwc->xhci is not cleared if xhci_plat_probe() fails. During
system suspend, dwc3_qcom_read_usb2_speed() checks dwc->xhci and assumes
xhci is initialised. Since the probe failed, hcd will be null, and the
subsequent access results in a NULL pointer dereference

Unable to handle kernel NULL pointer dereference at
virtual address 0000000000000058

Call trace:
  dwc3_qcom_pm_suspend
  dpm_run_callback
  device_suspend
  dpm_suspend
  dpm_suspend_start
  suspend_devices_and_enter
  pm_suspend

Explicitly check the hcd pointer in dwc3_qcom_read_usb2_speed
before accessing it.

Fixes: 6895ea55c385 ("usb: dwc3: qcom: Configure wakeup interrupts during suspend")
Signed-off-by: Prashanth K <prashanth.k@oss.qualcomm.com>
---
Changes in v2:
 - Added null pointer check in dwc3-qcom.c instead of marking xhci as null in host.c
 - Link to v1: https://lore.kernel.org/all/20261014105711.1708244-1-prashanth.k@oss.qualcomm.com

 drivers/usb/dwc3/dwc3-qcom.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index ac68b4218b56..6ca51d37c35a 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -241,7 +241,8 @@ static enum usb_device_speed dwc3_qcom_read_usb2_speed(struct dwc3_qcom *qcom, i
 	hcd = platform_get_drvdata(dwc->xhci);
 
 #ifdef CONFIG_USB
-	udev = usb_hub_find_child(hcd->self.root_hub, port_index + 1);
+	if (hcd)
+		udev = usb_hub_find_child(hcd->self.root_hub, port_index + 1);
 #else
 	udev = NULL;
 #endif
-- 
2.34.1

Re: [PATCH v2] usb: dwc3: qcom: check for hcd in dwc3_qcom_read_usb2_speed

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] usb: dwc3: qcom: uninitialized udev pointer in dwc3_qcom_read_usb2_speed()
--

commit 9364e775b526fa295084d32e8e9579d5847360c6
Author: Prashanth K <prashanth.k@oss.qualcomm.com>

usb: dwc3: qcom: check for hcd in dwc3_qcom_read_usb2_speed

This commit checks whether hcd is non-NULL in dwc3_qcom_read_usb2_speed()
before dereferencing it to avoid a NULL pointer dereference during system
suspend if xhci_plat_probe() failed.

> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index ac68b4218b56b..6ca51d37c35ac 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> @@ -241,7 +241,8 @@ static enum usb_device_speed dwc3_qcom_read_usb2_speed(struct dwc3_qcom *qcom, i
>  	hcd = platform_get_drvdata(dwc->xhci);
>  
>  #ifdef CONFIG_USB
> -	udev = usb_hub_find_child(hcd->self.root_hub, port_index + 1);
> +	if (hcd)
> +		udev = usb_hub_find_child(hcd->self.root_hub, port_index + 1);
>  #else
>  	udev = NULL;
>  #endif

[Severity: High]
In drivers/usb/dwc3/dwc3-qcom.c, dwc3_qcom_read_usb2_speed() declares udev on
the stack without initializing it:

drivers/usb/dwc3/dwc3-qcom.c:dwc3_qcom_read_usb2_speed() {
	struct usb_device *udev;
	struct usb_hcd __maybe_unused *hcd;
	struct dwc3 *dwc = &qcom->dwc;

	hcd = platform_get_drvdata(dwc->xhci);

#ifdef CONFIG_USB
	if (hcd)
		udev = usb_hub_find_child(hcd->self.root_hub, port_index + 1);
#else
	udev = NULL;
#endif
	if (!udev)
		return USB_SPEED_UNKNOWN;

	return udev->speed;
}

When CONFIG_USB is enabled and platform_get_drvdata(dwc->xhci) returns NULL
during dwc3_qcom_pm_suspend() -> dwc3_qcom_read_usb2_speed() (for example,
when xhci_plat_probe() failed), won't udev remain uninitialized and cause
if (!udev) and return udev->speed to read and dereference an uninitialized
pointer?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261022044748.2303528-1-prashanth.k@oss.qualcomm.com?part=1