Re: [PATCH 1/4] platform/x86/amd/pmc: Fix LPS0 and debugfs leaks when STB init fails

[Ilpo Järvinen]

On Fri, 17 Jul 2026, Mario Limonciello wrote:

> amd_pmc_probe() registers the LPS0 s2idle handler with
> acpi_register_lps0_dev() and creates the driver's debugfs directory
> before calling amd_stb_s2d_init(), which is the last step in probe that
> can fail.
> 
> When amd_stb_s2d_init() fails (for example the S2D telemetry region
> cannot be ioremapped on a long-running system, or the SMU rejects the
> S2D setup) the error path only calls pci_dev_put() and returns.  This
> leaves amd_pmc_s2idle_dev_ops on the global lps0_s2idle_devops_head list
> and leaks the debugfs directory, while the devm-managed resources
> backing the handler are torn down.
> 
> Reloading the module then walks the corrupted list in
> acpi_register_lps0_dev() and hits:
> 
>   list_add corruption. next->prev should be prev, but was NULL.
>   kernel BUG at lib/list_debug.c:29!
>    acpi_register_lps0_dev+0x44/0x80
>    amd_pmc_probe+0x224/0x380 [amd_pmc]
>    platform_probe+0x67/0x90
> 
> Even without a reload, the stale registration means the next s2idle
> transition calls into torn-down driver state.
> 
> Unwind the debugfs directory and the LPS0 registration on the
> amd_stb_s2d_init() error path.  acpi_unregister_lps0_dev() is safe to
> call unconditionally here: it is guarded on the same conditions as
> acpi_register_lps0_dev(), which is exactly what amd_pmc_remove() already
> relies on.
> 
> Assisted-by: Claude:opus
> Reported-by: Francis De Brabandere <francisdb@xxxxxxxxx>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221759
> Tested-by: Francis De Brabandere <francisdb@xxxxxxxxx>
> Fixes: 83ad6974dd3b ("platform/x86/amd/pmc: Move STB block into amd_pmc_s2d_init()")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
> ---
>  drivers/platform/x86/amd/pmc/pmc.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c
> index d50ea62fa2f3a..630a664bdd2f4 100644
> --- a/drivers/platform/x86/amd/pmc/pmc.c
> +++ b/drivers/platform/x86/amd/pmc/pmc.c
> @@ -919,13 +919,17 @@ static int amd_pmc_probe(struct platform_device *pdev)
>  	amd_pmc_dbgfs_register(dev);
>  	err = amd_stb_s2d_init(dev);
>  	if (err)
> -		goto err_pci_dev_put;
> +		goto err_dbgfs_unregister;
>  
>  	if (IS_ENABLED(CONFIG_AMD_MP2_STB))
>  		amd_mp2_stb_init(dev);
>  	pm_report_max_hw_sleep(U64_MAX);
>  	return 0;
>  
> +err_dbgfs_unregister:
> +	amd_pmc_dbgfs_unregister(dev);
> +	if (IS_ENABLED(CONFIG_SUSPEND))
> +		acpi_unregister_lps0_dev(&amd_pmc_s2idle_dev_ops);
>  err_pci_dev_put:
>  	pci_dev_put(rdev);
>  	return err;
> 

No need to change this patch as this looks to be using reverse from the 
order they were init in, but amd_pmc_remove() is using different order
and should IMO changed to match (there probably isn't any good reason 
for the other order).

Sashiko mentions a few low value pre-existing problems that too would be 
nice to address eventually.

-- 
 i.