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

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

[Wentao Liang]

rtc_class_open() takes a reference to the RTC device, but all paths
that return after it succeeded, apart from the final one where the
alarm is programmed, leave the function without dropping it. Route
them through a common exit that calls rtc_class_close().

Fixes: 59348401ebed ("platform/x86: amd-pmc: Add special handling for timer based S0i3 wakeup")
Cc: stable@vger.kernel.org
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 drivers/platform/x86/amd/pmc/pmc.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c
index cae3fcafd4d7..f40e8d9b124a 100644
--- a/drivers/platform/x86/amd/pmc/pmc.c
+++ b/drivers/platform/x86/amd/pmc/pmc.c
@@ -569,32 +569,39 @@ static int amd_pmc_verify_czn_rtc(struct amd_pmc_dev *pdev, u32 *arg)
 		return 0;
 	rc = rtc_read_alarm(rtc_device, &alarm);
 	if (rc)
-		return rc;
+		goto out;
 	if (!alarm.enabled) {
 		dev_dbg(pdev->dev, "alarm not enabled\n");
-		return 0;
+		rc = 0;
+		goto out;
 	}
 	rc = rtc_read_time(rtc_device, &tm);
 	if (rc)
-		return rc;
+		goto out;
 	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;
+	}
 
 	/* 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;
+	}
 
 	*arg |= (duration << 16);
 	rc = rtc_alarm_irq_enable(rtc_device, 0);
 	pm_pr_dbg("wakeup timer programmed for %lld seconds\n", duration);
 
+out:
+	rtc_class_close(rtc_device);
 	return rc;
 }
 
-- 
2.34.1

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

[Ilpo Järvinen]

On Thu, 17 Sep 2026, Wentao Liang wrote:

> rtc_class_open() takes a reference to the RTC device, but all paths
> that return after it succeeded, apart from the final one where the
> alarm is programmed, leave the function without dropping it. Route
> them through a common exit that calls rtc_class_close().
> 
> Fixes: 59348401ebed ("platform/x86: amd-pmc: Add special handling for timer based S0i3 wakeup")
> Cc: stable@vger.kernel.org
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> ---
>  drivers/platform/x86/amd/pmc/pmc.c | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c
> index cae3fcafd4d7..f40e8d9b124a 100644
> --- a/drivers/platform/x86/amd/pmc/pmc.c
> +++ b/drivers/platform/x86/amd/pmc/pmc.c
> @@ -569,32 +569,39 @@ static int amd_pmc_verify_czn_rtc(struct amd_pmc_dev *pdev, u32 *arg)
>  		return 0;
>  	rc = rtc_read_alarm(rtc_device, &alarm);
>  	if (rc)
> -		return rc;
> +		goto out;
>  	if (!alarm.enabled) {
>  		dev_dbg(pdev->dev, "alarm not enabled\n");
> -		return 0;
> +		rc = 0;
> +		goto out;
>  	}
>  	rc = rtc_read_time(rtc_device, &tm);
>  	if (rc)
> -		return rc;
> +		goto out;
>  	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;
> +	}
>  
>  	/* 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;
> +	}
>  
>  	*arg |= (duration << 16);
>  	rc = rtc_alarm_irq_enable(rtc_device, 0);
>  	pm_pr_dbg("wakeup timer programmed for %lld seconds\n", duration);
>  
> +out:
> +	rtc_class_close(rtc_device);
>  	return rc;
>  }

I was expecting 2 patch series, with one which adds the DEFINE_FREE() into 
the public rtc header, not going back to the old approach. You also seemed 
to forget to increase the version number in the subject.

-- 
 i.

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

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 2552 bytes --]

On Thu, 17 Sep 2026, Ilpo Järvinen wrote:

> On Thu, 17 Sep 2026, Wentao Liang wrote:
> 
> > rtc_class_open() takes a reference to the RTC device, but all paths
> > that return after it succeeded, apart from the final one where the
> > alarm is programmed, leave the function without dropping it. Route
> > them through a common exit that calls rtc_class_close().
> > 
> > Fixes: 59348401ebed ("platform/x86: amd-pmc: Add special handling for timer based S0i3 wakeup")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> > ---
> >  drivers/platform/x86/amd/pmc/pmc.c | 21 ++++++++++++++-------
> >  1 file changed, 14 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c
> > index cae3fcafd4d7..f40e8d9b124a 100644
> > --- a/drivers/platform/x86/amd/pmc/pmc.c
> > +++ b/drivers/platform/x86/amd/pmc/pmc.c
> > @@ -569,32 +569,39 @@ static int amd_pmc_verify_czn_rtc(struct amd_pmc_dev *pdev, u32 *arg)
> >  		return 0;
> >  	rc = rtc_read_alarm(rtc_device, &alarm);
> >  	if (rc)
> > -		return rc;
> > +		goto out;
> >  	if (!alarm.enabled) {
> >  		dev_dbg(pdev->dev, "alarm not enabled\n");
> > -		return 0;
> > +		rc = 0;
> > +		goto out;
> >  	}
> >  	rc = rtc_read_time(rtc_device, &tm);
> >  	if (rc)
> > -		return rc;
> > +		goto out;
> >  	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;
> > +	}
> >  
> >  	/* 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;
> > +	}
> >  
> >  	*arg |= (duration << 16);
> >  	rc = rtc_alarm_irq_enable(rtc_device, 0);
> >  	pm_pr_dbg("wakeup timer programmed for %lld seconds\n", duration);
> >  
> > +out:
> > +	rtc_class_close(rtc_device);
> >  	return rc;
> >  }
> 
> I was expecting 2 patch series, with one which adds the DEFINE_FREE() into 
> the public rtc header, not going back to the old approach. You also seemed 
> to forget to increase the version number in the subject.

My apologies, I now realized it was sent earlier by somebody else so I'm 
waiting update from them to fix this issue.

-- 
 i.