Re: [PATCH] platform/x86/amd/pmc: Fix RTC reference leak in amd_pmc_verify_czn_rtc()

[Mario Limonciello]

On 9/14/26 07:20, Ma Ke wrote:
> amd_pmc_verify_czn_rtc() opens the RTC with rtc_class_open(), which
> takes both a device reference and a module reference on the RTC
> driver. rtc_class_close() is the matching release, but it is not
> called.
>
> Use a single exit path and close the RTC there, in the same way
> ntp.c:sync_hw_clock() does with its out_close: label.
>
> Found by code review.
>
> Fixes: 59348401ebed ("platform/x86: amd-pmc: Add special handling for timer based S0i3 wakeup")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Ma Ke <make_ruc2021@xxxxxxx>

Rather than adding all the gotos, can this be done with a __free() 
cleanup call on the declaration?

> ---
>   drivers/platform/x86/amd/pmc/pmc.c | 22 +++++++++++++++-------
>   1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c
> index 6792aa2c6187..543bd95e4766 100644
> --- a/drivers/platform/x86/amd/pmc/pmc.c
> +++ b/drivers/platform/x86/amd/pmc/pmc.c
> @@ -671,33 +671,41 @@ static int amd_pmc_verify_czn_rtc(struct amd_pmc_dev *pdev, u32 *arg)
>   	if (rc) {
>   		if (rc == -ENOENT)
>   			dev_dbg(pdev->dev, "no alarm pending\n");
> -		return rc == -ENOENT ? 0 : rc;
> +		rc = rc == -ENOENT ? 0 : rc;
> +		goto out_close;
>   	}
>   	if (!alarm.enabled) {
>   		dev_dbg(pdev->dev, "alarm not enabled\n");
> -		return 0;
> +		rc = 0;
> +		goto out_close;
>   	}
>   	rc = rtc_read_time(rtc_device, &tm);
>   	if (rc)
> -		return rc;
> +		goto out_close;
>   	then = rtc_tm_to_time64(&alarm.time);
>   	now = rtc_tm_to_time64(&tm);
>   	duration = then-now;
>   
>   	/* in the past */
> -	if (then < now)
> -		return 0;
> +	if (then < now) {
> +		rc = 0;
> +		goto out_close;
> +	}
>   
>   	/* will be stored in upper 16 bits of s0i3 hint argument,
>   	 * so timer wakeup from s0i3 is limited to ~18 hours or less
>   	 */
> -	if (duration <= 4 || duration > U16_MAX)
> -		return -EINVAL;
> +	if (duration <= 4 || duration > U16_MAX) {
> +		rc = -EINVAL;
> +		goto out_close;
> +	}
>   
>   	*arg |= (duration << 16);
>   	rc = rtc_alarm_irq_enable(rtc_device, 0);
>   	pm_pr_dbg("wakeup timer programmed for %lld seconds\n", duration);
>   
> +out_close:
> +	rtc_class_close(rtc_device);
>   	return rc;
>   }
>