[PATCH] ASoC: rt5665: cancel jack detect work on removal to avoid use-after-free

[PATCH] ASoC: rt5665: cancel jack detect work on removal to avoid use-after-free

[Hyunjung Ko]

rt5665_jack_detect_work() runs from the delayed work jack_detect_work,
which is queued from the jack IRQ handler and from the jack-detect setup,
and the jd_check_work polls button state; both dereference the driver
data via container_of().

struct rt5665_priv and the jack IRQ are both allocated with devm against
the i2c device. The i2c driver has no .remove callback, and the component
.remove and the .shutdown callback only reset the chip; neither disables
the IRQ nor cancels the delayed work. On unbind devres frees the IRQ and
rt5665_priv while jack_detect_work (or jd_check_work) can still be queued,
so the pending work then runs against freed memory (use-after-free).

Disable the IRQ and cancel both delayed works before the chip reset, and
wire that teardown into a new i2c .remove callback so it runs on unbind,
mirroring the sibling rt5682 driver.

Found by static analysis and code review; not runtime-reproduced (needs
RT5665 hardware).

Fixes: 33ada14a26c8 ("ASoC: add rt5665 codec driver")
Cc: stable@vger.kernel.org
Signed-off-by: Hyunjung Ko <hj351016@gmail.com>
---
 sound/soc/codecs/rt5665.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/sound/soc/codecs/rt5665.c b/sound/soc/codecs/rt5665.c
index 0000000..0000000 100644
--- a/sound/soc/codecs/rt5665.c
+++ b/sound/soc/codecs/rt5665.c
@@ -4837,9 +4837,19 @@ static void rt5665_i2c_shutdown(struct i2c_client *client)
 {
 	struct rt5665_priv *rt5665 = i2c_get_clientdata(client);

+	if (client->irq)
+		disable_irq(client->irq);
+	cancel_delayed_work_sync(&rt5665->jack_detect_work);
+	cancel_delayed_work_sync(&rt5665->jd_check_work);
+
 	regmap_write(rt5665->regmap, RT5665_RESET, 0);
 }

+static void rt5665_i2c_remove(struct i2c_client *client)
+{
+	rt5665_i2c_shutdown(client);
+}
+
 #ifdef CONFIG_OF
 static const struct of_device_id rt5665_of_match[] = {
 	{.compatible = "realtek,rt5665"},
@@ -4865,6 +4875,7 @@ static struct i2c_driver rt5665_i2c_driver = {
 		.acpi_match_table = ACPI_PTR(rt5665_acpi_match),
 	},
 	.probe = rt5665_i2c_probe,
+	.remove = rt5665_i2c_remove,
 	.shutdown = rt5665_i2c_shutdown,
 	.id_table = rt5665_i2c_id,
 };
--
2.43.0

Re: [PATCH] ASoC: rt5665: cancel jack detect work on removal to avoid use-after-free

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential
issues to consider:
- [High] ASoC: rt5665: infinite hang in rt5665_i2c_shutdown() canceling
  jack_detect_work
- [High] ASoC: rt5665: uncanceled calibrate_work in
  rt5665_i2c_shutdown() causes UAF
- [Medium] ASoC: rt5665: unconditional disable_irq() in
  rt5665_i2c_shutdown() on IRQ error
- [Low] ASoC: rt5665: inaccurate handler name and queuing site in commit
  message
--

commit 753d54cd6a5c4d1a6c0962dfc60f96a690d96477
Author: Hyunjung Ko <hj351016@gmail.com>

ASoC: rt5665: cancel jack detect work on removal to avoid use-after-free

This commit disables the client IRQ and cancels jack_detect_work and
jd_check_work in rt5665_i2c_shutdown(), and wires up an I2C .remove callback
that invokes rt5665_i2c_shutdown().

> rt5665_jack_detect_work() runs from the delayed work jack_detect_work,
> which is queued from the jack IRQ handler and from the jack-detect setup,
> and the jd_check_work polls button state; both dereference the driver
> data via container_of().

[Severity: Low]
In sound/soc/codecs/rt5665.c, the handler for jack_detect_work is named
rt5665_jack_detect_handler(), and it is only queued from rt5665_irq();
rt5665_set_jack_detect() only configures registers and stores hs_jack.

> diff --git a/sound/soc/codecs/rt5665.c b/sound/soc/codecs/rt5665.c
> index 62b21092e352b..bce2fc8c9bc6b 100644
> --- a/sound/soc/codecs/rt5665.c
> +++ b/sound/soc/codecs/rt5665.c
> @@ -4837,9 +4837,19 @@ static void rt5665_i2c_shutdown(struct i2c_client *client)
>  {
>  	struct rt5665_priv *rt5665 = i2c_get_clientdata(client);
>  
> +	if (client->irq)
> +		disable_irq(client->irq);

[Severity: Medium]
Can this disable an IRQ that the driver never acquired?

In rt5665_i2c_probe(), a failure in devm_request_threaded_irq() only logs an
error and lets probe succeed with client->irq still set:

sound/soc/codecs/rt5665.c:rt5665_i2c_probe() {
	if (i2c->irq) {
		ret = devm_request_threaded_irq(&i2c->dev, i2c->irq, NULL,
			rt5665_irq, IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING
			| IRQF_ONESHOT, "rt5665", rt5665);
		if (ret)
			dev_err(&i2c->dev, "Failed to request IRQ: %d\n", ret);

	}
}

If that request fails, rt5665_i2c_shutdown() will still call
disable_irq(client->irq) on remove or shutdown, leaving the IRQ disable depth
unbalanced.

> +	cancel_delayed_work_sync(&rt5665->jack_detect_work);

[Severity: High]
Will cancel_delayed_work_sync(&rt5665->jack_detect_work) hang forever if the
device is unbound or shut down before the sound card is instantiated or
calibrated?

rt5665_i2c_probe() requests the IRQ before registering the component, and
rt5665_jack_detect_handler() spins inside the work callback waiting for the
component, card instantiation, and calibration:

sound/soc/codecs/rt5665.c:rt5665_jack_detect_handler() {
	while (!rt5665->component) {
		pr_debug("%s codec = null\n", __func__);
		usleep_range(10000, 15000);
	}

	while (!snd_soc_card_is_instantiated(rt5665->component->card)) {
		pr_debug("%s\n", __func__);
		usleep_range(10000, 15000);
	}

	while (!rt5665->calibration_done) {
		pr_debug("%s calibration not ready\n", __func__);
		usleep_range(10000, 15000);
	}
}

If an interrupt schedules jack_detect_work while any of those conditions is
false and the device is then unbound or shut down, cancel_delayed_work_sync()
waits indefinitely for a loop that will never make progress.  The sibling
drivers fixed this by rescheduling the delayed work instead of looping inside
the handler; see 4c33de0673ce ("ASoC: rt5682: do not block workqueue if card
is unbound") and a6d78661dc90 ("ASoC: rt5668: do not block workqueue if card
is unbound").

> +	cancel_delayed_work_sync(&rt5665->jd_check_work);

[Severity: High]
Does rt5665->calibrate_work also need to be canceled here?

Unlike rt5682, rt5665_probe() schedules rt5665->calibrate_work with a 100 ms
delay, and rt5665_calibrate_handler() -> rt5665_calibrate() sleeps across
polling loops while accessing rt5665, rt5665->component, and rt5665->regmap.
If the I2C device is unbound while calibrate_work is pending or running,
devres frees rt5665_priv and the work handler accesses freed memory.  Note
that rt5665_calibrate_handler() also has an unbounded while
(!snd_soc_card_is_instantiated(rt5665->component->card)) loop that would block
cancel_delayed_work_sync() if not converted to reschedule.

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

Re: [PATCH] ASoC: rt5665: cancel jack detect work on removal to avoid use-after-free

[khj]

Thanks for the thorough review - the points are all valid. The
jack_detect_work and calibrate_work handlers busy-wait for card
instantiation, so cancel_delayed_work_sync() on the teardown path can
hang, and calibrate_work needs the same handling. This needs the
reschedule rework the sibling drivers got (4c33de0673ce, a6d78661dc90)
before a cancel is safe, so please disregard this version.

Thanks,
Hyunjung

2026년 11월 10일 (화) 오전 2:45, <sashiko-bot@kernel.org>님이 작성:
>
> Thank you for your contribution! Sashiko AI review found 4 potential
> issues to consider:
> - [High] ASoC: rt5665: infinite hang in rt5665_i2c_shutdown() canceling
>   jack_detect_work
> - [High] ASoC: rt5665: uncanceled calibrate_work in
>   rt5665_i2c_shutdown() causes UAF
> - [Medium] ASoC: rt5665: unconditional disable_irq() in
>   rt5665_i2c_shutdown() on IRQ error
> - [Low] ASoC: rt5665: inaccurate handler name and queuing site in commit
>   message
> --
>
> commit 753d54cd6a5c4d1a6c0962dfc60f96a690d96477
> Author: Hyunjung Ko <hj351016@gmail.com>
>
> ASoC: rt5665: cancel jack detect work on removal to avoid use-after-free
>
> This commit disables the client IRQ and cancels jack_detect_work and
> jd_check_work in rt5665_i2c_shutdown(), and wires up an I2C .remove callback
> that invokes rt5665_i2c_shutdown().
>
> > rt5665_jack_detect_work() runs from the delayed work jack_detect_work,
> > which is queued from the jack IRQ handler and from the jack-detect setup,
> > and the jd_check_work polls button state; both dereference the driver
> > data via container_of().
>
> [Severity: Low]
> In sound/soc/codecs/rt5665.c, the handler for jack_detect_work is named
> rt5665_jack_detect_handler(), and it is only queued from rt5665_irq();
> rt5665_set_jack_detect() only configures registers and stores hs_jack.
>
> > diff --git a/sound/soc/codecs/rt5665.c b/sound/soc/codecs/rt5665.c
> > index 62b21092e352b..bce2fc8c9bc6b 100644
> > --- a/sound/soc/codecs/rt5665.c
> > +++ b/sound/soc/codecs/rt5665.c
> > @@ -4837,9 +4837,19 @@ static void rt5665_i2c_shutdown(struct i2c_client *client)
> >  {
> >       struct rt5665_priv *rt5665 = i2c_get_clientdata(client);
> >
> > +     if (client->irq)
> > +             disable_irq(client->irq);
>
> [Severity: Medium]
> Can this disable an IRQ that the driver never acquired?
>
> In rt5665_i2c_probe(), a failure in devm_request_threaded_irq() only logs an
> error and lets probe succeed with client->irq still set:
>
> sound/soc/codecs/rt5665.c:rt5665_i2c_probe() {
>         if (i2c->irq) {
>                 ret = devm_request_threaded_irq(&i2c->dev, i2c->irq, NULL,
>                         rt5665_irq, IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING
>                         | IRQF_ONESHOT, "rt5665", rt5665);
>                 if (ret)
>                         dev_err(&i2c->dev, "Failed to request IRQ: %d\n", ret);
>
>         }
> }
>
> If that request fails, rt5665_i2c_shutdown() will still call
> disable_irq(client->irq) on remove or shutdown, leaving the IRQ disable depth
> unbalanced.
>
> > +     cancel_delayed_work_sync(&rt5665->jack_detect_work);
>
> [Severity: High]
> Will cancel_delayed_work_sync(&rt5665->jack_detect_work) hang forever if the
> device is unbound or shut down before the sound card is instantiated or
> calibrated?
>
> rt5665_i2c_probe() requests the IRQ before registering the component, and
> rt5665_jack_detect_handler() spins inside the work callback waiting for the
> component, card instantiation, and calibration:
>
> sound/soc/codecs/rt5665.c:rt5665_jack_detect_handler() {
>         while (!rt5665->component) {
>                 pr_debug("%s codec = null\n", __func__);
>                 usleep_range(10000, 15000);
>         }
>
>         while (!snd_soc_card_is_instantiated(rt5665->component->card)) {
>                 pr_debug("%s\n", __func__);
>                 usleep_range(10000, 15000);
>         }
>
>         while (!rt5665->calibration_done) {
>                 pr_debug("%s calibration not ready\n", __func__);
>                 usleep_range(10000, 15000);
>         }
> }
>
> If an interrupt schedules jack_detect_work while any of those conditions is
> false and the device is then unbound or shut down, cancel_delayed_work_sync()
> waits indefinitely for a loop that will never make progress.  The sibling
> drivers fixed this by rescheduling the delayed work instead of looping inside
> the handler; see 4c33de0673ce ("ASoC: rt5682: do not block workqueue if card
> is unbound") and a6d78661dc90 ("ASoC: rt5668: do not block workqueue if card
> is unbound").
>
> > +     cancel_delayed_work_sync(&rt5665->jd_check_work);
>
> [Severity: High]
> Does rt5665->calibrate_work also need to be canceled here?
>
> Unlike rt5682, rt5665_probe() schedules rt5665->calibrate_work with a 100 ms
> delay, and rt5665_calibrate_handler() -> rt5665_calibrate() sleeps across
> polling loops while accessing rt5665, rt5665->component, and rt5665->regmap.
> If the I2C device is unbound while calibrate_work is pending or running,
> devres frees rt5665_priv and the work handler accesses freed memory.  Note
> that rt5665_calibrate_handler() also has an unbounded while
> (!snd_soc_card_is_instantiated(rt5665->component->card)) loop that would block
> cancel_delayed_work_sync() if not converted to reschedule.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20261109172144.1851173-1-hj351016@gmail.com?part=1