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

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- 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.