Re: [PATCH v1 2/2] usb: dwc3: Add Renesas R-Car Gen5 DWC3 xHCI USB controller glue

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] The driver leaks the runtime PM usage counter on probe failure and module unbind, permanently preventing the device from suspending.
- [High] The driver acquires an exclusive reset control but never deasserts it, potentially leaving the hardware in a permanent reset state.
- [High] Use-after-free risk due to accessing a device tree property string after dropping its node reference.
- [Low] The dev_pm_ops structure is unconditionally assigned without using the pm_ptr() wrapper, violating power management subsystem guidelines.
--

--- Patch [2]: [PATCH v1 2/2] usb: dwc3: Add Renesas R-Car Gen5 DWC3 xHCI USB controller glue ---
commit fc962bfb4e19df62045a5d6f590433718732e755
Author: Thanh Quan <thanh.quan.xn@renesas.com>

usb: dwc3: Add Renesas R-Car Gen5 DWC3 xHCI USB controller glue

This commit adds a new driver for the glue logic surrounding the DWC3
USB controller core on the Renesas R-Car Gen5 SoC.

> diff --git a/drivers/usb/dwc3/dwc3-rcar-gen5.c b/drivers/usb/dwc3/dwc3-rcar-gen5.c
> new file mode 100644
> index 0000000000000..344d0851559f5
> --- /dev/null
> +++ b/drivers/usb/dwc3/dwc3-rcar-gen5.c

[ ... ]

> +static int dwc3_rcar_gen5_probe(struct platform_device *pdev)
> +{

[ ... ]

> +	reset = devm_reset_control_get(dev, NULL);
> +	if (IS_ERR(reset))
> +		return dev_err_probe(dev, PTR_ERR(reset), "Failed to get reset control\n");

[Severity: High]
Does this leave the hardware in a permanent reset state? We acquire the
reset control here, but it doesn't look like we ever call
reset_control_deassert() on it later in the probe function.

[ ... ]

> +	ret = of_property_read_string(subnode, "maximum-speed", &maximum_speed);
> +	of_node_put(subnode);
> +	if (ret)
> +		return dev_err_probe(dev, -ENODEV, "Failed to determine maximum speed\n");
> +
> +	priv->use_usb3_flow = !strcmp(maximum_speed, "super-speed-plus") ||
> +			      !strcmp(maximum_speed, "super-speed");

[Severity: High]
Can this cause a use-after-free read?

The property string pointer maximum_speed points directly into the device
tree node's memory. By calling of_node_put() before the strcmp()
evaluations, the node memory could theoretically be freed if an overlay
is concurrently removed.

Should the of_node_put() happen after we're completely done with
maximum_speed?

> +	ret = devm_pm_runtime_enable(dev);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to enable runtime PM\n");
> +
> +	ret = pm_runtime_resume_and_get(dev);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to resume runtime PM\n");
> +
> +	return devm_of_platform_populate(dev);
> +}

[Severity: High]
Does this leak the runtime PM usage counter if devm_of_platform_populate()
fails? It looks like we return its error code directly without calling
pm_runtime_put().

[ ... ]

> +static struct platform_driver dwc3_rcar_gen5_driver = {
> +	.probe		= dwc3_rcar_gen5_probe,
> +	.driver		= {
> +		.name	= "renesas-rcar-gen5-usb",
> +		.of_match_table = dwc3_rcar_gen5_of_match,

[Severity: High]
Does this driver need a .remove callback?

Without one, the PM usage counter incremented during probe via
pm_runtime_resume_and_get() is never dropped when the module is unbound,
permanently preventing the device from suspending.

> +		.pm	= &dwc3_rcar_gen5_pm_ops,

[Severity: Low]
Should this use the pm_ptr() wrapper? Assigning the dev_pm_ops structure
unconditionally leaves dead code in the kernel when CONFIG_PM is disabled.

> +	},
> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728011656.465040-1-marek.vasut+renesas@mailbox.org?part=2