All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/4] ASoC: cancel pending delayed works on codec removal
@ 2026-10-30 17:04 Tristan Madani
  2026-10-30 17:04 ` [PATCH v4 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 17:04 UTC (permalink / raw)
  To: Mark Brown, Oder Chiou, Liam Girdwood, Jaroslav Kysela,
	Takashi Iwai
  Cc: 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 v3:
  - rt5668 (1/4): track IRQ acquisition with irq_acquired flag to
    avoid calling disable_irq() when devm_request_threaded_irq()
    failed.  Re-enable IRQ in rt5668_probe() when a sound card
    rebinds after rt5668_remove() disabled it, preventing permanent
    IRQ silencing across unbind/rebind cycles.
  - Patches 2-4/4: no code changes.

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

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

-- 
2.47.3


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

* [PATCH v4 1/4] ASoC: rt5668: cancel delayed works on component removal
  2026-10-30 17:04 [PATCH v4 0/4] ASoC: cancel pending delayed works on codec removal Tristan Madani
@ 2026-10-30 17:04 ` Tristan Madani
  2026-10-30 17:22   ` sashiko-bot
  2026-10-30 17:04 ` [PATCH v4 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 17:04 UTC (permalink / raw)
  To: Mark Brown, Oder Chiou, Liam Girdwood, Jaroslav Kysela,
	Takashi Iwai
  Cc: Bard Liao, Misael Lopez Cruz, patches, linux-sound, stable,
	Tristan Madani

rt5668_remove() does not disable the IRQ or cancel the pending
jack_detect_work and jd_check_work delayed works.  If the component is
unbound while work is pending, the handlers access freed memory.

Track IRQ acquisition in rt5668_i2c_probe() via an irq_acquired flag.
In rt5668_remove(), disable the IRQ (if acquired) and cancel both
delayed works before resetting the codec.  Re-enable the IRQ in
rt5668_probe() when a sound card rebinds to prevent permanent IRQ
silencing across card unbind/rebind cycles.

Fixes: e13da14e8543 ("ASoC: add rt5668 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..ab63a43b20b89 100644
--- a/sound/soc/codecs/rt5668.c
+++ b/sound/soc/codecs/rt5668.c
@@ -62,6 +62,9 @@ struct rt5668_priv {
 	int pll_out;
 
 	int jack_type;
+
+	bool irq_acquired;
+	bool irq_disabled;
 };
 
 static const struct reg_default rt5668_reg[] = {
@@ -2216,12 +2219,26 @@ static int rt5668_probe(struct snd_soc_component *component)
 
 	rt5668->component = component;
 
+	if (rt5668->irq_disabled) {
+		enable_irq(to_i2c_client(component->dev)->irq);
+		rt5668->irq_disabled = false;
+	}
+
 	return 0;
 }
 
 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);
 }
@@ -2533,7 +2550,8 @@ static int rt5668_i2c_probe(struct i2c_client *i2c)
 			| IRQF_ONESHOT, "rt5668", rt5668);
 		if (ret)
 			dev_err(&i2c->dev, "Failed to request IRQ: %d\n", ret);
-
+		else
+			rt5668->irq_acquired = true;
 	}
 
 	return devm_snd_soc_register_component(&i2c->dev, &soc_component_dev_rt5668,
-- 
2.47.3


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

* [PATCH v4 2/4] ASoC: rt5659: cancel jack_detect_work on component removal
  2026-10-30 17:04 [PATCH v4 0/4] ASoC: cancel pending delayed works on codec removal Tristan Madani
  2026-10-30 17:04 ` [PATCH v4 1/4] ASoC: rt5668: cancel delayed works on component removal Tristan Madani
@ 2026-10-30 17:04 ` Tristan Madani
  2026-10-30 17:17   ` sashiko-bot
  2026-10-30 17:04 ` [PATCH v4 3/4] ASoC: twl6040: cancel headset jack work " Tristan Madani
  2026-10-30 17:04 ` [PATCH v4 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 17:04 UTC (permalink / raw)
  To: Mark Brown, Oder Chiou, Liam Girdwood, Jaroslav Kysela,
	Takashi Iwai
  Cc: 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>
---
 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 v4 3/4] ASoC: twl6040: cancel headset jack work on component removal
  2026-10-30 17:04 [PATCH v4 0/4] ASoC: cancel pending delayed works on codec removal Tristan Madani
  2026-10-30 17:04 ` [PATCH v4 1/4] ASoC: rt5668: cancel delayed works on component removal Tristan Madani
  2026-10-30 17:04 ` [PATCH v4 2/4] ASoC: rt5659: cancel jack_detect_work " Tristan Madani
@ 2026-10-30 17:04 ` Tristan Madani
  2026-10-30 17:04 ` [PATCH v4 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 17:04 UTC (permalink / raw)
  To: Mark Brown, Oder Chiou, Liam Girdwood, Jaroslav Kysela,
	Takashi Iwai
  Cc: 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>
---
 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 v4 4/4] ASoC: wm8994: drain delayed works on remove and probe error
  2026-10-30 17:04 [PATCH v4 0/4] ASoC: cancel pending delayed works on codec removal Tristan Madani
                   ` (2 preceding siblings ...)
  2026-10-30 17:04 ` [PATCH v4 3/4] ASoC: twl6040: cancel headset jack work " Tristan Madani
@ 2026-10-30 17:04 ` Tristan Madani
  3 siblings, 0 replies; 7+ messages in thread
From: Tristan Madani @ 2026-10-30 17:04 UTC (permalink / raw)
  To: Mark Brown, Oder Chiou, Liam Girdwood, Jaroslav Kysela,
	Takashi Iwai
  Cc: 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>
---
 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 v4 2/4] ASoC: rt5659: cancel jack_detect_work on component removal
  2026-10-30 17:04 ` [PATCH v4 2/4] ASoC: rt5659: cancel jack_detect_work " Tristan Madani
@ 2026-10-30 17:17   ` sashiko-bot
  0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-10-30 17:17 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 54b53c52a3c171380da559e1b3114044473f6136
Author: Tristan Madani <tristmd@gmail.com>

ASoC: rt5659: cancel jack_detect_work on component removal

This commit tracks IRQ acquisition and delayed work initialization in
struct rt5659_priv so that rt5659_remove() disables the IRQ and cancels
jack_detect_work before resetting the codec, and rt5659_probe()
re-enables the IRQ on subsequent component binds.

> 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 v3 [1], does re-enabling the host IRQ here on
card rebind actually restore jack detection, or can it trigger a
use-after-free on rt5659->hs_jack?

At the end of rt5659_remove(), the codec is software-reset:

	regmap_write(rt5659->regmap, RT5659_RESET, 0);

Writing RT5659_RESET resets all codec hardware registers to power-on
defaults, wiping out the GPIO1 IRQ pin mux (RT5659_GP1_PIN_IRQ in
RT5659_GPIO_CTRL_1) and the jack-detection and IRQ control registers
(such as RT5659_EJD_CTRL_1, RT5659_RC_CLK_CTRL, RT5659_GPIO_CTRL_3,
RT5659_JD_CTRL_1, RT5659_IRQ_CTRL_1, and RT5659_IRQ_CTRL_2) that were
configured only once in rt5659_i2c_probe(). Because RT5659_RESET is a
volatile register, the regmap cache is not marked dirty or resynced on
re-probe either, leaving the codec unable to generate jack interrupts
after card 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, rt5659->hs_jack retains the dangling pointer. When
a card subsequently binds again, snd_soc_component_probe() calls
rt5659_probe() before the machine driver's init callback registers a new
jack. Calling enable_irq() at that point unmasks the IRQ while
rt5659->hs_jack still points to the freed jack, so an interrupt firing
before a new jack is set (or if the new card does not call
rt5659_set_jack_detect()) will schedule jack_detect_work and pass the
dangling rt5659->hs_jack pointer to snd_soc_jack_report().

>  	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 v3 [2], what happens if the I2C device is
unbound while no ASoC sound card is bound to the component, or if
devm_snd_soc_register_component() fails in rt5659_i2c_probe()?

In rt5659_i2c_probe(), when rt5659->pdata.jd_src is RT5659_JD3 or
RT5659_JD_HDA_HEADER, the driver initializes rt5659->jack_detect_work,
enables codec jack-detection interrupts, requests rt5659_irq() via
devm_request_threaded_irq(), and enables the GPIO1 IRQ output pin before
calling devm_snd_soc_register_component(). Once the IRQ is requested, any
interrupt on i2c->irq runs rt5659_irq() and queues
rt5659->jack_detect_work with a 250 ms delay.

However, in the ASoC core, snd_soc_del_component_unlocked() and
snd_soc_component_remove() only invoke component->driver->remove
(rt5659_remove) when component->card is non-NULL and the component has
been probed:

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

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

Because rt5659_i2c_driver defines no I2C remove callback and registers no
devm action (such as devm_delayed_work_autocancel()) to cancel
jack_detect_work, unbinding the I2C device without a bound sound card (or
failing devm_snd_soc_register_component()) skips rt5659_remove()
entirely. devres then frees the IRQ, regmap, and struct rt5659_priv while
rt5659->jack_detect_work remains pending, leading to a use-after-free when
the delayed work executes.

Similarly, rt5659_i2c_shutdown() writes RT5659_RESET without disabling the
IRQ or cancelling rt5659->jack_detect_work, allowing the work to run
against a reset codec during system shutdown.

[1] https://lore.kernel.org/r/20261030154202.2989936-3-tristan@talencesecurity.com
[2] https://lore.kernel.org/r/20261030154202.2989936-3-tristan@talencesecurity.com

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

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

* Re: [PATCH v4 1/4] ASoC: rt5668: cancel delayed works on component removal
  2026-10-30 17:04 ` [PATCH v4 1/4] ASoC: rt5668: cancel delayed works on component removal Tristan Madani
@ 2026-10-30 17:22   ` sashiko-bot
  0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-10-30 17:22 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: uncancelled delayed work on I2C unbind in rt5668_remove()
- [High] ASoC: rt5668: stale jack and regcache state across rebind in rt5668_remove()
- [Low] ASoC: rt5668: invalid commit SHA in Fixes tag
--

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

ASoC: rt5668: cancel delayed works on component removal

This patch tracks IRQ acquisition in rt5668_i2c_probe(), disables the IRQ and
cancels pending jack detection delayed works in rt5668_remove(), and
re-enables the IRQ in rt5668_probe() when a sound card rebinds.

>     ASoC: rt5668: cancel delayed works on component removal
>     
>     rt5668_remove() does not disable the IRQ or cancel the pending
>     jack_detect_work and jd_check_work delayed works.  If the component is
>     unbound while work is pending, the handlers access freed memory.
>     
>     Track IRQ acquisition in rt5668_i2c_probe() via an irq_acquired flag.
>     In rt5668_remove(), disable the IRQ (if acquired) and cancel both
>     delayed works before resetting the codec.  Re-enable the IRQ in
>     rt5668_probe() when a sound card rebinds to prevent permanent IRQ
>     silencing across card unbind/rebind cycles.
>     
>     Fixes: e13da14e8543 ("ASoC: add rt5668 codec driver")
>     Cc: stable@vger.kernel.org
>     Signed-off-by: Tristan Madani <tristan@talencesecurity.com>

[Severity: Low]
Does commit e13da14e8543 ("ASoC: add rt5668 codec driver") exist in the git
tree? It looks like sound/soc/codecs/rt5668.c was introduced by:

  Fixes: d59fb2856223 ("ASoC: rt5668: add rt5668B codec driver")

> diff --git a/sound/soc/codecs/rt5668.c b/sound/soc/codecs/rt5668.c
> index cd6245d6b91af..29d70802a046a 100644
> --- a/sound/soc/codecs/rt5668.c
> +++ b/sound/soc/codecs/rt5668.c

[ ... ]

> @@ -2216,12 +2219,26 @@ static int rt5668_probe(struct snd_soc_component *component)
>  
>  	rt5668->component = component;
>  
> +	if (rt5668->irq_disabled) {
> +		enable_irq(to_i2c_client(component->dev)->irq);
> +		rt5668->irq_disabled = false;
> +	}
> +
>  	return 0;
>  }
>  
>  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]
As reported by Sashiko on v2 [1], if the I2C device is unbound while no sound
card is bound, can rt5668_jack_detect_handler() still run after devres frees
rt5668?

In rt5668_i2c_probe(), the driver initializes jack_detect_work and
jd_check_work and immediately requests and enables i2c->irq before
devm_snd_soc_register_component() and before any sound card binds:

sound/soc/codecs/rt5668.c:rt5668_i2c_probe() {
	INIT_DELAYED_WORK(&rt5668->jack_detect_work,
				rt5668_jack_detect_handler);
	INIT_DELAYED_WORK(&rt5668->jd_check_work,
				rt5668_jd_check_handler);
	...
	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 an interrupt fires while no card is bound or instantiated (or if
devm_snd_soc_register_component() fails), rt5668_irq() queues
rt5668->jack_detect_work, which re-arms itself every 15 ms:

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 rt5668_remove() is the snd_soc_component_driver.remove callback and
rt5668_i2c_driver has no .remove callback or devm cleanup action, unbinding
the I2C device when component->card is NULL goes through:

devm_component_release()
  -> snd_soc_unregister_component_by_driver()
    -> snd_soc_del_component_unlocked()

which skips snd_soc_unbind_card() and snd_soc_component_remove() when card is
NULL. As a result, rt5668_remove() is never called on I2C unbind without a
bound card, and devres frees the IRQ and struct rt5668_priv while
rt5668->jack_detect_work remains pending and self-rearming.

Should the delayed work cancellation (or a devm action / i2c_driver.remove
callback) also run on I2C teardown?

[Severity: High]
Also in rt5668_probe() and rt5668_remove(), when re-enabling the IRQ across a
sound card unbind and rebind cycle, does rt5668_remove() leave stale state in
rt5668->hs_jack, rt5668->jack_type, and the REGCACHE_MAPLE regmap cache after
calling rt5668_reset()?

Specifically, when rt5668_remove() calls rt5668_reset(rt5668->regmap), it
resets the hardware registers to power-on defaults via the volatile
RT5668_RESET register without resetting or marking dirty the regmap cache, and
without clearing rt5668->hs_jack, rt5668->jack_type, or rt5668->component:

1. When the previous card unbound, soc_cleanup_card_resources() freed its
   snd_jack. If the machine driver allocated struct snd_soc_jack with devm on
   the card device without calling snd_soc_component_set_jack(component, NULL,
   NULL) on teardown, rt5668->hs_jack is left dangling. When rt5668_probe()
   calls enable_irq() on rebind before set_jack updates hs_jack (or if the new
   card does not call set_jack), an interrupt will cause
   rt5668_jack_detect_handler() to pass the freed rt5668->hs_jack pointer to
   snd_soc_jack_report().

2. When the rebound card calls rt5668_set_jack_detect(), regmap_update_bits()
   and snd_soc_component_update_bits() on non-volatile cached registers such
   as RT5668_CBJ_CTRL_3, RT5668_SAR_IL_CMD_1, RT5668_GPIO_CTRL_1,
   RT5668_RC_CLK_CTRL, RT5668_PWR_ANLG_2, and RT5668_IRQ_CTRL_2:

sound/soc/codecs/rt5668.c:rt5668_set_jack_detect() {
		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);
}

   will see those bits already set in the regmap cache from the first bind and
   skip writing them to the reset hardware, leaving the codec IRQ output pin
   and jack detection disabled in hardware.

3. If a headset was plugged in when the card unbound (rt5668->jack_type != 0),
   on rebind rt5668_jack_detect_handler() will see rt5668->jack_type != 0 and
   skip initial headset detection via rt5668_headset_detect(rt5668->component,
   1):

sound/soc/codecs/rt5668.c:rt5668_jack_detect_handler() {
		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);
}

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

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

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

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

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-30 17:04 [PATCH v4 0/4] ASoC: cancel pending delayed works on codec removal Tristan Madani
2026-10-30 17:04 ` [PATCH v4 1/4] ASoC: rt5668: cancel delayed works on component removal Tristan Madani
2026-10-30 17:22   ` sashiko-bot
2026-10-30 17:04 ` [PATCH v4 2/4] ASoC: rt5659: cancel jack_detect_work " Tristan Madani
2026-10-30 17:17   ` sashiko-bot
2026-10-30 17:04 ` [PATCH v4 3/4] ASoC: twl6040: cancel headset jack work " Tristan Madani
2026-10-30 17:04 ` [PATCH v4 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.