Re: [PATCH] ASoC: wm8962: Prevent mic_work rearm during removal

[Charles Keepax <ckeepax@opensource.cirrus.com>]

On Mon, Sep 21, 2026 at 07:57:24PM -0400, Myeonghun Pak wrote:
> wm8962_remove() cancels mic_work, but on unbind that cancel runs
> before the interrupt that queues it is released.
> 
> wm8962_i2c_probe() requests the interrupt with
> devm_request_threaded_irq() and registers the component afterwards.
> devres releases in LIFO order, so wm8962_remove() runs before
> free_irq().  A MICD or MICSCD event in between makes wm8962_irq()
> queue mic_work again, and the delayed work then calls
> snd_soc_component_read() and snd_soc_jack_report() on the freed
> component.
> 
> Commit ca50410b731c ("ASoC: wm8962: Move interrupt initalisation to
> probe()") dropped the free_irq() that used to precede
> cancel_delayed_work_sync() in wm8962_remove().
> 
> Disable the interrupt in wm8962_i2c_remove(), which runs before devres
> release.  disable_irq() waits for the threaded handler, and the
> existing cancel then drains mic_work with no producer left.
> 
> This issue was identified during our ongoing static-analysis research
> while reviewing kernel code.
> 
> Fixes: ca50410b731c ("ASoC: wm8962: Move interrupt initalisation to probe()")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Co-developed-by: Ijae Kim <ae878000@gmail.com>
> Signed-off-by: Ijae Kim <ae878000@gmail.com>
> Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
> ---
> Found by inspection; I do not have the hardware, so this is not runtime
> tested.
> 
>  sound/soc/codecs/wm8962.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/sound/soc/codecs/wm8962.c b/sound/soc/codecs/wm8962.c
> index 8a9598161b35..210ec96fab4c 100644
> --- a/sound/soc/codecs/wm8962.c
> +++ b/sound/soc/codecs/wm8962.c
> @@ -3890,6 +3890,16 @@ static int wm8962_i2c_probe(struct i2c_client *i2c)
>  
>  static void wm8962_i2c_remove(struct i2c_client *client)
>  {
> +	struct wm8962_priv *wm8962 = i2c_get_clientdata(client);
> +
> +	/*
> +	 * The IRQ is devm-managed, so it is freed only after the component
> +	 * has been unregistered and wm8962_remove() has already cancelled
> +	 * mic_work.  Silence the producer here instead.
> +	 */
> +	if (wm8962->irq)
> +		disable_irq(wm8962->irq);
> +
>  	pm_runtime_disable(&client->dev);
>  }

Something seems a little off here. We are disabling the IRQ in
wm8962_i2c_remove() but what was the point of cancelling the work
in wm8962_remove()? A new one can reassert in the time between
them. This feels like maybe this is not the correct fix. Step 1
is probably to decide is it reasonable for the IRQ to run whilst
the component is not probed? If that is fine then probably
putting the cancel_work into devm_ makes sense, if that isn't
fine then perhaps the IRQ should actually be unregistered on
component remove rather than devm_.

Thanks,
Charles