Skip to contentSkip to conversation

ASoC: amd: fix IRQ handler races in AMD SoundWire drivers

PATCH In review Vijendar Mukunda 4 participants 20 messages Nov 7 – Nov 9

Subsystems (4)
Sound ASoC Sound - sound open firmware (SOF) drivers AMD ASoC drivers
lore
Share & tools
lore

Vijendar Mukunda

author maintainer
#0 lore raw

cover ASoC: amd: fix IRQ handler races in AMD SoundWire drivers

These four patches fix two related race conditions in the AMD SoundWire IRQ handlers for the PS (ACP63) and SOF platforms.

Patches 1 and 2 guard against a NULL dereference in the ACP external IRQ handler. The handler is registered before sdw_amd_probe() assigns adata->sdw, so a stray interrupt in the probe window, or one that fires after sdw_amd_exit() frees the context, can reach the adata->sdw->pdev[i] dereference with adata->sdw still NULL.

Patches 3 and 4 guard schedule_work() calls with the interrupt_enabled flag so that an in-flight IRQ arriving while the SoundWire manager is being torn down does not queue new work after amd_disable_sdw_interrupts() has cleared the flag.

NOTE: Patches 3 and 4 depend on the following patch which introduces the interrupt_enabled field in the AMD SoundWire manager:

https://lore.kernel.org/linux-sound/[email protected]/

Until that patch is merged, patches 3 and 4 should not be picked up independently.

Vijendar Mukunda (4):
ASoC: amd: ps: guard adata->sdw dereferences with NULL check in IRQ
handler
ASoC: SOF: amd: guard adata->sdw dereferences with NULL check in IRQ
handler
ASoC: SOF: amd: guard SDW IRQ work scheduling with interrupt_enabled
flag
ASoC: amd: ps: guard SDW IRQ work scheduling with interrupt_enabled
flag

 sound/soc/amd/ps/pci-ps.c | 32 ++++++++++++++++++++------------
 sound/soc/sof/amd/acp.c   | 32 ++++++++++++++++++++------------
 2 files changed, 40 insertions(+), 24 deletions(-)

--
2.53.0

author maintainer
#1 lore raw

PATCH 1/4 ASoC: amd: ps: guard adata->sdw dereferences with NULL check in IRQ handler

The ACP external IRQ handler is registered before sdw_amd_probe() assigns adata->sdw, so a stray interrupt in the probe window could reach the adata->sdw->pdev[i] dereference with adata->sdw still NULL.

The same window exists on the teardown side: after sdw_amd_exit() frees the context but before adata->sdw is cleared by the caller.

Add NULL checks for adata->sdw in acp63_irq_handler() before dereferencing ->pdev[0] and ->pdev[1], and in handle_acp70_sdw_wake_event() for the same reason. The interrupt-status register write that acknowledges the IRQ is kept outside the guard so the hardware bit is always cleared regardless of whether the SoundWire context is live.

Fixes e1cb350610ce ("ASoC: amd: ps: handle SoundWire interrupts in acp pci driver")
Signed-off-by Vijendar Mukunda <[email protected]>
---
 sound/soc/amd/ps/pci-ps.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)
sound/soc/amd/ps/pci-ps.c +20 -12
index b604db4942dd..d0b66df10157 100644
@@ -26,16 +26,20 @@ static void handle_acp70_sdw_wake_event(struct acp63_dev_data *adata)
 	struct amd_sdw_manager *amd_manager;
 
 	if (adata->acp70_sdw0_wake_event) {
-		amd_manager = dev_get_drvdata(&adata->sdw->pdev[0]->dev);
-		if (amd_manager)
-			pm_request_resume(amd_manager->dev);
+		if (adata->sdw) {
+			amd_manager = dev_get_drvdata(&adata->sdw->pdev[0]->dev);
+			if (amd_manager)
+				pm_request_resume(amd_manager->dev);
+		}
 		adata->acp70_sdw0_wake_event = 0;
 	}
 
 	if (adata->acp70_sdw1_wake_event) {
-		amd_manager = dev_get_drvdata(&adata->sdw->pdev[1]->dev);
-		if (amd_manager)
-			pm_request_resume(amd_manager->dev);
+		if (adata->sdw) {
+			amd_manager = dev_get_drvdata(&adata->sdw->pdev[1]->dev);
+			if (amd_manager)
+				pm_request_resume(amd_manager->dev);
+		}
 		adata->acp70_sdw1_wake_event = 0;
 	}
 }

@@ -209,18 +213,22 @@ static irqreturn_t acp63_irq_handler(int irq, void *dev_id)
 	ext_intr_stat = readl(adata->acp63_base + ACP_EXTERNAL_INTR_STAT);
 	if (ext_intr_stat & ACP_SDW0_STAT) {
 		writel(ACP_SDW0_STAT, adata->acp63_base + ACP_EXTERNAL_INTR_STAT);
-		amd_manager = dev_get_drvdata(&adata->sdw->pdev[0]->dev);
-		if (amd_manager)
-			schedule_work(&amd_manager->amd_sdw_irq_thread);
+		if (adata->sdw) {
+			amd_manager = dev_get_drvdata(&adata->sdw->pdev[0]->dev);
+			if (amd_manager)
+				schedule_work(&amd_manager->amd_sdw_irq_thread);
+		}
 		irq_flag = 1;
 	}
 
 	ext_intr_stat1 = readl(adata->acp63_base + ACP_EXTERNAL_INTR_STAT1);
 	if (ext_intr_stat1 & ACP_SDW1_STAT) {
 		writel(ACP_SDW1_STAT, adata->acp63_base + ACP_EXTERNAL_INTR_STAT1);
-		amd_manager = dev_get_drvdata(&adata->sdw->pdev[1]->dev);
-		if (amd_manager)
-			schedule_work(&amd_manager->amd_sdw_irq_thread);
+		if (adata->sdw) {
+			amd_manager = dev_get_drvdata(&adata->sdw->pdev[1]->dev);
+			if (amd_manager)
+				schedule_work(&amd_manager->amd_sdw_irq_thread);
+		}
 		irq_flag = 1;
 	}
Signature · 1 line
2.53.0
AI review
#2 lore raw

Re: [PATCH 1/4] ASoC: amd: ps: guard adata->sdw dereferences with NULL check in IRQ handler

Thank you for your contribution! Sashiko AI review found 2 potential issues to consider: - [Medium] ASoC: amd: ps: NULL deref of adata->sdw->pdev[i] on single-link SDW configs - [Low] ASoC: amd: ps…

Thank you for your contribution! Sashiko AI review found 2 potential issues to consider:
- [Medium] ASoC: amd: ps: NULL deref of adata->sdw->pdev[i] on single-link SDW configs
- [Low] ASoC: amd: ps: inaccurate commit message teardown claim and Fixes tag
It also noticed 1 related pre-existing issue this patch does not introduce:
- [Critical] soundwire: amd: use-after-free in amd_sdw_manager_remove()
[1]
--

commit 929ea19624010c51ae72f2eb65f2e1a41edc5971
Author: Vijendar Mukunda <[email protected]>

Quoted 9 lines

ASoC: amd: ps: guard adata->sdw dereferences with NULL check in IRQ handler

The ACP external IRQ handler is registered before sdw_amd_probe() assigns adata->sdw, so a stray interrupt in the probe window could reach the adata->sdw->pdev[i] dereference with adata->sdw still NULL.

The same window exists on the teardown side: after sdw_amd_exit() frees the context but before adata->sdw is cleared by the caller.

Quoted 8 lines

Add NULL checks for adata->sdw in acp63_irq_handler() before dereferencing ->pdev[0] and ->pdev[1], and in handle_acp70_sdw_wake_event() for the same reason. The interrupt-status register write that acknowledges the IRQ is kept outside the guard so the hardware bit is always cleared regardless of whether the SoundWire context is live.

Fixes e1cb350610ce ("ASoC: amd: ps: handle SoundWire interrupts in acp pci driver")
Signed-off-by Vijendar Mukunda <[email protected]>

[Severity: Low]
Is the teardown description and Fixes tag accurate here?

In amd_sdw_exit(), sdw_amd_exit(acp_data->sdw) frees the context while acp_data->sdw is still non-NULL, and acp_data->sdw = NULL only runs after sdw_amd_exit() returns, so an if (adata->sdw) check in that window would see a dangling non-NULL pointer rather than NULL (snd_acp63_remove() instead avoids this by calling acp_hw_disable_interrupts() and devm_free_irq() before amd_sdw_exit()).

Also, e1cb350610ce ("ASoC: amd: ps: handle SoundWire interrupts in acp pci driver") predates adata->sdw. The adata->sdw dereferences in acp63_irq_handler() were introduced by eaf825037d6d ("ASoC: amd: ps:
refactor acp child platform device creation code"), and handle_acp70_sdw_wake_event() was added by 3898b189079c ("ASoC: amd: ps:
add soundwire wake interrupt handling").

Quoted 49 lines
sound/soc/amd/ps/pci-ps.c +15 -9
index b604db4942dd6..d0b66df101579 100644
@@ -26,16 +26,20 @@ static void handle_acp70_sdw_wake_event(struct acp63_dev_data *adata)
 	struct amd_sdw_manager *amd_manager;
 
 	if (adata->acp70_sdw0_wake_event) {
-		amd_manager = dev_get_drvdata(&adata->sdw->pdev[0]->dev);
-		if (amd_manager)
-			pm_request_resume(amd_manager->dev);
+		if (adata->sdw) {
+			amd_manager = dev_get_drvdata(&adata->sdw->pdev[0]->dev);
+			if (amd_manager)
+				pm_request_resume(amd_manager->dev);
+		}
 		adata->acp70_sdw0_wake_event = 0;
 	}
 
 	if (adata->acp70_sdw1_wake_event) {
-		amd_manager = dev_get_drvdata(&adata->sdw->pdev[1]->dev);
-		if (amd_manager)
-			pm_request_resume(amd_manager->dev);
+		if (adata->sdw) {
+			amd_manager = dev_get_drvdata(&adata->sdw->pdev[1]->dev);
+			if (amd_manager)
+				pm_request_resume(amd_manager->dev);
+		}
 		adata->acp70_sdw1_wake_event = 0;
 	}
 }

@@ -209,18 +213,22 @@ static irqreturn_t acp63_irq_handler(int irq, void *dev_id)
 	ext_intr_stat = readl(adata->acp63_base + ACP_EXTERNAL_INTR_STAT);
 	if (ext_intr_stat & ACP_SDW0_STAT) {
 		writel(ACP_SDW0_STAT, adata->acp63_base + ACP_EXTERNAL_INTR_STAT);
-		amd_manager = dev_get_drvdata(&adata->sdw->pdev[0]->dev);
-		if (amd_manager)
-			schedule_work(&amd_manager->amd_sdw_irq_thread);
+		if (adata->sdw) {
+			amd_manager = dev_get_drvdata(&adata->sdw->pdev[0]->dev);
+			if (amd_manager)
+				schedule_work(&amd_manager->amd_sdw_irq_thread);
+		}
 		irq_flag = 1;
 	}
 
 	ext_intr_stat1 = readl(adata->acp63_base + ACP_EXTERNAL_INTR_STAT1);
 	if (ext_intr_stat1 & ACP_SDW1_STAT) {
 		writel(ACP_SDW1_STAT, adata->acp63_base + ACP_EXTERNAL_INTR_STAT1);

sound/soc/amd/ps/pci-ps.c static irqreturn_t acp63_irq_handler(int irq, void *dev_id) · line 233

Quoted 10 lines
sound/soc/amd/ps/pci-ps.c +5 -3
-		amd_manager = dev_get_drvdata(&adata->sdw->pdev[1]->dev);
-		if (amd_manager)
-			schedule_work(&amd_manager->amd_sdw_irq_thread);
+		if (adata->sdw) {
+			amd_manager = dev_get_drvdata(&adata->sdw->pdev[1]->dev);
+			if (amd_manager)
+				schedule_work(&amd_manager->amd_sdw_irq_thread);
+		}
 		irq_flag = 1;
 	}

[Severity: Medium]
Can adata->sdw->pdev[0] or adata->sdw->pdev[1] still be NULL when adata->sdw is non-NULL?

In sdw_amd_probe_controller(), ctx->pdev[index] is only registered for links set in link_mask:

drivers/soundwire/amd_init.c:sdw_amd_probe_controller() {
    ...
	for (index = 0; index < count; index++) {
		if (!(link_mask & BIT(index)))
			continue;
    ...
		ctx->pdev[index] = platform_device_register_full(&pdevinfo[index]);
}

On single-link configurations (where link_mask is BIT(0) or BIT(1)), adata->sdw is non-NULL while the unused link's pdev[i] remains NULL. A stray interrupt or wake event for that link in acp63_irq_handler() or handle_acp70_sdw_wake_event() will then pass &adata->sdw->pdev[i]->dev to dev_get_drvdata() and cause a NULL pointer dereference, as acp70_enable_sdw_host_wake_interrupts() unmasks ACP70_SDW_HOST_WAKE_MASK for both links whenever either link enables host wake.

Should these checks also verify adata->sdw->pdev[0] and adata->sdw->pdev[1] (like handle_acp63_sdw_pme_event() does)?

[Severity: Critical]
This problem wasn't introduced by this patch, but can unbinding the child SoundWire manager device race with acp63_irq_handler() and cause a use-after-free of amd_manager [1]?

CPU0 CPU1
acp63_irq_handler()
amd_manager = dev_get_drvdata()
amd_sdw_manager_remove()
cancel_work_sync(...) amd_disable_sdw_interrupts()
<driver core frees amd_manager>
if (amd_manager)

    schedule_work(&amd_manager->amd_sdw_irq_thread);  <-- use-after-free

amd_sdw_manager_remove() masks SoundWire interrupts and cancels amd_sdw_work, but does not synchronize with the parent ACP IRQ handler or cancel amd_sdw_irq_thread before devres frees amd_manager. If acp63_irq_handler() reads amd_manager before removal finishes and calls schedule_work() afterwards, it queues work on the freed amd_manager.

[1] https://sashiko.dev/#/bug/linux-ab458b2e-f505-4a41-8238-a93ceb5d9c77

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
author maintainer
#3 lore raw

Re: [PATCH 1/4] ASoC: amd: ps: guard adata->sdw dereferences with NULL check in IRQ handler

Quoted 30 lines · On 11/7/26 23:42, [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 2 potential issues to consider:
- [Medium] ASoC: amd: ps: NULL deref of adata->sdw->pdev[i] on single-link SDW configs
- [Low] ASoC: amd: ps: inaccurate commit message teardown claim and Fixes tag
It also noticed 1 related pre-existing issue this patch does not introduce:
- [Critical] soundwire: amd: use-after-free in amd_sdw_manager_remove()
[1]
--

commit 929ea19624010c51ae72f2eb65f2e1a41edc5971
Author: Vijendar Mukunda <[email protected]>

Quoted 15 lines

ASoC: amd: ps: guard adata->sdw dereferences with NULL check in IRQ handler

The ACP external IRQ handler is registered before sdw_amd_probe() assigns adata->sdw, so a stray interrupt in the probe window could reach the adata->sdw->pdev[i] dereference with adata->sdw still NULL.

The same window exists on the teardown side: after sdw_amd_exit() frees the context but before adata->sdw is cleared by the caller.

Add NULL checks for adata->sdw in acp63_irq_handler() before dereferencing ->pdev[0] and ->pdev[1], and in handle_acp70_sdw_wake_event() for the same reason. The interrupt-status register write that acknowledges the IRQ is kept outside the guard so the hardware bit is always cleared regardless of whether the SoundWire context is live.

Quoted 4 lines
Quoted 2 lines
Fixes e1cb350610ce ("ASoC: amd: ps: handle SoundWire interrupts in acp pci driver")
Signed-off-by Vijendar Mukunda <[email protected]>

[Severity: Low]
Is the teardown description and Fixes tag accurate here?

you are correct. During teardown adata->sdw
is non-NULL while sdw_amd_exit() runs and is set to NULL only afterwards, so the if (adata->sdw) guard sees a dangling pointer rather than NULL in that window. snd_acp63_remove() avoids the race by calling acp_hw_disable_interrupts() and devm_free_irq() before amd_sdw_exit(). The teardown sentence will be removed from the commit message in v2.

Quoted 8 lines

In amd_sdw_exit(), sdw_amd_exit(acp_data->sdw) frees the context while acp_data->sdw is still non-NULL, and acp_data->sdw = NULL only runs after sdw_amd_exit() returns, so an if (adata->sdw) check in that window would see a dangling non-NULL pointer rather than NULL (snd_acp63_remove() instead avoids this by calling acp_hw_disable_interrupts() and devm_free_irq() before amd_sdw_exit()).

Quoted 6 lines

Also, e1cb350610ce ("ASoC: amd: ps: handle SoundWire interrupts in acp pci driver") predates adata->sdw. The adata->sdw dereferences in acp63_irq_handler() were introduced by eaf825037d6d ("ASoC: amd: ps:
refactor acp child platform device creation code"), and handle_acp70_sdw_wake_event() was added by 3898b189079c ("ASoC: amd: ps:
add soundwire wake interrupt handling").

Will use correct Fixes tag

Quoted 47 lines
Quoted 47 lines
sound/soc/amd/ps/pci-ps.c +15 -9
index b604db4942dd6..d0b66df101579 100644
@@ -26,16 +26,20 @@ static void handle_acp70_sdw_wake_event(struct acp63_dev_data *adata)
  	struct amd_sdw_manager *amd_manager;
 
  	if (adata->acp70_sdw0_wake_event) {
-		amd_manager = dev_get_drvdata(&adata->sdw->pdev[0]->dev);
-		if (amd_manager)
-			pm_request_resume(amd_manager->dev);
+		if (adata->sdw) {
+			amd_manager = dev_get_drvdata(&adata->sdw->pdev[0]->dev);
+			if (amd_manager)
+				pm_request_resume(amd_manager->dev);
+		}
  		adata->acp70_sdw0_wake_event = 0;
  	}
 
  	if (adata->acp70_sdw1_wake_event) {
-		amd_manager = dev_get_drvdata(&adata->sdw->pdev[1]->dev);
-		if (amd_manager)
-			pm_request_resume(amd_manager->dev);
+		if (adata->sdw) {
+			amd_manager = dev_get_drvdata(&adata->sdw->pdev[1]->dev);
+			if (amd_manager)
+				pm_request_resume(amd_manager->dev);
+		}
  		adata->acp70_sdw1_wake_event = 0;
  	}
  }

@@ -209,18 +213,22 @@ static irqreturn_t acp63_irq_handler(int irq, void *dev_id)
  	ext_intr_stat = readl(adata->acp63_base + ACP_EXTERNAL_INTR_STAT);
  	if (ext_intr_stat & ACP_SDW0_STAT) {
  		writel(ACP_SDW0_STAT, adata->acp63_base + ACP_EXTERNAL_INTR_STAT);
-		amd_manager = dev_get_drvdata(&adata->sdw->pdev[0]->dev);
-		if (amd_manager)
-			schedule_work(&amd_manager->amd_sdw_irq_thread);
+		if (adata->sdw) {
+			amd_manager = dev_get_drvdata(&adata->sdw->pdev[0]->dev);
+			if (amd_manager)
+				schedule_work(&amd_manager->amd_sdw_irq_thread);
+		}
  		irq_flag = 1;
  	}
Quoted 16 lines

sound/soc/amd/ps/pci-ps.c static irqreturn_t acp63_irq_handler(int irq, void *dev_id) · line 233

Quoted 13 lines
sound/soc/amd/ps/pci-ps.c +5 -3
  	ext_intr_stat1 = readl(adata->acp63_base + ACP_EXTERNAL_INTR_STAT1);
  	if (ext_intr_stat1 & ACP_SDW1_STAT) {
  		writel(ACP_SDW1_STAT, adata->acp63_base + ACP_EXTERNAL_INTR_STAT1);
-		amd_manager = dev_get_drvdata(&adata->sdw->pdev[1]->dev);
-		if (amd_manager)
-			schedule_work(&amd_manager->amd_sdw_irq_thread);
+		if (adata->sdw) {
+			amd_manager = dev_get_drvdata(&adata->sdw->pdev[1]->dev);
+			if (amd_manager)
+				schedule_work(&amd_manager->amd_sdw_irq_thread);
+		}
  		irq_flag = 1;
  	}

[Severity: Medium]
Can adata->sdw->pdev[0] or adata->sdw->pdev[1] still be NULL when adata->sdw is non-NULL?

for ACP6.3/7.0/7.1/7.2, all link platform
devices are probed and registered during sdw_amd_probe_controller(). The AMD-specific enable mask only controls whether the start sequence is invoked for a given link, not whether its pdev is registered. So pdev[0] and pdev[1] are always non-NULL when adata->sdw is non-NULL. No additional pdev[i] NULL checks are needed.

Quoted 20 lines

In sdw_amd_probe_controller(), ctx->pdev[index] is only registered for links set in link_mask:

drivers/soundwire/amd_init.c:sdw_amd_probe_controller() {
     ...
	for (index = 0; index < count; index++) {
		if (!(link_mask & BIT(index)))
			continue;
     ...
		ctx->pdev[index] = platform_device_register_full(&pdevinfo[index]);
}

On single-link configurations (where link_mask is BIT(0) or BIT(1)), adata->sdw is non-NULL while the unused link's pdev[i] remains NULL. A stray interrupt or wake event for that link in acp63_irq_handler() or handle_acp70_sdw_wake_event() will then pass &adata->sdw->pdev[i]->dev to dev_get_drvdata() and cause a NULL pointer dereference, as acp70_enable_sdw_host_wake_interrupts() unmasks ACP70_SDW_HOST_WAKE_MASK for both links whenever either link enables host wake.

Quoted 7 lines

Should these checks also verify adata->sdw->pdev[0] and adata->sdw->pdev[1] (like handle_acp63_sdw_pme_event() does)?

[Severity: Critical]
This problem wasn't introduced by this patch, but can unbinding the child SoundWire manager device race with acp63_irq_handler() and cause a use-after-free of amd_manager [1]?

The IRQ related fixes including this gap are addressed in the following series:

https://lore.kernel.org/linux-sound/[email protected]/

Quoted 19 lines

CPU0 CPU1
acp63_irq_handler()
amd_manager = dev_get_drvdata()
amd_sdw_manager_remove()
cancel_work_sync(...) amd_disable_sdw_interrupts()
<driver core frees amd_manager>
if (amd_manager)

     schedule_work(&amd_manager->amd_sdw_irq_thread);  <-- use-after-free

amd_sdw_manager_remove() masks SoundWire interrupts and cancels amd_sdw_work, but does not synchronize with the parent ACP IRQ handler or cancel amd_sdw_irq_thread before devres frees amd_manager. If acp63_irq_handler() reads amd_manager before removal finishes and calls schedule_work() afterwards, it queues work on the freed amd_manager.

[1] https://sashiko.dev/#/bug/linux-ab458b2e-f505-4a41-8238-a93ceb5d9c77

author maintainer
#4 lore raw

PATCH 2/4 ASoC: SOF: amd: guard adata->sdw dereferences with NULL check in IRQ handler

In reply to Vijendar Mukunda

The ACP external IRQ handler is registered before sdw_amd_probe() assigns adata->sdw, so a stray interrupt in the probe window could reach the adata->sdw->pdev[i] dereference with adata->sdw still NULL.

The same window exists on the teardown side: after sdw_amd_exit() frees the context but before adata->sdw is cleared by the caller.

Add NULL checks for adata->sdw in acp_irq_handler() before dereferencing ->pdev[0] and ->pdev[1], and in amd_sof_handle_acp70_sdw_wake_event() for the same reason. The interrupt-status register write that acknowledges the IRQ is kept outside the guard so the hardware bit is always cleared regardless of whether the SoundWire context is live.

Fixes 96eb81851012 ("ASoC: SOF: amd: add interrupt handling for SoundWire manager devices")
Signed-off-by Vijendar Mukunda <[email protected]>
---
 sound/soc/sof/amd/acp.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)
sound/soc/sof/amd/acp.c +20 -12
index 86e2402b8ffd..623e016f38b8 100644
@@ -478,16 +478,20 @@ static void amd_sof_handle_acp70_sdw_wake_event(struct acp_dev_data *adata)
 	struct amd_sdw_manager *amd_manager;
 
 	if (adata->acp70_sdw0_wake_event) {
-		amd_manager = dev_get_drvdata(&adata->sdw->pdev[0]->dev);
-		if (amd_manager)
-			pm_request_resume(amd_manager->dev);
+		if (adata->sdw) {
+			amd_manager = dev_get_drvdata(&adata->sdw->pdev[0]->dev);
+			if (amd_manager)
+				pm_request_resume(amd_manager->dev);
+		}
 		adata->acp70_sdw0_wake_event = 0;
 	}
 
 	if (adata->acp70_sdw1_wake_event) {
-		amd_manager = dev_get_drvdata(&adata->sdw->pdev[1]->dev);
-		if (amd_manager)
-			pm_request_resume(amd_manager->dev);
+		if (adata->sdw) {
+			amd_manager = dev_get_drvdata(&adata->sdw->pdev[1]->dev);
+			if (amd_manager)
+				pm_request_resume(amd_manager->dev);
+		}
 		adata->acp70_sdw1_wake_event = 0;
 	}
 }

@@ -579,10 +583,12 @@ static irqreturn_t acp_irq_handler(int irq, void *dev_id)
 
 	val = snd_sof_dsp_read(sdev, ACP_DSP_BAR, desc->ext_intr_stat);
 	if (val & ACP_SDW0_IRQ_MASK) {
-		amd_manager = dev_get_drvdata(&adata->sdw->pdev[0]->dev);
 		snd_sof_dsp_write(sdev, ACP_DSP_BAR, desc->ext_intr_stat, ACP_SDW0_IRQ_MASK);
-		if (amd_manager)
-			schedule_work(&amd_manager->amd_sdw_irq_thread);
+		if (adata->sdw) {
+			amd_manager = dev_get_drvdata(&adata->sdw->pdev[0]->dev);
+			if (amd_manager)
+				schedule_work(&amd_manager->amd_sdw_irq_thread);
+		}
 		irq_flag = 1;
 	}
 

@@ -608,11 +614,13 @@ static irqreturn_t acp_irq_handler(int irq, void *dev_id)
 	if (desc->ext_intr_stat1) {
 		val = snd_sof_dsp_read(sdev, ACP_DSP_BAR, desc->ext_intr_stat1);
 		if (val & ACP_SDW1_IRQ_MASK) {
-			amd_manager = dev_get_drvdata(&adata->sdw->pdev[1]->dev);
 			snd_sof_dsp_write(sdev, ACP_DSP_BAR, desc->ext_intr_stat1,
 					  ACP_SDW1_IRQ_MASK);
-			if (amd_manager)
-				schedule_work(&amd_manager->amd_sdw_irq_thread);
+			if (adata->sdw) {
+				amd_manager = dev_get_drvdata(&adata->sdw->pdev[1]->dev);
+				if (amd_manager)
+					schedule_work(&amd_manager->amd_sdw_irq_thread);
+			}
 			irq_flag = 1;
 		}
 		switch (adata->pci_rev) {
Signature · 1 line
2.53.0
AI review
#5 lore raw

Re: [PATCH 2/4] ASoC: SOF: amd: guard adata->sdw dereferences with NULL check in IRQ handler

Thank you for your contribution! Sashiko AI review found 2 potential issues to consider: - [Medium] ASoC: SOF: amd: NULL dereference of adata->sdw->pdev[i] in acp_irq_handler() - [Low] ASoC: SOF…

Thank you for your contribution! Sashiko AI review found 2 potential issues to consider:
- [Medium] ASoC: SOF: amd: NULL dereference of adata->sdw->pdev[i] in acp_irq_handler()
- [Low] ASoC: SOF: amd: inaccurate teardown claim and missing Fixes tag It also noticed 1 related pre-existing issue this patch does not introduce:
- [Medium] soundwire: amd: memory and platform device leaks in sdw_amd_probe() [1]
--

commit 920e1ff1deb60367eb67bae9d4d4b5880a64d07f
Author: Vijendar Mukunda <[email protected]>

Quoted 9 lines

ASoC: SOF: amd: guard adata->sdw dereferences with NULL check in IRQ handler

The ACP external IRQ handler is registered before sdw_amd_probe() assigns adata->sdw, so a stray interrupt in the probe window could reach the adata->sdw->pdev[i] dereference with adata->sdw still NULL.

The same window exists on the teardown side: after sdw_amd_exit() frees the context but before adata->sdw is cleared by the caller.

Quoted 8 lines

Add NULL checks for adata->sdw in acp_irq_handler() before dereferencing ->pdev[0] and ->pdev[1], and in amd_sof_handle_acp70_sdw_wake_event() for the same reason. The interrupt-status register write that acknowledges the IRQ is kept outside the guard so the hardware bit is always cleared regardless of whether the SoundWire context is live.

Fixes 96eb81851012 ("ASoC: SOF: amd: add interrupt handling for SoundWire manager devices")
Signed-off-by Vijendar Mukunda <[email protected]>

[Severity: Low]
Is the teardown window description in the second paragraph accurate? In amd_sof_sdw_exit(), sdw_amd_exit(acp_data->sdw) frees the context before acp_data->sdw = NULL runs, so adata->sdw is still non-NULL during that interval and an if (adata->sdw) check cannot guard it; teardown is safe instead because callers invoke free_irq(sdev->ipc_irq, sdev) before calling amd_sof_sdw_exit().

Also, since the unguarded adata->sdw dereferences in amd_sof_handle_acp70_sdw_wake_event() were introduced by a later commit, should this include:

Fixes 1f41bf67bfe3 ("ASoC: SOF: amd: add soundwire wake irq handling")
Quoted 53 lines
sound/soc/sof/amd/acp.c +15 -10
index 86e2402b8ffd2..623e016f38b83 100644
@@ -478,16 +478,20 @@ static void amd_sof_handle_acp70_sdw_wake_event(struct acp_dev_data *adata)
 	struct amd_sdw_manager *amd_manager;
 
 	if (adata->acp70_sdw0_wake_event) {
-		amd_manager = dev_get_drvdata(&adata->sdw->pdev[0]->dev);
-		if (amd_manager)
-			pm_request_resume(amd_manager->dev);
+		if (adata->sdw) {
+			amd_manager = dev_get_drvdata(&adata->sdw->pdev[0]->dev);
+			if (amd_manager)
+				pm_request_resume(amd_manager->dev);
+		}
 		adata->acp70_sdw0_wake_event = 0;
 	}
 
 	if (adata->acp70_sdw1_wake_event) {
-		amd_manager = dev_get_drvdata(&adata->sdw->pdev[1]->dev);
-		if (amd_manager)
-			pm_request_resume(amd_manager->dev);
+		if (adata->sdw) {
+			amd_manager = dev_get_drvdata(&adata->sdw->pdev[1]->dev);
+			if (amd_manager)
+				pm_request_resume(amd_manager->dev);
+		}
 		adata->acp70_sdw1_wake_event = 0;
 	}
 }

@@ -579,10 +583,12 @@ static irqreturn_t acp_irq_handler(int irq, void *dev_id)
 
 	val = snd_sof_dsp_read(sdev, ACP_DSP_BAR, desc->ext_intr_stat);
 	if (val & ACP_SDW0_IRQ_MASK) {
-		amd_manager = dev_get_drvdata(&adata->sdw->pdev[0]->dev);
 		snd_sof_dsp_write(sdev, ACP_DSP_BAR, desc->ext_intr_stat, ACP_SDW0_IRQ_MASK);
-		if (amd_manager)
-			schedule_work(&amd_manager->amd_sdw_irq_thread);
+		if (adata->sdw) {
+			amd_manager = dev_get_drvdata(&adata->sdw->pdev[0]->dev);
+			if (amd_manager)
+				schedule_work(&amd_manager->amd_sdw_irq_thread);
+		}
 		irq_flag = 1;
 	}
 

@@ -608,11 +614,13 @@ static irqreturn_t acp_irq_handler(int irq, void *dev_id)
 	if (desc->ext_intr_stat1) {
 		val = snd_sof_dsp_read(sdev, ACP_DSP_BAR, desc->ext_intr_stat1);
 		if (val & ACP_SDW1_IRQ_MASK) {
-			amd_manager = dev_get_drvdata(&adata->sdw->pdev[1]->dev);
 			snd_sof_dsp_write(sdev, ACP_DSP_BAR, desc->ext_intr_stat1,

sound/soc/sof/amd/acp.c static irqreturn_t acp_irq_handler(int irq, void *dev_id) · line 625

Quoted 10 lines
sound/soc/sof/amd/acp.c +5 -2
 					  ACP_SDW1_IRQ_MASK);
-			if (amd_manager)
-				schedule_work(&amd_manager->amd_sdw_irq_thread);
+			if (adata->sdw) {
+				amd_manager = dev_get_drvdata(&adata->sdw->pdev[1]->dev);
+				if (amd_manager)
+					schedule_work(&amd_manager->amd_sdw_irq_thread);
+			}
 			irq_flag = 1;
 		}

[Severity: Medium]
Can adata->sdw->pdev[0] or adata->sdw->pdev[1] still be NULL here when adata->sdw is non-NULL?

In sdw_amd_probe_controller(), ctx->pdev[index] is only populated for links set in link_mask:

drivers/soundwire/amd_init.c:sdw_amd_probe_controller() {
	...
	for (index = 0; index < count; index++) {
		if (!(link_mask & BIT(index)))
			continue;
		...
		ctx->pdev[index] = platform_device_register_full(&pdevinfo[index]);
	...
}

On single-link configurations where only one SoundWire manager is active, the unused link's pdev[i] remains NULL while adata->sdw is non-NULL. Passing &adata->sdw->pdev[i]->dev to dev_get_drvdata() on a stray interrupt or wake event for the unpopulated link will cause a NULL pointer dereference in hard IRQ context, unlike the sibling ACP7x handlers in this file (such as sof_acp7x_handle_sdw_manager_irq()) which check adata->sdw->pdev[instance] before calling dev_get_drvdata().

[Severity: Medium]
This problem wasn't introduced by this patch, but in the same SoundWire probe path [1], does a failure in sdw_amd_startup() leak the sdw_amd_ctx memory and platform devices allocated by sdw_amd_probe_controller(), as well as adata->dmic_dev in amd_sof_acp_probe()?

drivers/soundwire/amd_init.c:sdw_amd_probe() {
	*sdw_ctx = sdw_amd_probe_controller(res);
	if (!*sdw_ctx)
		return -ENODEV;

	return sdw_amd_startup(*sdw_ctx);
}
sound/soc/sof/amd/acp.c:amd_sof_acp_probe() {
	...
	adata->dmic_dev = platform_device_register_data(sdev->dev, "dmic-codec",
							PLATFORM_DEVID_NONE, NULL, 0);
	...
	ret = amd_sof_sdw_probe(sdev);
	if (ret < 0) {
		dev_err(sdev->dev, "error: SoundWire probe error\n");
		free_irq(sdev->ipc_irq, sdev);
		return ret;
	}
	...
unregister_dev:
	platform_device_unregister(adata->dmic_dev);
	return ret;
}

When sdw_amd_startup() fails, sdw_amd_probe() returns without unregistering the ctx->pdev[index] devices or freeing ctx, and amd_sof_acp_probe() returns directly after free_irq() instead of jumping to unregister_dev to unregister adata->dmic_dev.

[1] https://sashiko.dev/#/bug/linux-3af16781-2451-4877-8594-c12800a7bba5

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
author maintainer
#6 lore raw

PATCH 3/4 ASoC: SOF: amd: guard SDW IRQ work scheduling with interrupt_enabled flag

In reply to Vijendar Mukunda

acp_irq_handler() in sound/soc/sof/amd/acp.c called schedule_work() for the SoundWire manager workqueue guarded only by a NULL check on amd_manager, the same gap fixed in the PS platform IRQ handler.

Add the interrupt_enabled check so that an in-flight IRQ arriving while the SoundWire manager is being torn down does not queue new work after amd_disable_sdw_interrupts() has cleared the flag.

Reported-by Lee Jones <[email protected]>
Closes https://sashiko.dev/#/bug/linux-ab458b2e-f505-4a41-8238-a93ceb5d9c77
Fixes 96eb81851012 ("ASoC: SOF: amd: add interrupt handling for SoundWire manager devices")
Signed-off-by Vijendar Mukunda <[email protected]>
---
 sound/soc/sof/amd/acp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
sound/soc/sof/amd/acp.c +2 -2
index 623e016f38b8..5c4eab0be497 100644
@@ -586,7 +586,7 @@ static irqreturn_t acp_irq_handler(int irq, void *dev_id)
 		snd_sof_dsp_write(sdev, ACP_DSP_BAR, desc->ext_intr_stat, ACP_SDW0_IRQ_MASK);
 		if (adata->sdw) {
 			amd_manager = 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 = 1;

@@ -618,7 +618,7 @@ static irqreturn_t acp_irq_handler(int irq, void *dev_id)
 					  ACP_SDW1_IRQ_MASK);
 			if (adata->sdw) {
 				amd_manager = 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 = 1;
Signature · 1 line
2.53.0
AI review
#7 lore raw

Re: [PATCH 3/4] ASoC: SOF: amd: guard SDW IRQ work scheduling with interrupt_enabled flag

Thank you for your contribution! Sashiko AI review found 2 potential issues to consider: - [High] ASoC: SOF: amd: build failure in acp_irq_handler() on missing struct member - [Medium] ASoC: SOF…

Thank you for your contribution! Sashiko AI review found 2 potential issues to consider:
- [High] ASoC: SOF: amd: build failure in acp_irq_handler() on missing struct member
- [Medium] ASoC: SOF: amd: unguarded work queue in sof_acp7x_handle_sdw_manager_irq()
--

commit 84b5998c660c53c97f1f81d73a850a90f66dfb5d
Author: Vijendar Mukunda <[email protected]>

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

This commit checks amd_manager->interrupt_enabled before calling schedule_work(&amd_manager->amd_sdw_irq_thread) in acp_irq_handler().

Quoted 14 lines
sound/soc/sof/amd/acp.c +1 -1
index 623e016f38b83..5c4eab0be497c 100644
@@ -586,7 +586,7 @@ static irqreturn_t acp_irq_handler(int irq, void *dev_id)
 		snd_sof_dsp_write(sdev, ACP_DSP_BAR, desc->ext_intr_stat, ACP_SDW0_IRQ_MASK);
 		if (adata->sdw) {
 			amd_manager = 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 = 1;

sound/soc/sof/amd/acp.c static irqreturn_t acp_irq_handler(int irq, void *dev_id) · line 624

Quoted 8 lines
sound/soc/sof/amd/acp.c +1 -1
@@ -618,7 +618,7 @@ static irqreturn_t acp_irq_handler(int irq, void *dev_id)
 					  ACP_SDW1_IRQ_MASK);
 			if (adata->sdw) {
 				amd_manager = 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 = 1;

[Severity: High]
Does this break the build when CONFIG_SND_SOC_SOF_AMD_COMMON is enabled? struct amd_sdw_manager in include/linux/soundwire/sdw_amd.h has no interrupt_enabled member, and amd_enable_sdw_interrupts() and amd_disable_sdw_interrupts() in drivers/soundwire/amd_manager.c only update hardware interrupt mask registers without setting or clearing any software flag.

[Severity: Medium]
Assuming interrupt_enabled is added to struct amd_sdw_manager, should sof_acp7x_handle_sdw_manager_irq() also check amd_manager->interrupt_enabled before queuing amd_sdw_irq_thread?

sound/soc/sof/amd/acp.c:sof_acp7x_handle_sdw_manager_irq() {
	...
		amd_manager = dev_get_drvdata(&adata->sdw->pdev[instance]->dev);
		if (amd_manager)
			schedule_work(&amd_manager->amd_sdw_irq_thread);
}

On ACP7x platforms, an in-flight IRQ in acp7x_irq_handler() racing with amd_sdw_manager_remove() or amd_deinit_sdw_manager() can still queue amd_sdw_irq_thread after interrupts are disabled and pending work is cancelled, leading to a use-after-free on amd_manager or register accesses to a disabled manager.

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
author maintainer
#8 lore raw

Re: [PATCH 3/4] ASoC: SOF: amd: guard SDW IRQ work scheduling with interrupt_enabled flag

Quoted 46 lines · On 11/7/26 23:38, [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 2 potential issues to consider:
- [High] ASoC: SOF: amd: build failure in acp_irq_handler() on missing struct member
- [Medium] ASoC: SOF: amd: unguarded work queue in sof_acp7x_handle_sdw_manager_irq()
--

commit 84b5998c660c53c97f1f81d73a850a90f66dfb5d
Author: Vijendar Mukunda <[email protected]>

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

This commit checks amd_manager->interrupt_enabled before calling schedule_work(&amd_manager->amd_sdw_irq_thread) in acp_irq_handler().

Quoted 14 lines
sound/soc/sof/amd/acp.c +1 -1
index 623e016f38b83..5c4eab0be497c 100644
@@ -586,7 +586,7 @@ static irqreturn_t acp_irq_handler(int irq, void *dev_id)
  		snd_sof_dsp_write(sdev, ACP_DSP_BAR, desc->ext_intr_stat, ACP_SDW0_IRQ_MASK);
  		if (adata->sdw) {
  			amd_manager = 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 = 1;

sound/soc/sof/amd/acp.c static irqreturn_t acp_irq_handler(int irq, void *dev_id) · line 624

Quoted 8 lines
sound/soc/sof/amd/acp.c +1 -1
@@ -618,7 +618,7 @@ static irqreturn_t acp_irq_handler(int irq, void *dev_id)
  					  ACP_SDW1_IRQ_MASK);
  			if (adata->sdw) {
  				amd_manager = 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 = 1;

[Severity: High]
Does this break the build when CONFIG_SND_SOC_SOF_AMD_COMMON is enabled? struct amd_sdw_manager in include/linux/soundwire/sdw_amd.h has no interrupt_enabled member, and amd_enable_sdw_interrupts() and amd_disable_sdw_interrupts() in drivers/soundwire/amd_manager.c only update hardware interrupt mask registers without setting or clearing any software flag.

Quoted 4 lines

[Severity: Medium]
Assuming interrupt_enabled is added to struct amd_sdw_manager, should sof_acp7x_handle_sdw_manager_irq() also check amd_manager->interrupt_enabled before queuing amd_sdw_irq_thread?

The interrupt_enabled field is introduced by a prerequisite patch in the AMD SoundWire manager series:

https://lore.kernel.org/linux-sound/[email protected]/

The cover letter for this series explicitly notes that patches 3 and 4 depend on that patch and should not be picked up independently until it is merged. There is no build failure when the series is applied in the correct order.

Quoted 7 lines
sound/soc/sof/amd/acp.c:sof_acp7x_handle_sdw_manager_irq() {
	...
		amd_manager = dev_get_drvdata(&adata->sdw->pdev[instance]->dev);
		if (amd_manager)
			schedule_work(&amd_manager->amd_sdw_irq_thread);
}
Quoted 5 lines

On ACP7x platforms, an in-flight IRQ in acp7x_irq_handler() racing with amd_sdw_manager_remove() or amd_deinit_sdw_manager() can still queue amd_sdw_irq_thread after interrupts are disabled and pending work is cancelled, leading to a use-after-free on amd_manager or register accesses to a disabled manager.

ACP7.B/7.F SoundWire manager support has not yet been added to the SoundWire driver, so adata->sdw is never assigned on ACP7x platforms and this branch is never entered.  The interrupt_enabled guard for sof_acp7x_handle_sdw_manager_irq() will be addressed as part of the upcoming ACP7.B/7.F SoundWire support series.

Quoted 1 line
build bot
#9 lore raw

Re: [PATCH 3/4] ASoC: SOF: amd: guard SDW IRQ work scheduling with interrupt_enabled flag

In reply to Vijendar Mukunda
kernel test robot noticed the following build errors:

Hi Vijendar,

kernel test robot noticed the following build errors:

[auto build test ERROR on broonie-sound/for-next]
[also build test ERROR on vkoul-dmaengine/next linus/master v7.4-rc1 next-20261105] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Vijendar-Mukunda/ASoC-amd-ps-guard-adata-sdw-dereferences-with-NULL-check-in-IRQ-handler/20261107-232226
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
patch link:    https://lore.kernel.org/r/20261107175339.3547918-4-Vijendar.Mukunda%40amd.com
patch subject: [PATCH 3/4] ASoC: SOF: amd: guard SDW IRQ work scheduling with interrupt_enabled flag
config: x86_64-rhel-9.4-ltp (https://download.01.org/0day-ci/archive/20261108/[email protected]/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20261108/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

sound/soc/sof/amd/acp.c: In function 'acp_irq_handler':

>> sound/soc/sof/amd/acp.c:589:55: error: 'struct amd_sdw_manager' has no member named 'interrupt_enabled'
     589 |                         if (amd_manager && amd_manager->interrupt_enabled)
         |                                                       ^~
   sound/soc/sof/amd/acp.c:621:63: error: 'struct amd_sdw_manager' has no member named 'interrupt_enabled'
     621 |                                 if (amd_manager && amd_manager->interrupt_enabled)
         |                                                               ^~

vim +589 sound/soc/sof/amd/acp.c

566
567 static irqreturn_t acp_irq_handler(int irq, void *dev_id)
568 {
569 struct amd_sdw_manager *amd_manager;
570 struct snd_sof_dev *sdev = dev_id;
571 const struct sof_amd_acp_desc *desc = get_chip_info(sdev->pdata);
572 struct acp_dev_data *adata = sdev->pdata->hw_pdata;
573 unsigned int base = desc->dsp_intr_base;
574 unsigned int val;
575 int irq_flag = 0, wake_irq_flag = 0;
576
577 val = snd_sof_dsp_read(sdev, ACP_DSP_BAR, base + DSP_SW_INTR_STAT_OFFSET);
578 if (val & ACP_DSP_TO_HOST_IRQ) {
579 snd_sof_dsp_write(sdev, ACP_DSP_BAR, base + DSP_SW_INTR_STAT_OFFSET,
580 ACP_DSP_TO_HOST_IRQ);
581 return IRQ_WAKE_THREAD;
582 }
583
584 val = snd_sof_dsp_read(sdev, ACP_DSP_BAR, desc->ext_intr_stat);
585 if (val & ACP_SDW0_IRQ_MASK) {
586 snd_sof_dsp_write(sdev, ACP_DSP_BAR, desc->ext_intr_stat, ACP_SDW0_IRQ_MASK);
587 if (adata->sdw) {
588 amd_manager = dev_get_drvdata(&adata->sdw->pdev[0]->dev);

 > 589				if (amd_manager && amd_manager->interrupt_enabled)

590 schedule_work(&amd_manager->amd_sdw_irq_thread);
591 }
592 irq_flag = 1;
593 }
594
595 if (val & ACP_ERROR_IRQ_MASK) {
596 snd_sof_dsp_write(sdev, ACP_DSP_BAR, desc->ext_intr_stat, ACP_ERROR_IRQ_MASK);
597 snd_sof_dsp_write(sdev, ACP_DSP_BAR, desc->acp_sw0_i2s_err_reason, 0);
598 /* ACP_SW1_I2S_ERROR_REASON is newly added register from rmb platform onwards */
599 switch (adata->pci_rev) {
600 case ACP_RMB_PCI_ID:
601 case ACP63_PCI_ID:
602 case ACP70_PCI_ID:
603 case ACP71_PCI_ID:
604 case ACP72_PCI_ID:
605 snd_sof_dsp_write(sdev, ACP_DSP_BAR, ACP_SW1_I2S_ERROR_REASON, 0);
606 break;
607 default:
608 break;
609 }
610 snd_sof_dsp_write(sdev, ACP_DSP_BAR, desc->acp_error_stat, 0);
611 irq_flag = 1;
612 }
613
614 if (desc->ext_intr_stat1) {
615 val = snd_sof_dsp_read(sdev, ACP_DSP_BAR, desc->ext_intr_stat1);
616 if (val & ACP_SDW1_IRQ_MASK) {
617 snd_sof_dsp_write(sdev, ACP_DSP_BAR, desc->ext_intr_stat1,
618 ACP_SDW1_IRQ_MASK);
619 if (adata->sdw) {
620 amd_manager = dev_get_drvdata(&adata->sdw->pdev[1]->dev);
621 if (amd_manager && amd_manager->interrupt_enabled)
622 schedule_work(&amd_manager->amd_sdw_irq_thread);
623 }
624 irq_flag = 1;
625 }
626 switch (adata->pci_rev) {
627 case ACP70_PCI_ID:
628 case ACP71_PCI_ID:
629 case ACP72_PCI_ID:
630 wake_irq_flag = amd_sof_check_and_handle_acp70_sdw_wake_irq(sdev);
631 break;
632 }
633 }
634 if (irq_flag || wake_irq_flag)
635 return IRQ_HANDLED;
636 else
637 return IRQ_NONE;
638 }
639

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

author maintainer
#10 lore raw

Re: [PATCH 3/4] ASoC: SOF: amd: guard SDW IRQ work scheduling with interrupt_enabled flag

Quoted 18 lines · kernel test robot

Hi Vijendar,

kernel test robot noticed the following build errors:

[auto build test ERROR on broonie-sound/for-next]
[also build test ERROR on vkoul-dmaengine/next linus/master v7.4-rc1 next-20261105] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Vijendar-Mukunda/ASoC-amd-ps-guard-adata-sdw-dereferences-with-NULL-check-in-IRQ-handler/20261107-232226
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
patch link:    https://lore.kernel.org/r/20261107175339.3547918-4-Vijendar.Mukunda%40amd.com
patch subject: [PATCH 3/4] ASoC: SOF: amd: guard SDW IRQ work scheduling with interrupt_enabled flag
config: x86_64-rhel-9.4-ltp (https://download.01.org/0day-ci/archive/20261108/[email protected]/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20261108/[email protected]/reproduce)
Quoted 9 lines

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

sound/soc/sof/amd/acp.c: In function 'acp_irq_handler':

Quoted 1 line
Quoted 1 line

sound/soc/sof/amd/acp.c:589:55: error: 'struct amd_sdw_manager' has no member named 'interrupt_enabled'

As mentioned in cover letter, Patch 3/4 & 4/4 has dependency on below patch which introduces the interrupt_enabled field in the AMD SoundWire manager:

https://lore.kernel.org/linux-sound/[email protected]/

Quoted 87 lines
     589 |                         if (amd_manager && amd_manager->interrupt_enabled)
         |                                                       ^~
   sound/soc/sof/amd/acp.c:621:63: error: 'struct amd_sdw_manager' has no member named 'interrupt_enabled'
     621 |                                 if (amd_manager && amd_manager->interrupt_enabled)
         |                                                               ^~

vim +589 sound/soc/sof/amd/acp.c

   566	
   567	static irqreturn_t acp_irq_handler(int irq, void *dev_id)
   568	{
   569		struct amd_sdw_manager *amd_manager;
   570		struct snd_sof_dev *sdev = dev_id;
   571		const struct sof_amd_acp_desc *desc = get_chip_info(sdev->pdata);
   572		struct acp_dev_data *adata = sdev->pdata->hw_pdata;
   573		unsigned int base = desc->dsp_intr_base;
   574		unsigned int val;
   575		int irq_flag = 0, wake_irq_flag = 0;
   576	
   577		val = snd_sof_dsp_read(sdev, ACP_DSP_BAR, base + DSP_SW_INTR_STAT_OFFSET);
   578		if (val & ACP_DSP_TO_HOST_IRQ) {
   579			snd_sof_dsp_write(sdev, ACP_DSP_BAR, base + DSP_SW_INTR_STAT_OFFSET,
   580					  ACP_DSP_TO_HOST_IRQ);
   581			return IRQ_WAKE_THREAD;
   582		}
   583	
   584		val = snd_sof_dsp_read(sdev, ACP_DSP_BAR, desc->ext_intr_stat);
   585		if (val & ACP_SDW0_IRQ_MASK) {
   586			snd_sof_dsp_write(sdev, ACP_DSP_BAR, desc->ext_intr_stat, ACP_SDW0_IRQ_MASK);
   587			if (adata->sdw) {
   588				amd_manager = dev_get_drvdata(&adata->sdw->pdev[0]->dev);
Quoted 1 line

589 if (amd_manager && amd_manager->interrupt_enabled)

   590					schedule_work(&amd_manager->amd_sdw_irq_thread);
   591			}
   592			irq_flag = 1;
   593		}
   594	
   595		if (val & ACP_ERROR_IRQ_MASK) {
   596			snd_sof_dsp_write(sdev, ACP_DSP_BAR, desc->ext_intr_stat, ACP_ERROR_IRQ_MASK);
   597			snd_sof_dsp_write(sdev, ACP_DSP_BAR, desc->acp_sw0_i2s_err_reason, 0);
   598			/* ACP_SW1_I2S_ERROR_REASON is newly added register from rmb platform onwards */
   599			switch (adata->pci_rev) {
   600			case ACP_RMB_PCI_ID:
   601			case ACP63_PCI_ID:
   602			case ACP70_PCI_ID:
   603			case ACP71_PCI_ID:
   604			case ACP72_PCI_ID:
   605				snd_sof_dsp_write(sdev, ACP_DSP_BAR, ACP_SW1_I2S_ERROR_REASON, 0);
   606				break;
   607			default:
   608				break;
   609			}
   610			snd_sof_dsp_write(sdev, ACP_DSP_BAR, desc->acp_error_stat, 0);
   611			irq_flag = 1;
   612		}
   613	
   614		if (desc->ext_intr_stat1) {
   615			val = snd_sof_dsp_read(sdev, ACP_DSP_BAR, desc->ext_intr_stat1);
   616			if (val & ACP_SDW1_IRQ_MASK) {
   617				snd_sof_dsp_write(sdev, ACP_DSP_BAR, desc->ext_intr_stat1,
   618						  ACP_SDW1_IRQ_MASK);
   619				if (adata->sdw) {
   620					amd_manager = dev_get_drvdata(&adata->sdw->pdev[1]->dev);
   621					if (amd_manager && amd_manager->interrupt_enabled)
   622						schedule_work(&amd_manager->amd_sdw_irq_thread);
   623				}
   624				irq_flag = 1;
   625			}
   626			switch (adata->pci_rev) {
   627			case ACP70_PCI_ID:
   628			case ACP71_PCI_ID:
   629			case ACP72_PCI_ID:
   630				wake_irq_flag = amd_sof_check_and_handle_acp70_sdw_wake_irq(sdev);
   631				break;
   632			}
   633		}
   634		if (irq_flag || wake_irq_flag)
   635			return IRQ_HANDLED;
   636		else
   637			return IRQ_NONE;
   638	}
   639

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

build bot
#11 lore raw

Re: [PATCH 3/4] ASoC: SOF: amd: guard SDW IRQ work scheduling with interrupt_enabled flag

In reply to Vijendar Mukunda
kernel test robot noticed the following build errors:

Hi Vijendar,

kernel test robot noticed the following build errors:

[auto build test ERROR on broonie-sound/for-next]
[also build test ERROR on vkoul-dmaengine/next linus/master v7.4-rc1 next-20261105] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Vijendar-Mukunda/ASoC-amd-ps-guard-adata-sdw-dereferences-with-NULL-check-in-IRQ-handler/20261107-232226
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
patch link:    https://lore.kernel.org/r/20261107175339.3547918-4-Vijendar.Mukunda%40amd.com
patch subject: [PATCH 3/4] ASoC: SOF: amd: guard SDW IRQ work scheduling with interrupt_enabled flag
config: x86_64-rhel-9.4-rust (https://download.01.org/0day-ci/archive/20261108/[email protected]/config)
compiler: clang version 22.1.8 (https://github.com/llvm/llvm-project ca7933e47d3a3451d81e72ac174dcb5aa28b59d1)
rustc: rustc 1.96.0 (ac68faa20 2026-05-25)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20261108/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

>> sound/soc/sof/amd/acp.c:589:36: error: no member named 'interrupt_enabled' in 'struct amd_sdw_manager'

589 | if (amd_manager && amd_manager->interrupt_enabled)
| ~~~~~~~~~~~ ^
sound/soc/sof/amd/acp.c:621:37: error: no member named 'interrupt_enabled' in 'struct amd_sdw_manager'
621 | if (amd_manager && amd_manager->interrupt_enabled)
| ~~~~~~~~~~~ ^
2 errors generated.

vim +589 sound/soc/sof/amd/acp.c

566
567 static irqreturn_t acp_irq_handler(int irq, void *dev_id)
568 {
569 struct amd_sdw_manager *amd_manager;
570 struct snd_sof_dev *sdev = dev_id;
571 const struct sof_amd_acp_desc *desc = get_chip_info(sdev->pdata);
572 struct acp_dev_data *adata = sdev->pdata->hw_pdata;
573 unsigned int base = desc->dsp_intr_base;
574 unsigned int val;
575 int irq_flag = 0, wake_irq_flag = 0;
576
577 val = snd_sof_dsp_read(sdev, ACP_DSP_BAR, base + DSP_SW_INTR_STAT_OFFSET);
578 if (val & ACP_DSP_TO_HOST_IRQ) {
579 snd_sof_dsp_write(sdev, ACP_DSP_BAR, base + DSP_SW_INTR_STAT_OFFSET,
580 ACP_DSP_TO_HOST_IRQ);
581 return IRQ_WAKE_THREAD;
582 }
583
584 val = snd_sof_dsp_read(sdev, ACP_DSP_BAR, desc->ext_intr_stat);
585 if (val & ACP_SDW0_IRQ_MASK) {
586 snd_sof_dsp_write(sdev, ACP_DSP_BAR, desc->ext_intr_stat, ACP_SDW0_IRQ_MASK);
587 if (adata->sdw) {
588 amd_manager = dev_get_drvdata(&adata->sdw->pdev[0]->dev);

 > 589				if (amd_manager && amd_manager->interrupt_enabled)

590 schedule_work(&amd_manager->amd_sdw_irq_thread);
591 }
592 irq_flag = 1;
593 }
594
595 if (val & ACP_ERROR_IRQ_MASK) {
596 snd_sof_dsp_write(sdev, ACP_DSP_BAR, desc->ext_intr_stat, ACP_ERROR_IRQ_MASK);
597 snd_sof_dsp_write(sdev, ACP_DSP_BAR, desc->acp_sw0_i2s_err_reason, 0);
598 /* ACP_SW1_I2S_ERROR_REASON is newly added register from rmb platform onwards */
599 switch (adata->pci_rev) {
600 case ACP_RMB_PCI_ID:
601 case ACP63_PCI_ID:
602 case ACP70_PCI_ID:
603 case ACP71_PCI_ID:
604 case ACP72_PCI_ID:
605 snd_sof_dsp_write(sdev, ACP_DSP_BAR, ACP_SW1_I2S_ERROR_REASON, 0);
606 break;
607 default:
608 break;
609 }
610 snd_sof_dsp_write(sdev, ACP_DSP_BAR, desc->acp_error_stat, 0);
611 irq_flag = 1;
612 }
613
614 if (desc->ext_intr_stat1) {
615 val = snd_sof_dsp_read(sdev, ACP_DSP_BAR, desc->ext_intr_stat1);
616 if (val & ACP_SDW1_IRQ_MASK) {
617 snd_sof_dsp_write(sdev, ACP_DSP_BAR, desc->ext_intr_stat1,
618 ACP_SDW1_IRQ_MASK);
619 if (adata->sdw) {
620 amd_manager = dev_get_drvdata(&adata->sdw->pdev[1]->dev);
621 if (amd_manager && amd_manager->interrupt_enabled)
622 schedule_work(&amd_manager->amd_sdw_irq_thread);
623 }
624 irq_flag = 1;
625 }
626 switch (adata->pci_rev) {
627 case ACP70_PCI_ID:
628 case ACP71_PCI_ID:
629 case ACP72_PCI_ID:
630 wake_irq_flag = amd_sof_check_and_handle_acp70_sdw_wake_irq(sdev);
631 break;
632 }
633 }
634 if (irq_flag || wake_irq_flag)
635 return IRQ_HANDLED;
636 else
637 return IRQ_NONE;
638 }
639

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

author maintainer
#12 lore raw

Re: [PATCH 3/4] ASoC: SOF: amd: guard SDW IRQ work scheduling with interrupt_enabled flag

Quoted 19 lines · kernel test robot

Hi Vijendar,

kernel test robot noticed the following build errors:

[auto build test ERROR on broonie-sound/for-next]
[also build test ERROR on vkoul-dmaengine/next linus/master v7.4-rc1 next-20261105] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Vijendar-Mukunda/ASoC-amd-ps-guard-adata-sdw-dereferences-with-NULL-check-in-IRQ-handler/20261107-232226
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
patch link:    https://lore.kernel.org/r/20261107175339.3547918-4-Vijendar.Mukunda%40amd.com
patch subject: [PATCH 3/4] ASoC: SOF: amd: guard SDW IRQ work scheduling with interrupt_enabled flag
config: x86_64-rhel-9.4-rust (https://download.01.org/0day-ci/archive/20261108/[email protected]/config)
compiler: clang version 22.1.8 (https://github.com/llvm/llvm-project ca7933e47d3a3451d81e72ac174dcb5aa28b59d1)
rustc: rustc 1.96.0 (ac68faa20 2026-05-25)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20261108/[email protected]/reproduce)
Quoted 8 lines

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

Quoted 1 line
Quoted 1 line

sound/soc/sof/amd/acp.c:589:36: error: no member named 'interrupt_enabled' in 'struct amd_sdw_manager'

As mentioned in cover letter, Patch 3/4 & 4/4 has dependency on below patch which introduces the interrupt_enabled field in the AMD SoundWire manager:

https://lore.kernel.org/linux-sound/[email protected]/
This patch series was prepared based on ASoC tree.
Will combine these asoc patches along with Soundwire patches and will re spin the patch series.

Quoted 88 lines

589 | if (amd_manager && amd_manager->interrupt_enabled)
| ~~~~~~~~~~~ ^
sound/soc/sof/amd/acp.c:621:37: error: no member named 'interrupt_enabled' in 'struct amd_sdw_manager'
621 | if (amd_manager && amd_manager->interrupt_enabled)
| ~~~~~~~~~~~ ^
2 errors generated.

vim +589 sound/soc/sof/amd/acp.c

   566	
   567	static irqreturn_t acp_irq_handler(int irq, void *dev_id)
   568	{
   569		struct amd_sdw_manager *amd_manager;
   570		struct snd_sof_dev *sdev = dev_id;
   571		const struct sof_amd_acp_desc *desc = get_chip_info(sdev->pdata);
   572		struct acp_dev_data *adata = sdev->pdata->hw_pdata;
   573		unsigned int base = desc->dsp_intr_base;
   574		unsigned int val;
   575		int irq_flag = 0, wake_irq_flag = 0;
   576	
   577		val = snd_sof_dsp_read(sdev, ACP_DSP_BAR, base + DSP_SW_INTR_STAT_OFFSET);
   578		if (val & ACP_DSP_TO_HOST_IRQ) {
   579			snd_sof_dsp_write(sdev, ACP_DSP_BAR, base + DSP_SW_INTR_STAT_OFFSET,
   580					  ACP_DSP_TO_HOST_IRQ);
   581			return IRQ_WAKE_THREAD;
   582		}
   583	
   584		val = snd_sof_dsp_read(sdev, ACP_DSP_BAR, desc->ext_intr_stat);
   585		if (val & ACP_SDW0_IRQ_MASK) {
   586			snd_sof_dsp_write(sdev, ACP_DSP_BAR, desc->ext_intr_stat, ACP_SDW0_IRQ_MASK);
   587			if (adata->sdw) {
   588				amd_manager = dev_get_drvdata(&adata->sdw->pdev[0]->dev);
Quoted 1 line

589 if (amd_manager && amd_manager->interrupt_enabled)

   590					schedule_work(&amd_manager->amd_sdw_irq_thread);
   591			}
   592			irq_flag = 1;
   593		}
   594	
   595		if (val & ACP_ERROR_IRQ_MASK) {
   596			snd_sof_dsp_write(sdev, ACP_DSP_BAR, desc->ext_intr_stat, ACP_ERROR_IRQ_MASK);
   597			snd_sof_dsp_write(sdev, ACP_DSP_BAR, desc->acp_sw0_i2s_err_reason, 0);
   598			/* ACP_SW1_I2S_ERROR_REASON is newly added register from rmb platform onwards */
   599			switch (adata->pci_rev) {
   600			case ACP_RMB_PCI_ID:
   601			case ACP63_PCI_ID:
   602			case ACP70_PCI_ID:
   603			case ACP71_PCI_ID:
   604			case ACP72_PCI_ID:
   605				snd_sof_dsp_write(sdev, ACP_DSP_BAR, ACP_SW1_I2S_ERROR_REASON, 0);
   606				break;
   607			default:
   608				break;
   609			}
   610			snd_sof_dsp_write(sdev, ACP_DSP_BAR, desc->acp_error_stat, 0);
   611			irq_flag = 1;
   612		}
   613	
   614		if (desc->ext_intr_stat1) {
   615			val = snd_sof_dsp_read(sdev, ACP_DSP_BAR, desc->ext_intr_stat1);
   616			if (val & ACP_SDW1_IRQ_MASK) {
   617				snd_sof_dsp_write(sdev, ACP_DSP_BAR, desc->ext_intr_stat1,
   618						  ACP_SDW1_IRQ_MASK);
   619				if (adata->sdw) {
   620					amd_manager = dev_get_drvdata(&adata->sdw->pdev[1]->dev);
   621					if (amd_manager && amd_manager->interrupt_enabled)
   622						schedule_work(&amd_manager->amd_sdw_irq_thread);
   623				}
   624				irq_flag = 1;
   625			}
   626			switch (adata->pci_rev) {
   627			case ACP70_PCI_ID:
   628			case ACP71_PCI_ID:
   629			case ACP72_PCI_ID:
   630				wake_irq_flag = amd_sof_check_and_handle_acp70_sdw_wake_irq(sdev);
   631				break;
   632			}
   633		}
   634		if (irq_flag || wake_irq_flag)
   635			return IRQ_HANDLED;
   636		else
   637			return IRQ_NONE;
   638	}
   639

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

author maintainer
#13 lore raw

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

In reply to Vijendar Mukunda

acp63_irq_handler() called schedule_work() for the SoundWire manager workqueue guarded only by a NULL pointer check on amd_manager. Add a check for amd_manager->interrupt_enabled so that an in-flight interrupt that fires while the SoundWire manager is being torn down does not queue new work after amd_disable_sdw_interrupts() has cleared the flag.

Reported-by Lee Jones <[email protected]>
Closes https://sashiko.dev/#/bug/linux-ab458b2e-f505-4a41-8238-a93ceb5d9c77
Fixes e1cb350610ce ("ASoC: amd: ps: handle SoundWire interrupts in acp pci driver")
Signed-off-by Vijendar Mukunda <[email protected]>
---
 sound/soc/amd/ps/pci-ps.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
sound/soc/amd/ps/pci-ps.c +2 -2
index d0b66df10157..0442010e969c 100644
@@ -215,7 +215,7 @@ static irqreturn_t acp63_irq_handler(int irq, void *dev_id)
 		writel(ACP_SDW0_STAT, adata->acp63_base + ACP_EXTERNAL_INTR_STAT);
 		if (adata->sdw) {
 			amd_manager = 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 = 1;

@@ -226,7 +226,7 @@ static irqreturn_t acp63_irq_handler(int irq, void *dev_id)
 		writel(ACP_SDW1_STAT, adata->acp63_base + ACP_EXTERNAL_INTR_STAT1);
 		if (adata->sdw) {
 			amd_manager = 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 = 1;
Signature · 1 line
2.53.0
AI review
#14 lore raw

Re: [PATCH 4/4] ASoC: amd: ps: guard SDW IRQ work scheduling with interrupt_enabled flag

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…

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 <[email protected]>

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.

Quoted 14 lines
sound/soc/amd/ps/pci-ps.c +1 -1
index d0b66df101579..0442010e969c0 100644
@@ -215,7 +215,7 @@ static irqreturn_t acp63_irq_handler(int irq, void *dev_id)
 		writel(ACP_SDW0_STAT, adata->acp63_base + ACP_EXTERNAL_INTR_STAT);
 		if (adata->sdw) {
 			amd_manager = 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 = 1;

sound/soc/amd/ps/pci-ps.c static irqreturn_t acp63_irq_handler(int irq, void *dev_id) · line 232

Quoted 8 lines
sound/soc/amd/ps/pci-ps.c +1 -1
@@ -226,7 +226,7 @@ static irqreturn_t acp63_irq_handler(int irq, void *dev_id)
 		writel(ACP_SDW1_STAT, adata->acp63_base + ACP_EXTERNAL_INTR_STAT1);
 		if (adata->sdw) {
 			amd_manager = 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 = 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 = dev_get_drvdata(...)
                                          if (amd_manager->interrupt_enabled)

amd_disable_sdw_interrupts()
interrupt_enabled = false
schedule_work(&amd_sdw_irq_thread)
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 locklessly 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.

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
author maintainer
#15 lore raw

Re: [PATCH 4/4] ASoC: amd: ps: guard SDW IRQ work scheduling with interrupt_enabled flag

Quoted 36 lines · On 11/7/26 23:42, [email protected] wrote:

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 <[email protected]>

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.

Quoted 20 lines
sound/soc/amd/ps/pci-ps.c +2 -2
index d0b66df101579..0442010e969c0 100644
@@ -215,7 +215,7 @@ static irqreturn_t acp63_irq_handler(int irq, void *dev_id)
  		writel(ACP_SDW0_STAT, adata->acp63_base + ACP_EXTERNAL_INTR_STAT);
  		if (adata->sdw) {
  			amd_manager = 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 = 1;

@@ -226,7 +226,7 @@ static irqreturn_t acp63_irq_handler(int irq, void *dev_id)
  		writel(ACP_SDW1_STAT, adata->acp63_base + ACP_EXTERNAL_INTR_STAT1);
  		if (adata->sdw) {
  			amd_manager = 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);
Quoted 8 lines

sound/soc/amd/ps/pci-ps.c static irqreturn_t acp63_irq_handler(int irq, void *dev_id) · line 232

Quoted 2 lines
sound/soc/amd/ps/pci-ps.c +0 -0
  		}
  		irq_flag = 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.

The interrupt_enabled member is added by patch 17/25 of the AMD SoundWire fix series which this ASoC series depends on. The dependency is stated in the cover letter of this series. When both series are applied together the build is clean.

Dependency:
https://lore.kernel.org/linux-sound/[email protected]/

Quoted 17 lines

[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 = dev_get_drvdata(...)
                                           if (amd_manager->interrupt_enabled)

amd_disable_sdw_interrupts()
interrupt_enabled = false
schedule_work(&amd_sdw_irq_thread)
device_unbind_cleanup()
devres_release_all() /* frees amd_manager */ dev_set_drvdata(dev, NULL)

Quoted 8 lines
                                         amd_sdw_irq_thread()
                                           /* dereferences freed amd_manager */
                                             <-- use-after-free

acp63_irq_handler() checks dev_get_drvdata() and interrupt_enabled locklessly 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.

The remove ordering shown above is the state before the fix series. Patch 14/25 of the AMD SoundWire fix series corrects the remove sequence to:

  amd_disable_sdw_interrupts()       /* clears interrupt_enabled, masks hardware IRQ source */ cancel_work_sync(&amd_sdw_irq_thread) cancel_work_sync(&amd_sdw_work)

Once SoundWire interrupts are masked at the hardware level, no new IRQ deliveries can occur. An IRQ handler already in flight before the mask took effect may still queue amd_sdw_irq_thread, and cancel_work_sync() drains both amd_sdw_irq_thread and any amd_sdw_work items it may have re-queued. synchronize_irq() is therefore not required: the hardware mask bounds the window, and cancel_work_sync() covers any work queued by the in-flight handler.

Quoted 1 line
build bot
#16 lore raw

Re: [PATCH 4/4] ASoC: amd: ps: guard SDW IRQ work scheduling with interrupt_enabled flag

In reply to Vijendar Mukunda
kernel test robot noticed the following build errors:

Hi Vijendar,

kernel test robot noticed the following build errors:

[auto build test ERROR on broonie-sound/for-next]
[also build test ERROR on vkoul-dmaengine/next linus/master v7.4-rc1 next-20261105] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Vijendar-Mukunda/ASoC-amd-ps-guard-adata-sdw-dereferences-with-NULL-check-in-IRQ-handler/20261107-232226
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
patch link:    https://lore.kernel.org/r/20261107175339.3547918-5-Vijendar.Mukunda%40amd.com
patch subject: [PATCH 4/4] ASoC: amd: ps: guard SDW IRQ work scheduling with interrupt_enabled flag
config: x86_64-rhel-9.4-ltp (https://download.01.org/0day-ci/archive/20261108/[email protected]/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20261108/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

sound/soc/amd/ps/pci-ps.c: In function 'acp63_irq_handler':

>> sound/soc/amd/ps/pci-ps.c:218:55: error: 'struct amd_sdw_manager' has no member named 'interrupt_enabled'
     218 |                         if (amd_manager && amd_manager->interrupt_enabled)
         |                                                       ^~
   sound/soc/amd/ps/pci-ps.c:229:55: error: 'struct amd_sdw_manager' has no member named 'interrupt_enabled'
     229 |                         if (amd_manager && amd_manager->interrupt_enabled)
         |                                                       ^~

vim +218 sound/soc/amd/ps/pci-ps.c

194
195 static irqreturn_t acp63_irq_handler(int irq, void *dev_id)
196 {
197 struct acp63_dev_data *adata;
198 struct pdm_dev_data *ps_pdm_data;
199 struct amd_sdw_manager *amd_manager;
200 u32 ext_intr_stat, ext_intr_stat1;
201 u16 irq_flag = 0;
202 u16 wake_irq_flag = 0;
203 u16 sdw_dma_irq_flag = 0;
204
205 adata = dev_id;
206 if (!adata)
207 return IRQ_NONE;
208 /* ACP interrupts will be cleared by reading particular bit and writing
209 * same value to the status register. writing zero's doesn't have any
210 * effect.
211 * Bit by bit checking of IRQ field is implemented.
212 */
213 ext_intr_stat = readl(adata->acp63_base + ACP_EXTERNAL_INTR_STAT);
214 if (ext_intr_stat & ACP_SDW0_STAT) {
215 writel(ACP_SDW0_STAT, adata->acp63_base + ACP_EXTERNAL_INTR_STAT);
216 if (adata->sdw) {
217 amd_manager = dev_get_drvdata(&adata->sdw->pdev[0]->dev);

 > 218				if (amd_manager && amd_manager->interrupt_enabled)

219 schedule_work(&amd_manager->amd_sdw_irq_thread);
220 }
221 irq_flag = 1;
222 }
223
224 ext_intr_stat1 = readl(adata->acp63_base + ACP_EXTERNAL_INTR_STAT1);
225 if (ext_intr_stat1 & ACP_SDW1_STAT) {
226 writel(ACP_SDW1_STAT, adata->acp63_base + ACP_EXTERNAL_INTR_STAT1);
227 if (adata->sdw) {
228 amd_manager = dev_get_drvdata(&adata->sdw->pdev[1]->dev);
229 if (amd_manager && amd_manager->interrupt_enabled)
230 schedule_work(&amd_manager->amd_sdw_irq_thread);
231 }
232 irq_flag = 1;
233 }
234
235 if (ext_intr_stat & ACP_ERROR_IRQ) {
236 writel(ACP_ERROR_IRQ, adata->acp63_base + ACP_EXTERNAL_INTR_STAT);
237 /* TODO: Report SoundWire Manager instance errors */
238 writel(0, adata->acp63_base + ACP_SW0_I2S_ERROR_REASON);
239 writel(0, adata->acp63_base + ACP_SW1_I2S_ERROR_REASON);
240 writel(0, adata->acp63_base + ACP_ERROR_STATUS);
241 irq_flag = 1;
242 }
243
244 if (adata->acp_rev >= ACP70_PCI_REV)
245 wake_irq_flag = check_and_handle_acp70_sdw_wake_irq(adata);
246
247 if (ext_intr_stat & BIT(PDM_DMA_STAT)) {
248 ps_pdm_data = dev_get_drvdata(&adata->pdm_dev->dev);
249 writel(BIT(PDM_DMA_STAT), adata->acp63_base + ACP_EXTERNAL_INTR_STAT);
250 if (ps_pdm_data->capture_stream)
251 snd_pcm_period_elapsed(ps_pdm_data->capture_stream);
252 irq_flag = 1;
253 }
254
255 sdw_dma_irq_flag = check_and_handle_sdw_dma_irq(adata, ext_intr_stat, ext_intr_stat1);
256 if (sdw_dma_irq_flag)
257 return IRQ_WAKE_THREAD;
258
259 if (irq_flag || wake_irq_flag)
260 return IRQ_HANDLED;
261 else
262 return IRQ_NONE;
263 }
264

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

author maintainer
#17 lore raw
In reply to Vijendar Mukunda
Quoted 14 lines · Vijendar Mukunda

These four patches fix two related race conditions in the AMD SoundWire IRQ handlers for the PS (ACP63) and SOF platforms.

Patches 1 and 2 guard against a NULL dereference in the ACP external IRQ handler. The handler is registered before sdw_amd_probe() assigns adata->sdw, so a stray interrupt in the probe window, or one that fires after sdw_amd_exit() frees the context, can reach the adata->sdw->pdev[i] dereference with adata->sdw still NULL.

Patches 3 and 4 guard schedule_work() calls with the interrupt_enabled flag so that an in-flight IRQ arriving while the SoundWire manager is being torn down does not queue new work after amd_disable_sdw_interrupts() has cleared the flag.

Quoted 7 lines

NOTE: Patches 3 and 4 depend on the following patch which introduces the interrupt_enabled field in the AMD SoundWire manager:

https://lore.kernel.org/linux-sound/[email protected]/

Until that patch is merged, patches 3 and 4 should not be picked up independently.

Hi Mark, Vinod,

The following two series have a build and correctness dependency on each other:

SoundWire fix series (25 patches):
https://lore.kernel.org/linux-sound/[email protected]/

Companion ASoC series (4 patches):
https://lore.kernel.org/alsa-devel/[email protected]/

ASoC patches 1/4 and 2/4 reference the interrupt_enabled member of struct amd_sdw_manager introduced by SoundWire patch 17/25, so the ASoC patches will not build without the SoundWire series. SoundWire patch 18/25 delays *sdw_ctx publication until after startup succeeds, opening a probe-window race in acp63_irq_handler() and acp_irq_handler() that ASoC patches 3/4 and 4/4 close with adata->sdw NULL checks.

Because the two series are split across trees, the Sashiko AI review tool sees each series in isolation and raises the cross-tree dependency as unresolved races:

https://sashiko.dev/#/patchset/20261107154659.3358223-1-Vijendar.Mukunda%40amd.com
https://sashiko.dev/#/patchset/20261107175339.3547918-1-Vijendar.Mukunda%40amd.com

When both series were combined and run through Sashiko as a single patch set, none of these cross-dependency issues were reported, confirming that the concerns are an artifact of the split rather than real bugs. Reposting as a single combined series would give Sashiko the full context and make the review cycle easier. Please let us know if that is the preferred approach.

Thanks,
Vijendar

Quoted 16 lines

Vijendar Mukunda (4):
ASoC: amd: ps: guard adata->sdw dereferences with NULL check in IRQ
handler
ASoC: SOF: amd: guard adata->sdw dereferences with NULL check in IRQ
handler
ASoC: SOF: amd: guard SDW IRQ work scheduling with interrupt_enabled
flag
ASoC: amd: ps: guard SDW IRQ work scheduling with interrupt_enabled
flag

 sound/soc/amd/ps/pci-ps.c | 32 ++++++++++++++++++++------------
 sound/soc/sof/amd/acp.c   | 32 ++++++++++++++++++++------------
 2 files changed, 40 insertions(+), 24 deletions(-)

--
2.53.0

author maintainer
#18 lore raw
Quoted 22 lines · Mukunda,Vijendar
Quoted 14 lines · Vijendar Mukunda

These four patches fix two related race conditions in the AMD SoundWire IRQ handlers for the PS (ACP63) and SOF platforms.

Patches 1 and 2 guard against a NULL dereference in the ACP external IRQ handler. The handler is registered before sdw_amd_probe() assigns adata->sdw, so a stray interrupt in the probe window, or one that fires after sdw_amd_exit() frees the context, can reach the adata->sdw->pdev[i] dereference with adata->sdw still NULL.

Patches 3 and 4 guard schedule_work() calls with the interrupt_enabled flag so that an in-flight IRQ arriving while the SoundWire manager is being torn down does not queue new work after amd_disable_sdw_interrupts() has cleared the flag.

Quoted 7 lines

NOTE: Patches 3 and 4 depend on the following patch which introduces the interrupt_enabled field in the AMD SoundWire manager:

https://lore.kernel.org/linux-sound/[email protected]/

Until that patch is merged, patches 3 and 4 should not be picked up independently.

Quoted 12 lines

Hi Mark, Vinod,

The following two series have a build and correctness dependency on each other:

SoundWire fix series (25 patches):
https://lore.kernel.org/linux-sound/[email protected]/

Companion ASoC series (4 patches):
https://lore.kernel.org/alsa-devel/[email protected]/

ASoC patches 1/4 and 2/4 reference the interrupt_enabled member of

I was referring to the patch ordering in my local copy, which differs from the series that was posted. In the posted series, the correct references are 3/4 and 4/4 for the |interrupt_enabled| dependency, and 1/4 and 2/4 for the |adata->sdw| NULL checks.

Quoted 6 lines

struct amd_sdw_manager introduced by SoundWire patch 17/25, so the ASoC patches will not build without the SoundWire series. SoundWire patch 18/25 delays *sdw_ctx publication until after startup succeeds, opening a probe-window race in acp63_irq_handler() and acp_irq_handler() that ASoC patches 3/4 and 4/4 close with adata->sdw NULL checks.

Correction to my previous reply: I quoted the wrong patch numbers.

In the posted series, patches 3/4 and 4/4 have the interrupt_enabled dependency, while patches 1/4 and 2/4 contain the adata->sdw NULL checks.

Sorry for the confusion.

Quoted 35 lines

Because the two series are split across trees, the Sashiko AI review tool sees each series in isolation and raises the cross-tree dependency as unresolved races:

https://sashiko.dev/#/patchset/20261107154659.3358223-1-Vijendar.Mukunda%40amd.com
https://sashiko.dev/#/patchset/20261107175339.3547918-1-Vijendar.Mukunda%40amd.com

When both series were combined and run through Sashiko as a single patch set, none of these cross-dependency issues were reported, confirming that the concerns are an artifact of the split rather than real bugs. Reposting as a single combined series would give Sashiko the full context and make the review cycle easier. Please let us know if that is the preferred approach.

Thanks,
Vijendar

Quoted 16 lines

Vijendar Mukunda (4):
ASoC: amd: ps: guard adata->sdw dereferences with NULL check in IRQ
handler
ASoC: SOF: amd: guard adata->sdw dereferences with NULL check in IRQ
handler
ASoC: SOF: amd: guard SDW IRQ work scheduling with interrupt_enabled
flag
ASoC: amd: ps: guard SDW IRQ work scheduling with interrupt_enabled
flag

 sound/soc/amd/ps/pci-ps.c | 32 ++++++++++++++++++++------------
 sound/soc/sof/amd/acp.c   | 32 ++++++++++++++++++++------------
 2 files changed, 40 insertions(+), 24 deletions(-)

--
2.53.0

maintainer
#19 lore raw
In reply to Vijendar Mukunda
Quoted 5 lines

Patches 3 and 4 guard schedule_work() calls with the interrupt_enabled flag so that an in-flight IRQ arriving while the SoundWire manager is being torn down does not queue new work after amd_disable_sdw_interrupts() has cleared the flag.

Quoted 7 lines

NOTE: Patches 3 and 4 depend on the following patch which introduces the interrupt_enabled field in the AMD SoundWire manager:

https://lore.kernel.org/linux-sound/[email protected]/

Until that patch is merged, patches 3 and 4 should not be picked up independently.

Please try to avoid mixing in things with differing dependencies into a single series, it makes things much harder to track.

Signature · 1 line
Mark Brown <[email protected]>