Message-ID: <sashiko-outbox-203157@kernel.org>
From: sashiko-bot@kernel.org
Subject: Re: [PATCH 4/4] ASoC: amd: ps: guard SDW IRQ work scheduling with
 interrupt_enabled flag
Reply-To: sashiko-reviews@lists.linux.dev
To: "Vijendar Mukunda" <Vijendar.Mukunda@amd.com>
Cc: linux-sound@vger.kernel.org
In-Reply-To: <20261107175339.3547918-5-Vijendar.Mukunda@amd.com>
References: <20261107175339.3547918-1-Vijendar.Mukunda@amd.com>
 <20261107175339.3547918-5-Vijendar.Mukunda@amd.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Date: Sat, 07 Nov 2026 18:12:14 +0000
Precedence: list

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] ASoC: amd: ps: build failure in acp63_irq_handler() due to
  missing struct member
- [Medium] ASoC: amd: ps: TOCTOU race on interrupt_enabled in
  acp63_irq_handler()
--

commit 371c40e74acce37765aa0469d9b4a4ac8ef4b3eb
Author: Vijendar Mukunda <Vijendar.Mukunda@amd.com>

ASoC: amd: ps: guard SDW IRQ work scheduling with interrupt_enabled flag

This commit guards SoundWire manager work scheduling in acp63_irq_handler()
with an amd_manager->interrupt_enabled check for both SoundWire instances.

> diff --git a/sound/soc/amd/ps/pci-ps.c b/sound/soc/amd/ps/pci-ps.c
> index d0b66df101579..0442010e969c0 100644
> --- a/sound/soc/amd/ps/pci-ps.c
> +++ b/sound/soc/amd/ps/pci-ps.c
> @@ -215,7 +215,7 @@ static irqreturn_t acp63_irq_handler(int irq, void *d=
ev_id)
>  		writel(ACP_SDW0_STAT, adata->acp63_base + ACP_EXTERNAL_INTR_STAT);
>  		if (adata->sdw) {
>  			amd_manager =3D dev_get_drvdata(&adata->sdw->pdev[0]->dev);
> -			if (amd_manager)
> +			if (amd_manager && amd_manager->interrupt_enabled)
>  				schedule_work(&amd_manager->amd_sdw_irq_thread);
>  		}
>  		irq_flag =3D 1;
> @@ -226,7 +226,7 @@ static irqreturn_t acp63_irq_handler(int irq, void *d=
ev_id)
>  		writel(ACP_SDW1_STAT, adata->acp63_base + ACP_EXTERNAL_INTR_STAT1);
>  		if (adata->sdw) {
>  			amd_manager =3D dev_get_drvdata(&adata->sdw->pdev[1]->dev);
> -			if (amd_manager)
> +			if (amd_manager && amd_manager->interrupt_enabled)
>  				schedule_work(&amd_manager->amd_sdw_irq_thread);
>  		}
>  		irq_flag =3D 1;

[Severity: High]
Does this break the build when CONFIG_SND_SOC_AMD_PS is enabled?  struct
amd_sdw_manager in include/linux/soundwire/sdw_amd.h has no
interrupt_enabled member, and neither amd_enable_sdw_interrupts() nor
amd_disable_sdw_interrupts() in drivers/soundwire/amd_manager.c sets or
clears such a flag.

[Severity: Medium]
Even with interrupt_enabled added and cleared in
amd_disable_sdw_interrupts(), can acp63_irq_handler() still race with
SoundWire manager teardown and cause a use-after-free of amd_manager?

CPU0                                    CPU1 (<hardirq>)
amd_sdw_manager_remove()
  cancel_work_sync(&amd_sdw_work)
                                        acp63_irq_handler()
                                          amd_manager =3D dev_get_drvdata(.=
..)
                                          if (amd_manager->interrupt_enable=
d)
  amd_disable_sdw_interrupts()
    interrupt_enabled =3D false
                                            schedule_work(&amd_sdw_irq_thre=
ad)
device_unbind_cleanup()
  devres_release_all()  /* frees amd_manager */
  dev_set_drvdata(dev, NULL)
                                        amd_sdw_irq_thread()
                                          /* dereferences freed amd_manager=
 */
                                            <-- use-after-free

acp63_irq_handler() checks dev_get_drvdata() and interrupt_enabled lockless=
ly
in hardirq context on the parent PCI device, while amd_sdw_manager_remove()
never cancels amd_sdw_irq_thread or synchronizes with the parent IRQ before
devres frees amd_manager.

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20261107175339.3547=
918-1-Vijendar.Mukunda@amd.com?part=3D4