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

[sashiko-bot@kernel.org]

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 <Vijendar.Mukunda@amd.com>

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().

> diff --git a/sound/soc/sof/amd/acp.c b/sound/soc/sof/amd/acp.c
> index 623e016f38b83..5c4eab0be497c 100644
> --- a/sound/soc/sof/amd/acp.c
> +++ b/sound/soc/sof/amd/acp.c
> @@ -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;

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261107175339.3547918-1-Vijendar.Mukunda@amd.com?part=3