[PATCH 0/4] ASoC: cancel pending delayed works on component remove

[PATCH 0/4] ASoC: cancel pending delayed works on component remove

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

Four ASoC codec drivers initialise delayed works that are not
cancelled during component removal.  If the card is unbound while
these works are pending, callbacks fire after DAPM widgets are freed.

rt5668 and rt5659 share the same pattern: the IRQ handler reschedules
work after the codec is reset.  Track IRQ acquisition so that
disable_irq() only quiesces a line the driver owns, then drain all
delayed works before resetting the hardware.

twl6040 frees the plug-detect IRQ but does not drain the work that the
handler schedules.

wm8994 frees IRQs and firmware but does not cancel any of the four
delayed works that IRQ handlers and DAPM events schedule.

Tristan Madani (4):
  ASoC: rt5668: cancel pending delayed works on component remove
  ASoC: rt5659: cancel jack_detect_work on component remove
  ASoC: twl6040: cancel headset jack work on component remove
  ASoC: wm8994: drain all delayed works on component remove

 sound/soc/codecs/rt5659.c  | 17 ++++++++++++++++-
 sound/soc/codecs/rt5659.h  |  2 ++
 sound/soc/codecs/rt5668.c  | 20 +++++++++++++++++++-
 sound/soc/codecs/twl6040.c |  1 +
 sound/soc/codecs/wm8994.c  |  4 ++++
 5 files changed, 42 insertions(+), 2 deletions(-)

-- 
2.47.3

[PATCH 1/4] ASoC: rt5668: cancel pending delayed works on component remove

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

The rt5668 codec initializes two delayed works (jack_detect_work,
jd_check_work) but neither is cancelled during component removal.
If the card is unbound while these works are pending, they fire on
freed resources when snd_soc_jack_report() dereferences the cleared
card pointer.

Track successful IRQ acquisition with a flag so that disable_irq()
only quiesces a line this driver actually owns.  On component remove,
disable the IRQ to stop the handler from rescheduling work, then
drain both delayed works before resetting the codec.  On the next
component probe (card rebind), re-enable the IRQ to restore jack
detection.

Fixes: d59fb2856223 ("ASoC: rt5668: add rt5668B codec driver")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
 sound/soc/codecs/rt5668.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/sound/soc/codecs/rt5668.c b/sound/soc/codecs/rt5668.c
index 4154bad276101..01b14c62fde7f 100644
--- a/sound/soc/codecs/rt5668.c
+++ b/sound/soc/codecs/rt5668.c
@@ -62,6 +62,8 @@ struct rt5668_priv {
 	int pll_out;
 
 	int jack_type;
+	bool irq_acquired;
+	bool irq_disabled;
 };
 
 static const struct reg_default rt5668_reg[] = {
@@ -2213,6 +2215,12 @@ static int rt5668_set_bias_level(struct snd_soc_component *component,
 static int rt5668_probe(struct snd_soc_component *component)
 {
 	struct rt5668_priv *rt5668 = snd_soc_component_get_drvdata(component);
+	struct i2c_client *i2c = to_i2c_client(component->dev);
+
+	if (rt5668->irq_disabled) {
+		enable_irq(i2c->irq);
+		rt5668->irq_disabled = false;
+	}
 
 	rt5668->component = component;
 
@@ -2222,7 +2230,15 @@ static int rt5668_probe(struct snd_soc_component *component)
 static void rt5668_remove(struct snd_soc_component *component)
 {
 	struct rt5668_priv *rt5668 = snd_soc_component_get_drvdata(component);
+	struct i2c_client *i2c = to_i2c_client(component->dev);
 
+	if (rt5668->irq_acquired) {
+		disable_irq(i2c->irq);
+		rt5668->irq_disabled = true;
+	}
+
+	cancel_delayed_work_sync(&rt5668->jack_detect_work);
+	cancel_delayed_work_sync(&rt5668->jd_check_work);
 	rt5668_reset(rt5668->regmap);
 }
 
@@ -2531,7 +2547,9 @@ static int rt5668_i2c_probe(struct i2c_client *i2c)
 		ret = devm_request_threaded_irq(&i2c->dev, i2c->irq, NULL,
 			rt5668_irq, IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING
 			| IRQF_ONESHOT, "rt5668", rt5668);
-		if (ret)
+		if (!ret)
+			rt5668->irq_acquired = true;
+		else
 			dev_err(&i2c->dev, "Failed to request IRQ: %d\n", ret);
 
 	}
-- 
2.47.3

[PATCH 2/4] ASoC: rt5659: cancel jack_detect_work on component remove

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

rt5659_remove() resets the codec via regmap_write() but does not cancel
the pending jack_detect_work. Since the IRQ is registered with
devm_request_threaded_irq(), it remains active until after the component
remove callback returns.  The IRQ handler (rt5659_irq) schedules
jack_detect_work, so a late interrupt can queue work that runs against a
reset codec.

Track successful IRQ acquisition with a flag so that disable_irq()
only quiesces a line this driver actually owns.  On component remove,
disable the IRQ to synchronize with any running handler and prevent
new enqueues, then drain the residual jack_detect_work synchronously
before resetting the hardware.  On the next component probe (card
rebind), re-enable the IRQ to restore jack detection.

Fixes: d3cb2de2479b ("ASoC: rt5659: add rt5659 codec driver")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
 sound/soc/codecs/rt5659.c | 17 ++++++++++++++++-
 sound/soc/codecs/rt5659.h |  2 ++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/sound/soc/codecs/rt5659.c b/sound/soc/codecs/rt5659.c
index 3097ee6d4e89f..340d3e7be7a40 100644
--- a/sound/soc/codecs/rt5659.c
+++ b/sound/soc/codecs/rt5659.c
@@ -3664,6 +3664,12 @@ static int rt5659_probe(struct snd_soc_component *component)
 {
 	struct snd_soc_dapm_context *dapm = snd_soc_component_to_dapm(component);
 	struct rt5659_priv *rt5659 = snd_soc_component_get_drvdata(component);
+	struct i2c_client *i2c = to_i2c_client(component->dev);
+
+	if (rt5659->irq_disabled) {
+		enable_irq(i2c->irq);
+		rt5659->irq_disabled = false;
+	}
 
 	rt5659->component = component;
 
@@ -3684,7 +3690,14 @@ static int rt5659_probe(struct snd_soc_component *component)
 static void rt5659_remove(struct snd_soc_component *component)
 {
 	struct rt5659_priv *rt5659 = snd_soc_component_get_drvdata(component);
+	struct i2c_client *i2c = to_i2c_client(component->dev);
+
+	if (rt5659->irq_acquired) {
+		disable_irq(i2c->irq);
+		rt5659->irq_disabled = true;
+	}
 
+	cancel_delayed_work_sync(&rt5659->jack_detect_work);
 	regmap_write(rt5659->regmap, RT5659_RESET, 0);
 }
 
@@ -4290,7 +4303,9 @@ static int rt5659_i2c_probe(struct i2c_client *i2c)
 		ret = devm_request_threaded_irq(&i2c->dev, i2c->irq, NULL,
 			rt5659_irq, IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING
 			| IRQF_ONESHOT, "rt5659", rt5659);
-		if (ret)
+		if (!ret)
+			rt5659->irq_acquired = true;
+		else
 			dev_err(&i2c->dev, "Failed to request IRQ: %d\n", ret);
 
 		/* Enable IRQ output for GPIO1 pin any way */
diff --git a/sound/soc/codecs/rt5659.h b/sound/soc/codecs/rt5659.h
index b49fd8baf4e7e..4df7d7dd99e16 100644
--- a/sound/soc/codecs/rt5659.h
+++ b/sound/soc/codecs/rt5659.h
@@ -1813,6 +1813,8 @@ struct rt5659_priv {
 	int jack_type;
 	bool hda_hp_plugged;
 	bool hda_mic_plugged;
+	bool irq_acquired;
+	bool irq_disabled;
 };
 
 int rt5659_set_jack_detect(struct snd_soc_component *component,
-- 
2.47.3

[PATCH 3/4] ASoC: twl6040: cancel headset jack work on component remove

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

twl6040_remove() frees the plug detect IRQ but does not cancel the
delayed work that the IRQ handler schedules.  If the work is still
pending after free_irq() returns, it fires on freed memory.

Add cancel_delayed_work_sync() after free_irq() to drain any residual
work.  The IRQ is freed first to stop the producer, then the work is
drained.

Fixes: a2d2362edf9f ("ASoC: twl6040: Add jack support for headset and handset")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
 sound/soc/codecs/twl6040.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/soc/codecs/twl6040.c b/sound/soc/codecs/twl6040.c
index 650836f9615fb..826bde1a9ba46 100644
--- a/sound/soc/codecs/twl6040.c
+++ b/sound/soc/codecs/twl6040.c
@@ -1136,6 +1136,7 @@ static void twl6040_remove(struct snd_soc_component *component)
 	struct twl6040_data *priv = snd_soc_component_get_drvdata(component);
 
 	free_irq(priv->plug_irq, component);
+	cancel_delayed_work_sync(&priv->hs_jack.work);
 }
 
 static const struct snd_soc_component_driver soc_component_dev_twl6040 = {
-- 
2.47.3

[PATCH 4/4] ASoC: wm8994: drain all delayed works on component remove

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

wm8994_component_remove() frees IRQs and firmware but does not cancel
any of the delayed works that IRQ handlers and DAPM events schedule.
jackdet_bootstrap can spawn mic_work (via wm1811_jackdet_irq), and
wm8958_mic_irq can queue mic_complete_work, whose callback may in turn
queue open_circuit_work.  If any of these works are pending when the
component is unbound, they fire after DAPM widgets are freed.

After freeing all IRQ producers and before releasing firmware, drain
the full work graph in producer-to-child order: jackdet_bootstrap,
mic_complete_work, mic_work, then open_circuit_work.

Fixes: 99af79dff5a6 ("ASoC: wm8994: Ensure we get a notification on startup for jackdet")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
 sound/soc/codecs/wm8994.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/sound/soc/codecs/wm8994.c b/sound/soc/codecs/wm8994.c
index 8bf58a6a8af7c..32c1786bd48ef 100644
--- a/sound/soc/codecs/wm8994.c
+++ b/sound/soc/codecs/wm8994.c
@@ -4592,6 +4592,10 @@ static void wm8994_component_remove(struct snd_soc_component *component)
 			free_irq(wm8994->micdet_irq, wm8994);
 		break;
 	}
+	cancel_delayed_work_sync(&wm8994->jackdet_bootstrap);
+	cancel_delayed_work_sync(&wm8994->mic_complete_work);
+	cancel_delayed_work_sync(&wm8994->mic_work);
+	cancel_delayed_work_sync(&wm8994->open_circuit_work);
 	release_firmware(wm8994->mbc);
 	release_firmware(wm8994->mbc_vss);
 	release_firmware(wm8994->enh_eq);
-- 
2.47.3

Re: [PATCH 1/4] ASoC: rt5668: cancel pending delayed works on component remove

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] ASoC: rt5668: delayed works not cancelled on I2C unbind without a card
- [High] ASoC: rt5668: stale jack state and regcache desync on component rebind
--

commit fa6da49bfd9e751eff346b0d1417045a1c8e0f42
Author: Tristan Madani <tristan@talencesecurity.com>

ASoC: rt5668: cancel pending delayed works on component remove

This commit tracks IRQ acquisition in struct rt5668_priv, disables the IRQ and
cancels jack_detect_work and jd_check_work in rt5668_remove(), and re-enables
the IRQ in rt5668_probe() on component rebind.

> diff --git a/sound/soc/codecs/rt5668.c b/sound/soc/codecs/rt5668.c
> index cd6245d6b91af..0e20e09b5434e 100644
> --- a/sound/soc/codecs/rt5668.c
> +++ b/sound/soc/codecs/rt5668.c
> @@ -62,6 +62,8 @@ struct rt5668_priv {
>  	int pll_out;
>  
>  	int jack_type;
> +	bool irq_acquired;
> +	bool irq_disabled;
>  };
>  
>  static const struct reg_default rt5668_reg[] = {
> @@ -2213,6 +2215,12 @@ static int rt5668_set_bias_level(struct snd_soc_component *component,
>  static int rt5668_probe(struct snd_soc_component *component)
>  {
>  	struct rt5668_priv *rt5668 = snd_soc_component_get_drvdata(component);
> +	struct i2c_client *i2c = to_i2c_client(component->dev);
> +
> +	if (rt5668->irq_disabled) {
> +		enable_irq(i2c->irq);
> +		rt5668->irq_disabled = false;
> +	}
>  
>  	rt5668->component = component;
>  
> @@ -2222,7 +2230,15 @@ static int rt5668_probe(struct snd_soc_component *component)
>  static void rt5668_remove(struct snd_soc_component *component)
>  {
>  	struct rt5668_priv *rt5668 = snd_soc_component_get_drvdata(component);
> +	struct i2c_client *i2c = to_i2c_client(component->dev);
>  
> +	if (rt5668->irq_acquired) {
> +		disable_irq(i2c->irq);
> +		rt5668->irq_disabled = true;
> +	}
> +
> +	cancel_delayed_work_sync(&rt5668->jack_detect_work);
> +	cancel_delayed_work_sync(&rt5668->jd_check_work);
>  	rt5668_reset(rt5668->regmap);
>  }

[Severity: High]
When a sound card unbinds and later rebinds to this component, can stale jack
state and regcache desynchronization break jack detection or dereference a
dangling rt5668->hs_jack pointer?

First, rt5668_remove() does not clear rt5668->hs_jack, rt5668->jack_type, or
rt5668->component when the previous card is removed and frees its
struct snd_soc_jack. On rebind, rt5668_probe() calls enable_irq(i2c->irq)
before rt5668->component is updated and before the new card sets a jack (and
rt5668_set_jack_detect() also schedules jack_detect_work before assigning
rt5668->hs_jack). If an interrupt fires and rt5668_jack_detect_handler() runs
after card instantiation before or without a new jack being set,
snd_soc_jack_report(rt5668->hs_jack, ...) will dereference the dangling
rt5668->hs_jack pointer from the old card.

Second, if a headset was plugged in when the card was unbound
(rt5668->jack_type != 0), rt5668_remove() resets the hardware without clearing
rt5668->jack_type. On rebind with the jack still inserted,
rt5668_jack_detect_handler() sees rt5668->jack_type != 0:

sound/soc/codecs/rt5668.c:rt5668_jack_detect_handler() {
	...
	if (!val) {
		/* jack in */
		if (rt5668->jack_type == 0) {
			/* jack was out, report jack type */
			rt5668->jack_type =
				rt5668_headset_detect(rt5668->component, 1);
		} else {
			/* jack is already in, report button event */
			rt5668->jack_type = SND_JACK_HEADSET;
			btn_type = rt5668_button_detect(rt5668->component);
	...
}

and takes the button detection branch instead of calling
rt5668_headset_detect(rt5668->component, 1) to reinitialize the headset and
button detection hardware.

Third, rt5668_remove() calls rt5668_reset(rt5668->regmap), which resets all
hardware registers to power-on defaults, without resetting or marking dirty
the REGCACHE_MAPLE regmap cache. When rt5668_set_jack_detect() runs on the
next card bind:

sound/soc/codecs/rt5668.c:rt5668_set_jack_detect() {
	...
		snd_soc_component_update_bits(component, RT5668_CBJ_CTRL_3,
			RT5668_CBJ_IN_BUF_EN, RT5668_CBJ_IN_BUF_EN);
		snd_soc_component_update_bits(component, RT5668_SAR_IL_CMD_1,
			RT5668_SAR_POW_MASK, RT5668_SAR_POW_EN);
		regmap_update_bits(rt5668->regmap, RT5668_GPIO_CTRL_1,
			RT5668_GP1_PIN_MASK, RT5668_GP1_PIN_IRQ);
		regmap_update_bits(rt5668->regmap, RT5668_RC_CLK_CTRL,
				RT5668_POW_IRQ | RT5668_POW_JDH |
				RT5668_POW_ANA, RT5668_POW_IRQ |
				RT5668_POW_JDH | RT5668_POW_ANA);
		regmap_update_bits(rt5668->regmap, RT5668_PWR_ANLG_2,
			RT5668_PWR_JDH | RT5668_PWR_JDL,
			RT5668_PWR_JDH | RT5668_PWR_JDL);
		regmap_update_bits(rt5668->regmap, RT5668_IRQ_CTRL_2,
			RT5668_JD1_EN_MASK | RT5668_JD1_POL_MASK,
			RT5668_JD1_EN | RT5668_JD1_POL_NOR);
	...
}

these cached non-volatile register updates become no-ops because the software
cache still holds the pre-reset bit values, leaving GPIO1 IRQ output
(RT5668_GP1_PIN_IRQ) and JD1 (RT5668_JD1_EN) disabled in hardware.

> @@ -2553,7 +2569,9 @@ static int rt5668_i2c_probe(struct i2c_client *i2c)
>  		ret = devm_request_threaded_irq(&i2c->dev, i2c->irq, NULL,
>  			rt5668_irq, IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING
>  			| IRQF_ONESHOT, "rt5668", rt5668);
> -		if (ret)
> +		if (!ret)
> +			rt5668->irq_acquired = true;
> +		else
>  			dev_err(&i2c->dev, "Failed to request IRQ: %d\n", ret);
>  
>  	}

[Severity: High]
What happens if the I2C device is unbound while no ASoC sound card is bound
(or if devm_snd_soc_register_component() fails after the IRQ is requested)?

In rt5668_i2c_probe(), devm_request_threaded_irq() enables i2c->irq before
any card is bound. If an interrupt fires while rt5668->component is NULL or
before card instantiation (or if snd_soc_component_set_jack(component, NULL,
NULL) queues jack_detect_work during card teardown), rt5668_irq() schedules
rt5668->jack_detect_work, which then Rearms itself every 15 ms indefinitely:

sound/soc/codecs/rt5668.c:rt5668_jack_detect_handler() {
	...
	if (!rt5668->component ||
	    !snd_soc_card_is_instantiated(rt5668->component->card)) {
		/* card not yet ready, try later */
		mod_delayed_work(system_power_efficient_wq,
				 &rt5668->jack_detect_work, msecs_to_jiffies(15));
		return;
	}
	...
}

Because snd_soc_component_remove() only invokes component->driver->remove
when component->card is non-NULL:

sound/soc/soc-component.c:snd_soc_component_remove() {
	if (!component->card)
		return;

	if (probed && component->driver->remove)
		component->driver->remove(component);
	...
}

unbinding the I2C client when no card is bound skips rt5668_remove()
completely, and rt5668_i2c_driver has no .remove callback or devm cleanup
action to cancel jack_detect_work and jd_check_work before devres frees
struct rt5668_priv. Similarly, rt5668_i2c_shutdown() resets the codec without
disabling the IRQ or cancelling the delayed works.

Should the delayed works also be cancelled (and the IRQ quiesced) on I2C
driver removal/devres release and shutdown, or via
devm_delayed_work_autocancel()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261026112212.2519775-1-tristmd@gmail.com?part=1

Re: [PATCH 2/4] ASoC: rt5659: cancel jack_detect_work on component remove

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] ASoC: rt5659: uninitialized delayed_work cancel in rt5659_remove()
- [High] ASoC: rt5659: uninitialized jack_detect_work used when jd_src is unset
--

commit ca81fa64a51ff1ffcf9ce836fb012f5147653078
Author: Tristan Madani <tristan@talencesecurity.com>

ASoC: rt5659: cancel jack_detect_work on component remove

This commit disables the codec IRQ and cancels jack_detect_work
synchronously in rt5659_remove() before resetting the codec, and
re-enables the IRQ in rt5659_probe() on component rebind.

> diff --git a/sound/soc/codecs/rt5659.c b/sound/soc/codecs/rt5659.c
> index af8051fa69f89..d9daede7310ea 100644
> --- a/sound/soc/codecs/rt5659.c
> +++ b/sound/soc/codecs/rt5659.c
[ ... ]
> @@ -3684,7 +3690,14 @@ static int rt5659_probe(struct snd_soc_component *component)
>  static void rt5659_remove(struct snd_soc_component *component)
>  {
>  	struct rt5659_priv *rt5659 = snd_soc_component_get_drvdata(component);
> +	struct i2c_client *i2c = to_i2c_client(component->dev);
> +
> +	if (rt5659->irq_acquired) {
> +		disable_irq(i2c->irq);
> +		rt5659->irq_disabled = true;
> +	}
>  
> +	cancel_delayed_work_sync(&rt5659->jack_detect_work);

[Severity: High]
Is rt5659->jack_detect_work guaranteed to be initialized before
rt5659_remove() calls cancel_delayed_work_sync()?

In sound/soc/codecs/rt5659.c, rt5659_i2c_probe() allocates rt5659 with
devm_kzalloc() and only calls INIT_DELAYED_WORK() when
rt5659->pdata.jd_src is RT5659_JD3 or RT5659_JD_HDA_HEADER:

sound/soc/codecs/rt5659.c:rt5659_i2c_probe() {
	...
	switch (rt5659->pdata.jd_src) {
	case RT5659_JD3:
		...
		INIT_DELAYED_WORK(&rt5659->jack_detect_work,
			rt5659_jack_detect_work);
		break;
	case RT5659_JD_HDA_HEADER:
		...
		INIT_DELAYED_WORK(&rt5659->jack_detect_work,
			rt5659_jack_detect_intel_hd_header);
		rt5659_intel_hd_header_probe_setup(rt5659);
		break;
	default:
		break;
	}
	...
}

When the optional realtek,jd-src property is omitted or set to 0
(RT5659_JD_NULL), the default branch is taken and
rt5659->jack_detect_work remains zero-initialized with work.func == NULL
and an uninitialized timer.

When snd_soc_component_remove() -> rt5659_remove() then runs on card
unbind or driver removal, cancel_delayed_work_sync() ->
__cancel_work_sync() calls timer_delete() on the uninitialized timer and
__flush_work() hits WARN_ON(!work->func).

Additionally, because rt5659_i2c_probe() still registers rt5659_irq when
i2c->irq is present and rt5659_set_jack_detect() invokes rt5659_irq(), an
interrupt or jack detection setup with pdata.jd_src == RT5659_JD_NULL will
call queue_delayed_work() on the uninitialized rt5659->jack_detect_work.

Should cancel_delayed_work_sync() (as well as IRQ registration or
work queueing) be guarded by checking whether rt5659->pdata.jd_src
configured jack_detect_work?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261026112212.2519775-1-tristmd@gmail.com?part=2

Re: [PATCH 4/4] ASoC: wm8994: drain all delayed works on component remove

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ASoC: wm8994: uninitialized mic_work cancel in wm8994_component_remove()
--

commit fac5aa258b308d77c115320a9d762e1310e3f8f2
Author: Tristan Madani <tristan@talencesecurity.com>

ASoC: wm8994: drain all delayed works on component remove

This commit cancels jackdet_bootstrap, mic_complete_work, mic_work, and
open_circuit_work synchronously in wm8994_component_remove() after freeing
IRQs and before releasing firmware.

> diff --git a/sound/soc/codecs/wm8994.c b/sound/soc/codecs/wm8994.c
> index 6b91e02f130b3..59b9cdcd5fc6a 100644
> --- a/sound/soc/codecs/wm8994.c
> +++ b/sound/soc/codecs/wm8994.c
> @@ -4636,6 +4636,10 @@ static void wm8994_component_remove(struct snd_soc_component *component)
>  			free_irq(wm8994->micdet_irq, wm8994);
>  		break;
>  	}
> +	cancel_delayed_work_sync(&wm8994->jackdet_bootstrap);
> +	cancel_delayed_work_sync(&wm8994->mic_complete_work);
> +	cancel_delayed_work_sync(&wm8994->mic_work);

[Severity: High]
In sound/soc/codecs/wm8994.c, wm8994_component_probe() only initializes
wm8994->mic_work when control->type is WM8994 or WM1811:

	switch (control->type) {
	case WM8994:
		INIT_DELAYED_WORK(&wm8994->mic_work, wm8994_mic_work);
		break;
	case WM1811:
		INIT_DELAYED_WORK(&wm8994->mic_work, wm1811_mic_work);
		break;
	default:
		break;
	}

When control->type is WM8958, wm8994->mic_work is left zero-initialized
with work.func == NULL and an uninitialized timer.

Won't calling cancel_delayed_work_sync(&wm8994->mic_work) unconditionally
in wm8994_component_remove() on a WM8958 device trigger WARN_ON(!work->func)
via __cancel_work_sync() -> __flush_work(), as well as an uninitialized
timer warning in timer_delete() when debug objects are enabled?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261026112212.2519775-1-tristmd@gmail.com?part=4

Re: [PATCH 4/4] ASoC: wm8994: drain all delayed works on component remove

[Mark Brown]

> wm8994_component_remove() frees IRQs and firmware but does not cancel
> any of the delayed works that IRQ handlers and DAPM events schedule.
> jackdet_bootstrap can spawn mic_work (via wm1811_jackdet_irq), and
> wm8958_mic_irq can queue mic_complete_work, whose callback may in turn
> queue open_circuit_work.  If any of these works are pending when the
> component is unbound, they fire after DAPM widgets are freed.

> +	cancel_delayed_work_sync(&wm8994->jackdet_bootstrap);
> +	cancel_delayed_work_sync(&wm8994->mic_complete_work);
> +	cancel_delayed_work_sync(&wm8994->mic_work);

Same issue here for mic_work.

-- 
Mark Brown <broonie@kernel.org>

Re: [PATCH 2/4] ASoC: rt5659: cancel jack_detect_work on component remove

[Mark Brown]

> Track successful IRQ acquisition with a flag so that disable_irq()
> only quiesces a line this driver actually owns.  On component remove,
> disable the IRQ to synchronize with any running handler and prevent
> new enqueues, then drain the residual jack_detect_work synchronously
> before resetting the hardware.  On the next component probe (card
> rebind), re-enable the IRQ to restore jack detection.

> @@ -3684,7 +3690,14 @@ static int rt5659_probe(struct snd_soc_component *component)
>  static void rt5659_remove(struct snd_soc_component *component)
>  {
>  	struct rt5659_priv *rt5659 = snd_soc_component_get_drvdata(component);
> +	struct i2c_client *i2c = to_i2c_client(component->dev);
> +
> +	if (rt5659->irq_acquired) {
> +		disable_irq(i2c->irq);
> +		rt5659->irq_disabled = true;
> +	}
>  
> +	cancel_delayed_work_sync(&rt5659->jack_detect_work);

This work isn't unconditionally initialised, cancelling something that
wasn't initialised is an error and will trigger warnings.  Either this
needs to be conditional or we should just always initialise.

-- 
Mark Brown <broonie@kernel.org>

[PATCH v9] ASoC: wm8994: drain delayed works on remove and probe error

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

wm8994_component_probe() schedules several delayed works to handle jack
detection and microphone status polling.  If the component is removed or
probe fails after these works are scheduled, they can execute after
resources are freed.

Cancel all pending delayed works in the error and removal paths.  Guard
cancel_delayed_work_sync(&wm8994->mic_work) with a type check because
INIT_DELAYED_WORK for mic_work only runs in the WM8994 and WM1811 code
paths.

Fixes: 99af79dff5a6 ("ASoC: wm8994: Ensure we get a notification on startup for jackdet")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
Changes since v8 [4/4]:
- Guard cancel_delayed_work_sync(&wm8994->mic_work) with type check
  (WM8994 || WM1811), as mic_work is only initialized in those code
  paths — addresses Mark Brown review comment
- Add cleanup to probe error path (err_irq label), not just remove

 sound/soc/codecs/wm8994.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/sound/soc/codecs/wm8994.c b/sound/soc/codecs/wm8994.c
index fc9894975a1d5..30a164796fd48 100644
--- a/sound/soc/codecs/wm8994.c
+++ b/sound/soc/codecs/wm8994.c
@@ -4563,6 +4563,11 @@ static int wm8994_component_probe(struct snd_soc_component *component)
 	wm8994_free_irq(wm8994->wm8994, WM8994_IRQ_FIFOS_ERR, component);
 	wm8994_free_irq(wm8994->wm8994, WM8994_IRQ_TEMP_SHUT, component);
 	wm8994_free_irq(wm8994->wm8994, WM8994_IRQ_TEMP_WARN, component);
+	cancel_delayed_work_sync(&wm8994->jackdet_bootstrap);
+	cancel_delayed_work_sync(&wm8994->mic_complete_work);
+	if (control->type == WM8994 || control->type == WM1811)
+		cancel_delayed_work_sync(&wm8994->mic_work);
+	cancel_delayed_work_sync(&wm8994->open_circuit_work);
 
 	return ret;
 }
@@ -4604,6 +4609,11 @@ static void wm8994_component_remove(struct snd_soc_component *component)
 			free_irq(wm8994->micdet_irq, wm8994);
 		break;
 	}
+	cancel_delayed_work_sync(&wm8994->jackdet_bootstrap);
+	cancel_delayed_work_sync(&wm8994->mic_complete_work);
+	if (control->type == WM8994 || control->type == WM1811)
+		cancel_delayed_work_sync(&wm8994->mic_work);
+	cancel_delayed_work_sync(&wm8994->open_circuit_work);
 	release_firmware(wm8994->mbc);
 	release_firmware(wm8994->mbc_vss);
 	release_firmware(wm8994->enh_eq);
-- 
2.47.3

Re: [PATCH v9] ASoC: wm8994: drain delayed works on remove and probe error

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] ASoC: wm8994: mic_complete_work cancelled before mic_work allows re-arming
- [High] ASoC: wm8994: wm1811_mic_work() can queue jackdet_bootstrap after cancel
--

commit 04e84773079f92c3b3f2e772c451810f75d5d070
Author: Tristan Madani <tristan@talencesecurity.com>

ASoC: wm8994: drain delayed works on remove and probe error

This commit cancels pending delayed works in the probe error and component
removal paths of the wm8994 codec driver to prevent them from executing after
resources are freed.

> diff --git a/sound/soc/codecs/wm8994.c b/sound/soc/codecs/wm8994.c
> index c65a7769a9d25..cd5f134c70dd1 100644
> --- a/sound/soc/codecs/wm8994.c
> +++ b/sound/soc/codecs/wm8994.c
> @@ -4574,6 +4574,11 @@ static int wm8994_component_probe(struct snd_soc_component *component)
>  	wm8994_free_irq(wm8994->wm8994, WM8994_IRQ_FIFOS_ERR, component);
>  	wm8994_free_irq(wm8994->wm8994, WM8994_IRQ_TEMP_SHUT, component);
>  	wm8994_free_irq(wm8994->wm8994, WM8994_IRQ_TEMP_WARN, component);
> +	cancel_delayed_work_sync(&wm8994->jackdet_bootstrap);
> +	cancel_delayed_work_sync(&wm8994->mic_complete_work);
> +	if (control->type == WM8994 || control->type == WM1811)
> +		cancel_delayed_work_sync(&wm8994->mic_work);
> +	cancel_delayed_work_sync(&wm8994->open_circuit_work);

[Severity: High]
Could cancelling wm8994->mic_complete_work before wm8994->mic_work in both
wm8994_component_probe() and wm8994_component_remove() allow
wm8994->mic_complete_work to be re-queued after it was already cancelled?

On WM1811, the delayed work chain proceeds in the order:

  jackdet_bootstrap (wm1811_jackdet_bootstrap -> wm1811_jackdet_irq)
    -> mic_work (wm1811_mic_work)
      -> WM8994_IRQ_MIC1_DET (wm8958_mic_irq)
        -> mic_complete_work (wm8958_mic_work -> wm8958_mic_id)
          -> open_circuit_work (wm8958_open_circuit_work)

Note that WM8994_IRQ_MIC1_DET is not freed in the err_irq path of
wm8994_component_probe(), nor in wm8994_component_remove() for WM1811/WM8958
when !wm8994->micdet_irq.

If wm1811_mic_work() is pending or executing when
cancel_delayed_work_sync(&wm8994->mic_complete_work) returns, wm1811_mic_work()
enables microphone detection in hardware:

sound/soc/codecs/wm8994.c:wm1811_mic_work() {
	...
		wm8994->mic_detecting = true;
		wm1811_jackdet_set_mode(component, WM1811_JACKDET_MODE_MIC);

		snd_soc_component_update_bits(component, WM8958_MIC_DETECT_1,
				    WM8958_MICD_ENA, WM8958_MICD_ENA);
	...
}

When WM8994_IRQ_MIC1_DET then fires, wm8958_mic_irq() queues
wm8994->mic_complete_work again:

sound/soc/codecs/wm8994.c:wm8958_mic_irq() {
	...
	if (wm8994->mic_detecting)
		queue_delayed_work(system_power_efficient_wq,
				   &wm8994->mic_complete_work,
				   msecs_to_jiffies(id_delay));
	...
}

Should wm8994->mic_work (and WM8994_IRQ_MIC1_DET) be stopped before
cancelling wm8994->mic_complete_work and wm8994->open_circuit_work so they
cannot execute after struct snd_soc_component and struct wm8994_priv are freed?

>  
>  	return ret;
>  }
> @@ -4615,6 +4620,11 @@ static void wm8994_component_remove(struct snd_soc_component *component)
>  			free_irq(wm8994->micdet_irq, wm8994);
>  		break;
>  	}
> +	cancel_delayed_work_sync(&wm8994->jackdet_bootstrap);
> +	cancel_delayed_work_sync(&wm8994->mic_complete_work);
> +	if (control->type == WM8994 || control->type == WM1811)
> +		cancel_delayed_work_sync(&wm8994->mic_work);
> +	cancel_delayed_work_sync(&wm8994->open_circuit_work);

[Severity: High]
What happens in wm8994_component_remove() on WM1811 when wm8994->jackdet is
true, control->pdata.jd_ext_cap is true, and wm8994->clk_has_run is still
false?

If WM8994_IRQ_GPIO(6) (wm1811_jackdet_irq) fires before the IRQ is freed in
wm8994_component_remove() and queues wm8994->mic_work (wm1811_mic_work):

1. cancel_delayed_work_sync(&wm8994->jackdet_bootstrap) returns immediately
   because jackdet_bootstrap is not yet queued.

2. cancel_delayed_work_sync(&wm8994->mic_work) then waits for the in-flight
   wm1811_mic_work() to complete.

3. Inside wm1811_mic_work(), force-enabling MICBIAS2 powers up its parent
   supply CLK_SYS:

sound/soc/codecs/wm8994.c:wm1811_mic_work() {
	...
	if (control->pdata.jd_ext_cap) {
		snd_soc_dapm_force_enable_pin(dapm, "MICBIAS2");
		snd_soc_dapm_sync(dapm);
	}
	...
}

4. Powering up CLK_SYS invokes clk_sys_event(SND_SOC_DAPM_POST_PMU), which
   queues wm8994->jackdet_bootstrap:

sound/soc/codecs/wm8994.c:clk_sys_event() {
	...
		if (wm8994->jackdet && !wm8994->clk_has_run) {
			queue_delayed_work(system_power_efficient_wq,
					   &wm8994->jackdet_bootstrap,
					   msecs_to_jiffies(1000));
			wm8994->clk_has_run = true;
		}
	...
}

Can this leave wm8994->jackdet_bootstrap armed after
wm8994_component_remove() finishes, leading to a use-after-free when the work
fires 1000ms later?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261029102515.3417777-1-tristmd@gmail.com?part=1

[PATCH v10] ASoC: wm8994: drain delayed works on remove and probe error

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

wm8994_component_probe() schedules several delayed works to handle jack
detection and microphone status polling.  If the component is removed or
probe fails after these works are scheduled, they can execute after
resources are freed.

Cancel all pending delayed works in the error and removal paths.  Guard
cancel_delayed_work_sync(&wm8994->mic_work) with a type check because
INIT_DELAYED_WORK for mic_work only runs in the WM8994 and WM1811 code
paths.

Cancel mic_work before jackdet_bootstrap and mic_complete_work because
wm1811_mic_work() can re-arm jackdet_bootstrap through the DAPM power-up
chain (force-enabling MICBIAS2 triggers clk_sys_event which queues
jackdet_bootstrap), and enabling microphone detection in hardware can
cause WM8994_IRQ_MIC1_DET to fire and re-queue mic_complete_work.

Fixes: 99af79dff5a6 ("ASoC: wm8994: Ensure we get a notification on startup for jackdet")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
Changes since v9:
- Reorder cancel_delayed_work_sync calls: cancel mic_work first,
  then jackdet_bootstrap, mic_complete_work, open_circuit_work.
  wm1811_mic_work() can re-arm jackdet_bootstrap through the DAPM
  power-up chain (MICBIAS2 -> CLK_SYS -> clk_sys_event) and can
  trigger MIC1_DET which re-queues mic_complete_work, so cancelling
  mic_work before its downstream consumers prevents re-arming after
  cancel.

Changes since v8 [4/4]:
- Guard cancel_delayed_work_sync(&wm8994->mic_work) with type check
  (WM8994 || WM1811), as mic_work is only initialized in those code
  paths — addresses Mark Brown review comment
- Add cleanup to probe error path (err_irq label), not just remove

 sound/soc/codecs/wm8994.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/sound/soc/codecs/wm8994.c b/sound/soc/codecs/wm8994.c
index fc9894975a1d5..5a493f29c2ca3 100644
--- a/sound/soc/codecs/wm8994.c
+++ b/sound/soc/codecs/wm8994.c
@@ -4563,6 +4563,11 @@ static int wm8994_component_probe(struct snd_soc_component *component)
 	wm8994_free_irq(wm8994->wm8994, WM8994_IRQ_FIFOS_ERR, component);
 	wm8994_free_irq(wm8994->wm8994, WM8994_IRQ_TEMP_SHUT, component);
 	wm8994_free_irq(wm8994->wm8994, WM8994_IRQ_TEMP_WARN, component);
+	if (control->type == WM8994 || control->type == WM1811)
+		cancel_delayed_work_sync(&wm8994->mic_work);
+	cancel_delayed_work_sync(&wm8994->jackdet_bootstrap);
+	cancel_delayed_work_sync(&wm8994->mic_complete_work);
+	cancel_delayed_work_sync(&wm8994->open_circuit_work);
 
 	return ret;
 }
@@ -4604,6 +4609,11 @@ static void wm8994_component_remove(struct snd_soc_component *component)
 			free_irq(wm8994->micdet_irq, wm8994);
 		break;
 	}
+	if (control->type == WM8994 || control->type == WM1811)
+		cancel_delayed_work_sync(&wm8994->mic_work);
+	cancel_delayed_work_sync(&wm8994->jackdet_bootstrap);
+	cancel_delayed_work_sync(&wm8994->mic_complete_work);
+	cancel_delayed_work_sync(&wm8994->open_circuit_work);
 	release_firmware(wm8994->mbc);
 	release_firmware(wm8994->mbc_vss);
 	release_firmware(wm8994->enh_eq);
-- 
2.47.3

Re: [PATCH v9] ASoC: wm8994: drain delayed works on remove and probe error

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 700 bytes --]

On Thu, Oct 29, 2026 at 10:25:15AM +0000, Tristan Madani wrote:
> From: Tristan Madani <tristan@talencesecurity.com>
> 
> wm8994_component_probe() schedules several delayed works to handle jack
> detection and microphone status polling.  If the component is removed or
> probe fails after these works are scheduled, they can execute after
> resources are freed.

You've taken what was previously a series of 4 patches with only v1
posted, resent only two of the patches individually and for some reason
incremented the version to v9.  This all makes things really confusing
to track for both me and my tooling.  Please resend the full series as a
single series, with all of the patches.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v10] ASoC: wm8994: drain delayed works on remove and probe error

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ASoC: wm8994: mic_work can be re-queued by wm1811_jackdet_bootstrap()
--

commit 57d6ba6d8a26f549eace6106e2dcf810fa0f529b
Author: Tristan Madani <tristan@talencesecurity.com>

ASoC: wm8994: drain delayed works on remove and probe error

This commit cancels pending delayed works in wm8994_component_probe() error
handling and wm8994_component_remove() to prevent works from executing after
resources are freed.

> diff --git a/sound/soc/codecs/wm8994.c b/sound/soc/codecs/wm8994.c
> index c65a7769a9d25..c95a7a4dffb19 100644
> --- a/sound/soc/codecs/wm8994.c
> +++ b/sound/soc/codecs/wm8994.c
> @@ -4574,6 +4574,11 @@ static int wm8994_component_probe(struct snd_soc_component *component)
>  	wm8994_free_irq(wm8994->wm8994, WM8994_IRQ_FIFOS_ERR, component);
>  	wm8994_free_irq(wm8994->wm8994, WM8994_IRQ_TEMP_SHUT, component);
>  	wm8994_free_irq(wm8994->wm8994, WM8994_IRQ_TEMP_WARN, component);
> +	if (control->type == WM8994 || control->type == WM1811)
> +		cancel_delayed_work_sync(&wm8994->mic_work);
> +	cancel_delayed_work_sync(&wm8994->jackdet_bootstrap);
> +	cancel_delayed_work_sync(&wm8994->mic_complete_work);
> +	cancel_delayed_work_sync(&wm8994->open_circuit_work);
>  
>  	return ret;
>  }
> @@ -4615,6 +4620,11 @@ static void wm8994_component_remove(struct snd_soc_component *component)
>  			free_irq(wm8994->micdet_irq, wm8994);
>  		break;
>  	}
> +	if (control->type == WM8994 || control->type == WM1811)
> +		cancel_delayed_work_sync(&wm8994->mic_work);
> +	cancel_delayed_work_sync(&wm8994->jackdet_bootstrap);
> +	cancel_delayed_work_sync(&wm8994->mic_complete_work);
> +	cancel_delayed_work_sync(&wm8994->open_circuit_work);

[Severity: High]
Can wm8994->mic_work be re-queued here if wm8994->jackdet_bootstrap runs
after cancel_delayed_work_sync(&wm8994->mic_work) completes?

In sound/soc/codecs/wm8994.c, wm1811_jackdet_bootstrap() calls
wm1811_jackdet_irq(), which queues wm8994->mic_work when a jack is present:

static void wm1811_jackdet_bootstrap(struct work_struct *work)
{
	struct wm8994_priv *wm8994 = container_of(work,
						struct wm8994_priv,
						jackdet_bootstrap.work);
	wm1811_jackdet_irq(0, wm8994);
}

and inside wm1811_jackdet_irq():

	if (present) {
		...
		delay = control->pdata.micdet_delay;
		queue_delayed_work(system_power_efficient_wq,
				   &wm8994->mic_work,
				   msecs_to_jiffies(delay));
	}

On WM1811 with jack detection enabled, powering up CLK_SYS triggers
clk_sys_event(), which schedules wm8994->jackdet_bootstrap.

Because wm1811_mic_work() can schedule wm8994->jackdet_bootstrap (via
snd_soc_dapm_force_enable_pin() -> clk_sys_event()) and
wm1811_jackdet_bootstrap() can in turn schedule wm8994->mic_work, the two
works have a cyclic dependency. If wm1811_jackdet_bootstrap() is already
executing or its timer fires between cancel_delayed_work_sync(&wm8994->mic_work)
and cancel_delayed_work_sync(&wm8994->jackdet_bootstrap), it will re-arm
wm8994->mic_work, leaving wm1811_mic_work() pending after
wm8994_component_remove() (or the wm8994_component_probe() error path) returns
and frees the component and wm8994_priv memory.

Would it make sense to use disable_delayed_work_sync() on these works during
teardown, or cancel wm8994->mic_work a second time after
cancel_delayed_work_sync(&wm8994->jackdet_bootstrap) finishes?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261029105749.3732334-1-tristmd@gmail.com?part=1

Re: [PATCH v10] ASoC: wm8994: drain delayed works on remove and probe error

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 662 bytes --]

On Thu, Oct 29, 2026 at 10:57:49AM +0000, Tristan Madani wrote:
> From: Tristan Madani <tristan@talencesecurity.com>
> 
> wm8994_component_probe() schedules several delayed works to handle jack
> detection and microphone status polling.  If the component is removed or
> probe fails after these works are scheduled, they can execute after
> resources are freed.

Please don't send new patches in reply to old patches or serieses, this
makes it harder for both people and tools to understand what is going
on - it can bury things in mailboxes and make it difficult to keep track
of what current patches are, both for the new patches and the old ones.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]