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

[Ilpo Järvinen]

On Mon, 14 Sep 2026, Mario Limonciello wrote:
> 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?

Yes, that's what I too want to see.

I think adding a local DEFINE_FREE() is necessary besides just using 
__free().

-- 
 i.

> > ---
> >   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;
> >   }
> >