mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] ASoC: wm8962: Prevent mic_work rearm during removal
@ 2026-09-21 23:57 Myeonghun Pak
  2026-09-22  8:55 ` Charles Keepax
  0 siblings, 1 reply; 2+ messages in thread
From: Myeonghun Pak @ 2026-09-21 23:57 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai
  Cc: Myeonghun Pak, patches, linux-sound, linux-kernel, stable, Ijae Kim

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);
 }
 

base-commit: 238650ef6c7c7cca08e032527329424c9fbd70e5
-- 
2.53.0


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] ASoC: wm8962: Prevent mic_work rearm during removal
  2026-09-21 23:57 [PATCH] ASoC: wm8962: Prevent mic_work rearm during removal Myeonghun Pak
@ 2026-09-22  8:55 ` Charles Keepax
  0 siblings, 0 replies; 2+ messages in thread
From: Charles Keepax @ 2026-09-22  8:55 UTC (permalink / raw)
  To: Myeonghun Pak
  Cc: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
	patches, linux-sound, linux-kernel, stable, Ijae Kim

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-09-22  8:55 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-21 23:57 [PATCH] ASoC: wm8962: Prevent mic_work rearm during removal Myeonghun Pak
2026-09-22  8:55 ` Charles Keepax

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®