Re: [PATCH v3] rtc: bd70528: enable the device's wakeup in the last step of .probe()

[Alexandre Belloni <alexandre.belloni@bootlin.com>]

On 12/12/2024 19:04:03+0900, Joe Hattori wrote:
> Current code leaves the device's wakeup enabled in the error path of
> .probe(), which results in a memory leak. Call device_init_wakeup() as
> the last step in the .probe() to avoid this leak.

Do you have more info on where the memory is allocated?

Coudln't we have a devm_ version of device_init_wakeup instead?

> 
> This bug was found by an experimental static analysis tool that I am
> developing.
> 
> Fixes: 32a4a4ebf768 ("rtc: bd70528: Initial support for ROHM bd70528 RTC")
> Signed-off-by: Joe Hattori <joe@pf.is.s.u-tokyo.ac.jp>
> ---
> Changes in V2:
> - Move the device_init_wakeup() to the last step of the .probe() to make
>   the cleanup simpler.
> ---
>  drivers/rtc/rtc-bd70528.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-bd70528.c b/drivers/rtc/rtc-bd70528.c
> index 954ac4ef53e8..d5cc4993f918 100644
> --- a/drivers/rtc/rtc-bd70528.c
> +++ b/drivers/rtc/rtc-bd70528.c
> @@ -312,9 +312,6 @@ static int bd70528_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> -	device_set_wakeup_capable(&pdev->dev, true);
> -	device_wakeup_enable(&pdev->dev);
> -
>  	rtc = devm_rtc_allocate_device(&pdev->dev);
>  	if (IS_ERR(rtc)) {
>  		dev_err(&pdev->dev, "RTC device creation failed\n");
> @@ -331,7 +328,12 @@ static int bd70528_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> -	return devm_rtc_register_device(rtc);
> +	ret = devm_rtc_register_device(rtc);
> +	if (ret)
> +		return ret;
> +
> +	device_init_wakeup(&pdev->dev, true);
> +	return 0;
>  }
>  
>  static const struct platform_device_id bd718x7_rtc_id[] = {
> -- 
> 2.34.1
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com