Re: [PATCH 16/21] soundwire: amd: Initialise and cancel IRQ work around manager lifetime

[Mukunda,Vijendar]

On 10/30/26 16:16, Lee Jones wrote:
> [You don't often get email from lee@xxxxxxxxxx. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> amd_sdw_manager_probe() publishes amd_manager via dev_set_drvdata()
> before initialising amd_sdw_irq_thread and amd_sdw_work, allowing an
> early interrupt to schedule uninitialised work.  During removal,
> amd_sdw_manager_remove() only cancels amd_sdw_work without first
> cancelling amd_sdw_irq_thread, allowing deferred IRQ work to re-queue
> amd_sdw_work or dereference amd_manager after it is freed.
>
> Initialise both work items before calling dev_set_drvdata() in
> amd_sdw_manager_probe(), and cancel amd_sdw_irq_thread before
> amd_sdw_work in amd_sdw_manager_remove().
>
> Fixes: 65f93e4096a0 ("soundwire: amd: add SoundWire manager interrupt handling")
> Closes: https://sashiko.dev/#/bug/linux-9890e8dc-4b7d-44d3-bfb0-171dc421e28c
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Lee Jones <lee@xxxxxxxxxx>
> ---
>   drivers/soundwire/amd_manager.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soundwire/amd_manager.c b/drivers/soundwire/amd_manager.c
> index 98b09d81f3b8..2e1038ea0f36 100644
> --- a/drivers/soundwire/amd_manager.c
> +++ b/drivers/soundwire/amd_manager.c
> @@ -1185,9 +1185,9 @@ static int amd_sdw_manager_probe(struct platform_device *pdev)
>                  sdw_bus_master_delete(&amd_manager->bus);
>                  return ret;
>          }
> -       dev_set_drvdata(dev, amd_manager);
>          INIT_WORK(&amd_manager->amd_sdw_irq_thread, amd_sdw_irq_thread);
>          INIT_WORK(&amd_manager->amd_sdw_work, amd_sdw_update_slave_status_work);
> +       dev_set_drvdata(dev, amd_manager);
The probe-side ordering issue is valid — an interrupt arriving after
dev_set_drvdata() but before INIT_WORK() could schedule work on an
uninitialised work_struct.

The remove-side fix is already posted as part of our v3 patch series:

https://lore.kernel.org/alsa-devel/20261013085111.2009061-6-Vijendar.Mukunda@xxxxxxx/T/#m60177687bb84758efdd6f5f09eba8ad32f0e4631

The probe-side fix will be included in our upcoming v4 patch series,
which contains major driver restructuring and customer-critical bug
fixes. Merging this patch independently would conflict with v4 and
complicate the review process.

Could you please drop this patch? Both issues will be covered by our
v3/v4 series.

>          return 0;
>   }
>
> @@ -1197,6 +1197,7 @@ static void amd_sdw_manager_remove(struct platform_device *pdev)
>          int ret;
>
>          pm_runtime_disable(&pdev->dev);
> +       cancel_work_sync(&amd_manager->amd_sdw_irq_thread);
proper fix already posted as part of v3 version.
https://lore.kernel.org/alsa-devel/20261013085111.2009061-6-Vijendar.Mukunda@xxxxxxx/T/#m60177687bb84758efdd6f5f09eba8ad32f0e4631

>          cancel_work_sync(&amd_manager->amd_sdw_work);
>          amd_disable_sdw_interrupts(amd_manager);
>          sdw_bus_master_delete(&amd_manager->bus);
> --
> 2.56.0.317.g454f8302e1-goog