Re: [PATCH v3 6/6] usb: dwc2: platform: fix stm32mp15 detection logic power management

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] usb: dwc2: NULL deref in dwc2_stm32_hw_disable() on
  dwc2_init_params() error
--

commit f918c81ccd698e45e8b3143adcead2cdea2d4018
Author: Fabrice Gasnier <fabrice.gasnier@foss.st.com>

usb: dwc2: platform: fix stm32mp15 detection logic power management

This commit moves STM32MP15 ID/VBUS detection logic enable and disable
handling into __dwc2_lowlevel_hw_enable() and __dwc2_lowlevel_hw_disable()
so the detection resources follow the low-level hardware power state.

> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
> index b65b9898bac0d..d61b088cbc265 100644
> --- a/drivers/usb/dwc2/platform.c
> +++ b/drivers/usb/dwc2/platform.c

[ ... ]

> @@ -541,33 +639,26 @@ static int dwc2_driver_probe(struct platform_device *dev)
>  		goto error;
>  
>  	if (hsotg->params.activate_stm_id_vb_detection) {
> -		u32 ggpio;
> -
>  		hsotg->usb33d = devm_regulator_get(hsotg->dev, "usb33d");
>  		if (IS_ERR(hsotg->usb33d)) {
>  			retval = PTR_ERR(hsotg->usb33d);
> +			/* skips low level dwc2_stm32_hw_disable() in case of error */
> +			hsotg->params.activate_stm_id_vb_detection = 0;
>  			dev_err_probe(hsotg->dev, retval, "failed to request usb33d supply\n");
>  			goto error;
>  		}
> -		retval = regulator_enable(hsotg->usb33d);
> +		retval = dwc2_stm32_hw_enable(hsotg);
>  		if (retval) {
> -			dev_err_probe(hsotg->dev, retval, "failed to enable usb33d supply\n");
> +			hsotg->params.activate_stm_id_vb_detection = 0;
> +			dev_err_probe(hsotg->dev, retval, "dwc2_stm32_hw_enable failed\n");
>  			goto error;
>  		}

[Severity: Medium]
Can a failure in dwc2_init_params() right before this block cause a NULL
pointer dereference on hsotg->usb33d in regulator_disable()?

In dwc2_init_params(), set_params(hsotg) sets
hsotg->params.activate_stm_id_vb_detection = true on STM32MP15 before
calling dwc2_limit_speed(hsotg):

drivers/usb/dwc2/params.c:dwc2_init_params() {
    ...
	if (set_params) {
		set_params(hsotg);
	} else {
    ...
	}

	ret = dwc2_limit_speed(hsotg);
	if (ret)
		return ret;
    ...
}

If dwc2_limit_speed() fails, dwc2_driver_probe() jumps to error before
devm_regulator_get() runs, and dwc2_lowlevel_hw_disable() ->
dwc2_stm32_hw_disable() passes hsotg->usb33d == NULL to
regulator_disable().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106-dwc2-low-power-fixes-stm32-v3-0-68d2a0e9bab6@foss.st.com?part=6