ASoC: wm8962: Prevent mic_work rearm during removal

PATCH In review Myeonghun Pak 2 participants 2 messages Sep 21 – Sep 22

Subsystems (4)

Stable branch Sound ASoC Wolfson microelectronics drivers

lore

Share & tools

lore

Myeonghun Pak

author

2026-09-21 23:57 UTC #0 lore raw

PATCH 1/1 ASoC: wm8962: Prevent mic_work rearm during removal

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

Assisted-by LLM

Co-developed-by Ijae Kim <[email protected]>

Signed-off-by Ijae Kim <[email protected]>

Signed-off-by Myeonghun Pak <[email protected]>

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

sound/soc/codecs/wm8962.c +10 -0

index 8a9598161b35..210ec96fab4c 100644
@@ -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

Signature · 1 line

2.53.0

Charles Keepax

2026-09-22 08:55 UTC #1 lore raw

Quoted 45 lines · Myeonghun Pak

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 [email protected]
Assisted-by LLM
Co-developed-by Ijae Kim <[email protected]>
Signed-off-by Ijae Kim <[email protected]>
Signed-off-by Myeonghun Pak <[email protected]>
---
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(+)
sound/soc/codecs/wm8962.c +2 -0
index 8a9598161b35..210ec96fab4c 100644
@@ -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);
+

sound/soc/codecs/wm8962.c static int wm8962_i2c_probe(struct i2c_client *i2c) · line 3904

Quoted 10 lines

sound/soc/codecs/wm8962.c +8 -0

+	/*
+	 * 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