Re: [PATCH] arm64: dts: qcom: glymur: Add missing USB clock, power and bandwidth votes

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Early return in `dwc3_qcom_remove()` on `pm_runtime_resume_and_get()` failure bypasses device teardown, leaking interconnect paths and causing a Use-After-Free.
--

--- Patch [1]: [PATCH] arm64: dts: qcom: glymur: Add missing USB clock, power and bandwidth votes ---
commit e8a88eddb13f0c6c7f86603fb6358155107b782d
Author: Greg Ociepka <[email protected]>

arm64: dts: qcom: glymur: Add missing USB clock, power and bandwidth votes
    
This patch adds missing assigned-clocks, required-opps, and interconnects
properties to the four USB3 controller nodes on Glymur. These changes ensure
the controllers properly vote for performance state, configure the master
clock, and request USB-DDR bandwidth.

> diff --git a/arch/arm64/boot/dts/qcom/glymur.dtsi b/arch/arm64/boot/dts/qcom/glymur.dtsi
> index f04058d2e0891..fbbc82550c442 100644
> --- a/arch/arm64/boot/dts/qcom/glymur.dtsi
> +++ b/arch/arm64/boot/dts/qcom/glymur.dtsi
> @@ -4584,9 +4588,18 @@ usb_0: usb@a600000 {
>  					  "ss_phy_irq";
>  
>  			power-domains = <&gcc GCC_USB30_PRIM_GDSC>;
> +			required-opps = <&rpmhpd_opp_nom>;
>  			resets = <&gcc GCC_USB30_PRIM_BCR>;
>  
>  			iommus = <&apps_smmu 0x1420 0x0>;
> +
> +			interconnects = <&aggre4_noc MASTER_USB3_0 QCOM_ICC_TAG_ALWAYS
> +					 &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
> +					<&hsc_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ACTIVE_ONLY
> +					 &config_noc SLAVE_USB3_0 QCOM_ICC_TAG_ACTIVE_ONLY>;

[Severity: High]
This isn't a bug introduced by this patch, but adding these interconnect 
properties exposes a resource leak and use-after-free in the driver's remove 
path during unbind.

In drivers/usb/dwc3/dwc3-qcom.c:dwc3_qcom_remove(), if
pm_runtime_resume_and_get() fails, the function returns early:

	if (pm_runtime_resume_and_get(qcom->dev) < 0)
		return;

	dwc3_core_remove(&qcom->dwc);
	clk_bulk_disable_unprepare(qcom->num_clocks, qcom->clks);
	dwc3_qcom_interconnect_exit(qcom);

This early return skips dwc3_core_remove() and dwc3_qcom_interconnect_exit().

Because the qcom structure is allocated with devm_kzalloc(), it is freed 
immediately after remove() completes, leaving the active dwc3 core operating 
on freed memory. It also leaks the newly populated interconnect paths that 
this patch enables.

Could the driver's error handling in dwc3_qcom_remove() be updated to ensure 
cleanup happens even if runtime resume fails?

> +			interconnect-names = "usb-ddr",
> +					     "apps-usb";
> +

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1