All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/4] ASoC: cancel pending delayed works on codec removal
@ 2026-10-30 15:41 Tristan Madani
  2026-10-30 15:41 ` [PATCH v3 1/4] ASoC: rt5668: cancel delayed works on component removal Tristan Madani
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Tristan Madani @ 2026-10-30 15:41 UTC (permalink / raw)
  To: Mark Brown
  Cc: Oder Chiou, Liam Girdwood, Jaroslav Kysela, Takashi Iwai,
	Bard Liao, Misael Lopez Cruz, patches, linux-sound, stable,
	Tristan Madani

Several ASoC codec drivers schedule delayed works for jack detection
and microphone polling but do not cancel them on component removal,
leaving handlers that can run against freed memory.

This series adds proper teardown ordering in four codec drivers:
rt5668, rt5659, twl6040, and wm8994.

Changes since v2:
  - rt5668 (1/4): also cancel jd_check_work (re-arms itself every
    500 jiffies while jack inserted). Disable IRQ before cancelling
    to prevent the handler from scheduling new work in the window
    between cancel and devm IRQ release.
  - rt5659 (2/4): no code change from v2.
  - twl6040 (3/4): swap ordering -- free_irq() first, then cancel
    residual work. Fixes the race where the handler fires between
    cancel and free_irq and re-arms the work.
  - wm8994 (4/4): no code change from v2.

Changes since v1:
  - Unified series per Mark Brown request.
  - rt5659: guard cancel with jack_detect_work_inited flag since
    INIT_DELAYED_WORK is conditional on jd_src, per Mark Brown.
  - wm8994: cancel mic_work before and after jackdet_bootstrap for
    cyclic re-arming through DAPM power-up chain.

Tristan Madani (4):
  ASoC: rt5668: cancel delayed works on component removal
  ASoC: rt5659: cancel jack_detect_work on component removal
  ASoC: twl6040: cancel headset jack work on component removal
  ASoC: wm8994: drain delayed works on remove and probe error

Tristan Madani (4):
  ASoC: rt5668: cancel delayed works on component removal
  ASoC: rt5659: cancel jack_detect_work on component removal
  ASoC: twl6040: cancel headset jack work on component removal
  ASoC: wm8994: drain delayed works on remove and probe error

 sound/soc/codecs/rt5659.c  | 20 +++++++++++++++++++-
 sound/soc/codecs/rt5659.h  |  3 +++
 sound/soc/codecs/rt5668.c  |  7 +++++++
 sound/soc/codecs/twl6040.c |  1 +
 sound/soc/codecs/wm8994.c  | 14 ++++++++++++++
 5 files changed, 44 insertions(+), 1 deletion(-)

-- 
2.47.3


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

* [PATCH v3 1/4] ASoC: rt5668: cancel delayed works on component removal
  2026-10-30 15:41 [PATCH v3 0/4] ASoC: cancel pending delayed works on codec removal Tristan Madani
@ 2026-10-30 15:41 ` Tristan Madani
  2026-10-30 15:56   ` sashiko-bot
  2026-10-30 15:42 ` [PATCH v3 2/4] ASoC: rt5659: cancel jack_detect_work " Tristan Madani
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: Tristan Madani @ 2026-10-30 15:41 UTC (permalink / raw)
  To: Mark Brown
  Cc: Oder Chiou, Liam Girdwood, Jaroslav Kysela, Takashi Iwai,
	Bard Liao, Misael Lopez Cruz, patches, linux-sound, stable,
	Tristan Madani

The rt5668 codec driver initializes jack_detect_work and jd_check_work
but neither is cancelled during component removal.  If the device is
unbound while work is pending, the handler can access freed memory.

The jack_detect_work handler schedules jd_check_work, which re-arms
itself every 500 jiffies while a jack remains inserted, so both must
be cancelled.

Because the IRQ is registered via devm_request_threaded_irq(), it
remains active until after the component remove callback returns.
Disable the IRQ first to synchronize with any running handler and
prevent new work from being scheduled, then cancel both works
synchronously.

Fixes: d59fb28562232 ("ASoC: rt5668: add rt5668B codec driver")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---

Changes in v3:
  - Also cancel jd_check_work (self-rearming every 500 jiffies)
  - Disable IRQ before cancel to prevent handler re-queueing

Changes in v2:
  - Resend as part of unified series (no code changes from v1)
 sound/soc/codecs/rt5668.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/sound/soc/codecs/rt5668.c b/sound/soc/codecs/rt5668.c
index 4154bad276101..b273449fac9db 100644
--- a/sound/soc/codecs/rt5668.c
+++ b/sound/soc/codecs/rt5668.c
@@ -2222,6 +2222,13 @@ 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 (i2c->irq)
+		disable_irq(i2c->irq);
+
+	cancel_delayed_work_sync(&rt5668->jack_detect_work);
+	cancel_delayed_work_sync(&rt5668->jd_check_work);
 
 	rt5668_reset(rt5668->regmap);
 }
-- 
2.47.3


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

* [PATCH v3 2/4] ASoC: rt5659: cancel jack_detect_work on component removal
  2026-10-30 15:41 [PATCH v3 0/4] ASoC: cancel pending delayed works on codec removal Tristan Madani
  2026-10-30 15:41 ` [PATCH v3 1/4] ASoC: rt5668: cancel delayed works on component removal Tristan Madani
@ 2026-10-30 15:42 ` Tristan Madani
  2026-10-30 15:53   ` sashiko-bot
  2026-10-30 15:42 ` [PATCH v3 3/4] ASoC: twl6040: cancel headset jack work " Tristan Madani
  2026-10-30 15:42 ` [PATCH v3 4/4] ASoC: wm8994: drain delayed works on remove and probe error Tristan Madani
  3 siblings, 1 reply; 7+ messages in thread
From: Tristan Madani @ 2026-10-30 15:42 UTC (permalink / raw)
  To: Mark Brown
  Cc: Oder Chiou, Liam Girdwood, Jaroslav Kysela, Takashi Iwai,
	Bard Liao, Misael Lopez Cruz, patches, linux-sound, stable,
	Tristan Madani

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.  Guard the
cancel_delayed_work_sync() call with a flag that tracks whether the
work was initialized, since INIT_DELAYED_WORK is only called for
specific jd_src configurations (JD3, JD_HDA_HEADER) and cancelling
an uninitialized work is undefined behavior.

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>
---

Changes in v3:
  - No code change from v2

Changes in v2:
  - Resend as part of unified series
  - Guard cancel with jack_detect_work_inited flag per Mark Brown
  - Track IRQ acquisition for disable_irq/enable_irq
 sound/soc/codecs/rt5659.c | 20 +++++++++++++++++++-
 sound/soc/codecs/rt5659.h |  3 +++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/sound/soc/codecs/rt5659.c b/sound/soc/codecs/rt5659.c
index 3097ee6d4e89f..cdf499fb36d67 100644
--- a/sound/soc/codecs/rt5659.c
+++ b/sound/soc/codecs/rt5659.c
@@ -3667,6 +3667,11 @@ static int rt5659_probe(struct snd_soc_component *component)
 
 	rt5659->component = component;
 
+	if (rt5659->irq_disabled) {
+		enable_irq(to_i2c_client(component->dev)->irq);
+		rt5659->irq_disabled = false;
+	}
+
 	switch (rt5659->pdata.jd_src) {
 	case RT5659_JD_HDA_HEADER:
 		break;
@@ -3684,6 +3689,15 @@ 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;
+	}
+
+	if (rt5659->jack_detect_work_inited)
+		cancel_delayed_work_sync(&rt5659->jack_detect_work);
 
 	regmap_write(rt5659->regmap, RT5659_RESET, 0);
 }
@@ -4271,6 +4285,7 @@ static int rt5659_i2c_probe(struct i2c_client *i2c)
 		regmap_write(rt5659->regmap, RT5659_IRQ_CTRL_2, 0x0040);
 		INIT_DELAYED_WORK(&rt5659->jack_detect_work,
 			rt5659_jack_detect_work);
+		rt5659->jack_detect_work_inited = true;
 		break;
 	case RT5659_JD_HDA_HEADER:
 		regmap_write(rt5659->regmap, RT5659_GPIO_CTRL_3, 0x8000);
@@ -4280,6 +4295,7 @@ static int rt5659_i2c_probe(struct i2c_client *i2c)
 		regmap_write(rt5659->regmap, RT5659_IRQ_CTRL_1,  0x0040);
 		INIT_DELAYED_WORK(&rt5659->jack_detect_work,
 			rt5659_jack_detect_intel_hd_header);
+		rt5659->jack_detect_work_inited = true;
 		rt5659_intel_hd_header_probe_setup(rt5659);
 		break;
 	default:
@@ -4290,7 +4306,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..e10b3844501b9 100644
--- a/sound/soc/codecs/rt5659.h
+++ b/sound/soc/codecs/rt5659.h
@@ -1813,6 +1813,9 @@ struct rt5659_priv {
 	int jack_type;
 	bool hda_hp_plugged;
 	bool hda_mic_plugged;
+	bool irq_acquired;
+	bool irq_disabled;
+	bool jack_detect_work_inited;
 };
 
 int rt5659_set_jack_detect(struct snd_soc_component *component,
-- 
2.47.3


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

* [PATCH v3 3/4] ASoC: twl6040: cancel headset jack work on component removal
  2026-10-30 15:41 [PATCH v3 0/4] ASoC: cancel pending delayed works on codec removal Tristan Madani
  2026-10-30 15:41 ` [PATCH v3 1/4] ASoC: rt5668: cancel delayed works on component removal Tristan Madani
  2026-10-30 15:42 ` [PATCH v3 2/4] ASoC: rt5659: cancel jack_detect_work " Tristan Madani
@ 2026-10-30 15:42 ` Tristan Madani
  2026-10-30 15:42 ` [PATCH v3 4/4] ASoC: wm8994: drain delayed works on remove and probe error Tristan Madani
  3 siblings, 0 replies; 7+ messages in thread
From: Tristan Madani @ 2026-10-30 15:42 UTC (permalink / raw)
  To: Mark Brown
  Cc: Oder Chiou, Liam Girdwood, Jaroslav Kysela, Takashi Iwai,
	Bard Liao, Misael Lopez Cruz, patches, linux-sound, stable,
	Tristan Madani

The twl6040 codec driver initializes a delayed work for headset jack
detection (hs_jack.work) but the component remove callback only frees
the plug IRQ without cancelling the pending work.

Free the IRQ first so that the threaded handler cannot schedule new
work, then cancel any residual queued work synchronously.  This
ordering prevents the race where the handler fires between a cancel
and the free_irq, re-arming the work after it was 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>
---

Changes in v3:
  - Swap ordering: free_irq() first, then cancel residual work.
    Prevents handler from re-arming work between cancel and free_irq.

Changes in v2:
  - Resend as part of unified series (no code changes from v1)
 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


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

* [PATCH v3 4/4] ASoC: wm8994: drain delayed works on remove and probe error
  2026-10-30 15:41 [PATCH v3 0/4] ASoC: cancel pending delayed works on codec removal Tristan Madani
                   ` (2 preceding siblings ...)
  2026-10-30 15:42 ` [PATCH v3 3/4] ASoC: twl6040: cancel headset jack work " Tristan Madani
@ 2026-10-30 15:42 ` Tristan Madani
  3 siblings, 0 replies; 7+ messages in thread
From: Tristan Madani @ 2026-10-30 15:42 UTC (permalink / raw)
  To: Mark Brown
  Cc: Oder Chiou, Liam Girdwood, Jaroslav Kysela, Takashi Iwai,
	Bard Liao, Misael Lopez Cruz, patches, linux-sound, stable,
	Tristan Madani

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 and after jackdet_bootstrap because the two can
re-arm each other: 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
wm1811_jackdet_bootstrap() detects a jack and ultimately queues
mic_work.  The second mic_work cancel catches any re-arm from a
jackdet_bootstrap handler that was in flight during the first cancel.

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 in v3:
  - No code change from v2

Changes in v2:
  - Resend as part of unified series
  - Cancel mic_work before and after jackdet_bootstrap for cyclic re-arm
 sound/soc/codecs/wm8994.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/sound/soc/codecs/wm8994.c b/sound/soc/codecs/wm8994.c
index 8bf58a6a8af7c..b9e0df83b9bfe 100644
--- a/sound/soc/codecs/wm8994.c
+++ b/sound/soc/codecs/wm8994.c
@@ -4551,6 +4551,13 @@ 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);
+	if (control->type == WM8994 || control->type == WM1811)
+		cancel_delayed_work_sync(&wm8994->mic_work);
+	cancel_delayed_work_sync(&wm8994->mic_complete_work);
+	cancel_delayed_work_sync(&wm8994->open_circuit_work);
 
 	return ret;
 }
@@ -4592,6 +4599,13 @@ 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);
+	if (control->type == WM8994 || control->type == WM1811)
+		cancel_delayed_work_sync(&wm8994->mic_work);
+	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


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

* Re: [PATCH v3 2/4] ASoC: rt5659: cancel jack_detect_work on component removal
  2026-10-30 15:42 ` [PATCH v3 2/4] ASoC: rt5659: cancel jack_detect_work " Tristan Madani
@ 2026-10-30 15:53   ` sashiko-bot
  0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-10-30 15:53 UTC (permalink / raw)
  To: Tristan Madani; +Cc: linux-sound

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] ASoC: rt5659: uncancelled jack_detect_work on I2C teardown without card
- [High] ASoC: rt5659: broken jack detect and dangling hs_jack in rt5659_probe()
--

commit 2bb6cdac1d7b83279322bc0304f15b740f1928f7
Author: Tristan Madani <tristmd@gmail.com>

ASoC: rt5659: cancel jack_detect_work on component removal

This commit disables the codec IRQ and cancels pending jack_detect_work in
rt5659_remove() before resetting the codec, and re-enables the IRQ in
rt5659_probe() when a component is rebound.

> diff --git a/sound/soc/codecs/rt5659.c b/sound/soc/codecs/rt5659.c
> index af8051fa69f89..68f7d5673e190 100644
> --- a/sound/soc/codecs/rt5659.c
> +++ b/sound/soc/codecs/rt5659.c
> @@ -3667,6 +3667,11 @@ static int rt5659_probe(struct snd_soc_component *component)
>  
>  	rt5659->component = component;
>  
> +	if (rt5659->irq_disabled) {
> +		enable_irq(to_i2c_client(component->dev)->irq);
> +		rt5659->irq_disabled = false;
> +	}

[Severity: High]
As reported by Sashiko on v2 [1], does re-enabling the IRQ in rt5659_probe()
restore jack detection or leave a dangling rt5659->hs_jack pointer on card
rebind?

At the end of rt5659_remove(), writing RT5659_RESET resets all codec hardware
registers to defaults, wiping out the GPIO1 IRQ pin mux (RT5659_GP1_PIN_IRQ in
RT5659_GPIO_CTRL_1) and the jack-detect and IRQ registers (RT5659_EJD_CTRL_1,
RT5659_RC_CLK_CTRL, RT5659_JD_CTRL_1, RT5659_IRQ_CTRL_1, RT5659_IRQ_CTRL_2,
RT5659_PWR_ANLG_1, and RT5659_PWR_ANLG_2) that were only configured in
rt5659_i2c_probe(). Because RT5659_RESET is volatile in
rt5659_volatile_register(), the REGCACHE_RBTREE regmap cache is also not
marked dirty or resynced on re-probe, so the codec cannot generate jack
detection interrupts after rebind.

In addition, rt5659_remove() does not clear rt5659->hs_jack or
rt5659->component. When a sound card unbinds and its card-owned struct
snd_soc_jack is freed, a subsequent card bind invokes
snd_soc_component_probe() -> rt5659_probe() before the machine driver's init
callback registers a new jack. Calling enable_irq() here while rt5659->hs_jack
still points to the freed jack from the previous card allows any interrupt to
schedule jack_detect_work, which passes the dangling rt5659->hs_jack pointer
to snd_soc_jack_report():

sound/soc/codecs/rt5659.c:rt5659_jack_detect_intel_hd_header() {
	...
	if (!rt5659->hs_jack)
		return;
	...
	snd_soc_jack_report(rt5659->hs_jack, rt5659->jack_type,
		SND_JACK_HEADSET);
}

>  
>  	switch (rt5659->pdata.jd_src) {
>  	case RT5659_JD_HDA_HEADER:
>  		break;
> @@ -3684,6 +3689,15 @@ 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;
> +	}
> +
> +	if (rt5659->jack_detect_work_inited)
> +		cancel_delayed_work_sync(&rt5659->jack_detect_work);
>  
>  	regmap_write(rt5659->regmap, RT5659_RESET, 0);
>  }

[Severity: High]
As reported by Sashiko on v2 [2], can rt5659->jack_detect_work still remain
queued and cause a use-after-free of struct rt5659_priv and rt5659->regmap
when the I2C device is unbound without a bound sound card?

In rt5659_i2c_probe(), when rt5659->pdata.jd_src is RT5659_JD3 or
RT5659_JD_HDA_HEADER, the driver initializes rt5659->jack_detect_work,
configures the jack-detection interrupt registers, registers rt5659_irq() via
devm_request_threaded_irq(), and enables RT5659_GP1_PIN_IRQ before calling
devm_snd_soc_register_component(). Once the IRQ is registered, an interrupt on
i2c->irq runs rt5659_irq() and queues rt5659->jack_detect_work on
system_power_efficient_wq with a 250 ms delay.

However, disable_irq() and cancel_delayed_work_sync() are only placed in
rt5659_remove(), which is the snd_soc_component_driver remove callback. In the
ASoC core, component removal skips the driver remove callback whenever no card
is bound to the component:

sound/soc/soc-core.c:snd_soc_del_component_unlocked() {
	...
	if (card) {
		instantiated = card->instantiated;
		snd_soc_unbind_card(card);
		if (instantiated)
			list_add(&card->list, &unbind_card_list);
	}
}

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

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

If devm_snd_soc_register_component() fails in rt5659_i2c_probe(), or if the
I2C device is unbound while component->card is NULL, rt5659_remove() is never
called and devres frees the IRQ, regmap, and struct rt5659_priv while
rt5659->jack_detect_work is still pending or running. Similarly,
rt5659_i2c_shutdown() resets the codec via RT5659_RESET without disabling the
IRQ or cancelling rt5659->jack_detect_work.

Would registering the delayed work with devm_delayed_work_autocancel() (or
handling teardown at the I2C driver level) ensure the work is always cancelled
after the IRQ is freed on all unbind paths?

[1] https://lore.kernel.org/r/20261029212504.2997364-3-tristmd@gmail.com
[2] https://lore.kernel.org/r/20261029212504.2997364-3-tristmd@gmail.com

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030154202.2989936-1-tristan@talencesecurity.com?part=2

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

* Re: [PATCH v3 1/4] ASoC: rt5668: cancel delayed works on component removal
  2026-10-30 15:41 ` [PATCH v3 1/4] ASoC: rt5668: cancel delayed works on component removal Tristan Madani
@ 2026-10-30 15:56   ` sashiko-bot
  0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-10-30 15:56 UTC (permalink / raw)
  To: Tristan Madani; +Cc: linux-sound

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] ASoC: rt5668: jack_detect_work UAF on I2C unbind in rt5668_remove()
- [High] ASoC: rt5668: unbalanced disable_irq() in rt5668_remove() on card rebind
- [Medium] ASoC: rt5668: disable_irq() in rt5668_remove() when IRQ request failed
--

commit d5c07e49b35a8b8aca80e062381c33477ad1f222
Author: Tristan Madani <tristmd@gmail.com>

ASoC: rt5668: cancel delayed works on component removal

This commit disables the codec IRQ and cancels jack_detect_work and
jd_check_work in rt5668_remove() to prevent work handlers from running
after removal.

> diff --git a/sound/soc/codecs/rt5668.c b/sound/soc/codecs/rt5668.c
> index cd6245d6b91af..7501c6c6ee128 100644
> --- a/sound/soc/codecs/rt5668.c
> +++ b/sound/soc/codecs/rt5668.c
> @@ -2222,6 +2222,13 @@ 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 (i2c->irq)
> +		disable_irq(i2c->irq);

[Severity: High]
Does calling disable_irq(i2c->irq) in the ASoC component remove callback
rt5668_remove() leave the IRQ permanently disabled across sound card
unbind/rebind cycles or deferred probe rollbacks?

In rt5668_i2c_probe(), i2c->irq is requested once at I2C device probe time
via devm_request_threaded_irq(), whereas rt5668_probe() and rt5668_remove()
are snd_soc_component_driver callbacks invoked when an ASoC card binds or
unbinds the component:

sound/soc/codecs/rt5668.c:rt5668_probe() {
	struct rt5668_priv *rt5668 = snd_soc_component_get_drvdata(component);

	rt5668->component = component;

	return 0;
}

If the sound card is unbound and later rebound while the I2C device stays
bound, or if snd_soc_bind_card() fails or defers on a later component after
rt5668_probe() succeeds (causing soc_cleanup_card_resources() ->
soc_remove_link_components() -> snd_soc_component_remove() to run),
rt5668_remove() calls disable_irq(i2c->irq). Because neither rt5668_probe()
nor rt5668_set_jack_detect() calls enable_irq(i2c->irq), the IRQ disable
depth remains incremented on subsequent card binds.

[Severity: Medium]
Also, what happens here if devm_request_threaded_irq() failed during
rt5668_i2c_probe()?

In rt5668_i2c_probe(), a failure to request the IRQ is logged as non-fatal
while i2c->irq remains non-zero:

	if (i2c->irq) {
		ret = devm_request_threaded_irq(&i2c->dev, i2c->irq, NULL,
			rt5668_irq, IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING
			| IRQF_ONESHOT, "rt5668", rt5668);
		if (ret)
			dev_err(&i2c->dev, "Failed to request IRQ: %d\n", ret);

	}

When rt5668_remove() later checks only if (i2c->irq), won't it call
disable_irq(i2c->irq) on an IRQ line that rt5668 never successfully
claimed, causing an unbalanced IRQ disable?

> +
> +	cancel_delayed_work_sync(&rt5668->jack_detect_work);
> +	cancel_delayed_work_sync(&rt5668->jd_check_work);

[Severity: High]
As reported by Sashiko on v2 [1], can rt5668->jack_detect_work still cause
a use-after-free when the I2C device is unbound while no sound card is
bound?

In rt5668_i2c_probe(), rt5668->jack_detect_work and rt5668->jd_check_work
are initialized and rt5668_irq() is registered via
devm_request_threaded_irq() before devm_snd_soc_register_component(). If an
interrupt fires before a sound card is bound or instantiated, rt5668_irq()
queues rt5668->jack_detect_work, and rt5668_jack_detect_handler() re-arms
itself every 15 ms:

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

When the I2C device is unbound while component->card is NULL,
devm_component_release() -> snd_soc_unregister_component_by_driver() ->
snd_soc_del_component_unlocked() skips snd_soc_unbind_card() because card is
NULL, so snd_soc_component_remove() and rt5668_remove() are never called.
Because rt5668_i2c_driver has no i2c_driver.remove callback (or devm action)
to disable the IRQ and cancel the delayed works on I2C unbind, devres frees
rt5668 while rt5668->jack_detect_work remains pending. Should the IRQ
disable and work cancellation be handled at the I2C device lifecycle level
(similar to rt5682-i2c.c and rt5682s.c) rather than only in rt5668_remove()?

[1] https://lore.kernel.org/r/sashiko-outbox-190014@kernel.org

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030154202.2989936-1-tristan@talencesecurity.com?part=1

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

end of thread, other threads:[~2026-10-30 15:56 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-30 15:41 [PATCH v3 0/4] ASoC: cancel pending delayed works on codec removal Tristan Madani
2026-10-30 15:41 ` [PATCH v3 1/4] ASoC: rt5668: cancel delayed works on component removal Tristan Madani
2026-10-30 15:56   ` sashiko-bot
2026-10-30 15:42 ` [PATCH v3 2/4] ASoC: rt5659: cancel jack_detect_work " Tristan Madani
2026-10-30 15:53   ` sashiko-bot
2026-10-30 15:42 ` [PATCH v3 3/4] ASoC: twl6040: cancel headset jack work " Tristan Madani
2026-10-30 15:42 ` [PATCH v3 4/4] ASoC: wm8994: drain delayed works on remove and probe error Tristan Madani

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.