Re: [PATCH] ASoC: amd: ps: fix snd_acp63_remove() teardown ordering

[Mukunda,Vijendar]

On 9/23/26 14:56, Fan Wu wrote:
> The ACP threaded interrupt handler dereferences the SoundWire and PDM
> child platform devices, but snd_acp63_remove() unregisters them while
> the interrupt is still registered: devm_request_threaded_irq() ties its
> release to devres cleanup, which runs only after the remove callback
> returns. A completion in this window is a use-after-free.
>
> Fix this by masking the ACP interrupt sources and calling devm_free_irq()
> before the first child device is unregistered. The interrupt line is
> shared, and acp_hw_deinit() clears the sources only after the children
> are gone, which would leave the line raised with no handler left to
> ack it. The window predates the tagged refactor, which only reshaped
> the dereferences.
>
> This issue was found by an in-house static analysis tool.
>
> Fixes: eaf825037d6d ("ASoC: amd: ps: refactor acp child platform device creation code")
> Cc: stable@xxxxxxxxxxxxxxx
> Co-developed-by: Song Li <songl@xxxxxxxxxx>
> Signed-off-by: Song Li <songl@xxxxxxxxxx>
> Signed-off-by: Fan Wu <fanwu01@xxxxxxxxxx>
> ---
>   sound/soc/amd/ps/pci-ps.c | 6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/sound/soc/amd/ps/pci-ps.c b/sound/soc/amd/ps/pci-ps.c
> index 729f9aa..01ee697 100644
> --- a/sound/soc/amd/ps/pci-ps.c
> +++ b/sound/soc/amd/ps/pci-ps.c
> @@ -738,6 +738,12 @@ static void snd_acp63_remove(struct pci_dev *pci)
>   	int ret;
>   
>   	adata = pci_get_drvdata(pci);
> +	/* Mask the interrupt sources before freeing the shared IRQ. */
> +	writel(ACP_EXT_INTR_STAT_CLEAR_MASK,
> +	       adata->acp63_base + ACP_EXTERNAL_INTR_STAT);
> +	writel(0, adata->acp63_base + ACP_EXTERNAL_INTR_CNTL);
> +	writel(0, adata->acp63_base + ACP_EXTERNAL_INTR_ENB);
++ The fix open-codes the register offsets (ACP_EXTERNAL_INTR_STAT, 
ACP_EXTERNAL_INTR_CNTL, ACP_EXTERNAL_INTR_ENB) directly in 
snd_acp63_remove(). Future platforms may have different interrupt 
control register offsets, which would require changes in this remove 
path as well. Consider adding a disable_interrupts callback to struct 
acp_hw_ops and invoking it here instead. This keeps the remove path 
platform-agnostic and the interrupt masking logic co-located with its 
platform-specific counterpart in ps-common.c.
> +	devm_free_irq(&pci->dev, pci->irq, adata);
>   	if (adata->sdw) {
>   		amd_sdw_exit(adata);
>   		platform_device_unregister(adata->sdw_dma_dev);