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

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

[Hyunjung Ko]

rt5668_jack_detect_handler() runs from the delayed work jack_detect_work,
which is queued from the jack IRQ handler rt5668_irq() and from
rt5668_set_jack_detect(), and re-arms itself while a button is held. The
handler and the delayed work jd_check_work dereference the driver data
via container_of().

struct rt5668_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
rt5668_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
RT5668B hardware).

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

diff --git a/sound/soc/codecs/rt5668.c b/sound/soc/codecs/rt5668.c
index 0000000..0000000 100644
--- a/sound/soc/codecs/rt5668.c
+++ b/sound/soc/codecs/rt5668.c
@@ -2566,9 +2566,19 @@ static void rt5668_i2c_shutdown(struct i2c_client *client)
 {
 	struct rt5668_priv *rt5668 = i2c_get_clientdata(client);

+	if (client->irq)
+		disable_irq(client->irq);
+	cancel_delayed_work_sync(&rt5668->jack_detect_work);
+	cancel_delayed_work_sync(&rt5668->jd_check_work);
+
 	rt5668_reset(rt5668->regmap);
 }

+static void rt5668_i2c_remove(struct i2c_client *client)
+{
+	rt5668_i2c_shutdown(client);
+}
+
 #ifdef CONFIG_OF
 static const struct of_device_id rt5668_of_match[] = {
 	{.compatible = "realtek,rt5668b"},
@@ -2592,6 +2602,7 @@ static struct i2c_driver rt5668_i2c_driver = {
 		.acpi_match_table = ACPI_PTR(rt5668_acpi_match),
 	},
 	.probe = rt5668_i2c_probe,
+	.remove = rt5668_i2c_remove,
 	.shutdown = rt5668_i2c_shutdown,
 	.id_table = rt5668_i2c_id,
 };
--
2.43.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] ASoC: rt5668: unbalanced disable_irq() in
  rt5668_i2c_shutdown()
--

commit 7ffec355074a97397bd6323b0aff3802254c40bf
Author: Hyunjung Ko <hj351016@gmail.com>

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

This commit disables the jack IRQ and cancels the jack detection delayed
works in rt5668_i2c_shutdown(), and hooks that path into a new i2c
.remove callback.

> diff --git a/sound/soc/codecs/rt5668.c b/sound/soc/codecs/rt5668.c
> index 29d70802a046a..c56bbe3fc7bea 100644
> --- a/sound/soc/codecs/rt5668.c
> +++ b/sound/soc/codecs/rt5668.c
> @@ -2584,9 +2584,19 @@ static void rt5668_i2c_shutdown(struct i2c_client *client)
>  {
>  	struct rt5668_priv *rt5668 = i2c_get_clientdata(client);
>  
> +	if (client->irq)
> +		disable_irq(client->irq);
> +	cancel_delayed_work_sync(&rt5668->jack_detect_work);
> +	cancel_delayed_work_sync(&rt5668->jd_check_work);
> +
>  	rt5668_reset(rt5668->regmap);
>  }
>  
> +static void rt5668_i2c_remove(struct i2c_client *client)
> +{
> +	rt5668_i2c_shutdown(client);
> +}

[Severity: Medium]
Does calling disable_irq(client->irq) unconditionally here conflict with
965bad6195d0 ("ASoC: rt5668: cancel delayed works on component removal")?

That commit updated the component remove callback rt5668_remove() to track
IRQ ownership and state via rt5668->irq_acquired and rt5668->irq_disabled,
cancel both delayed works, and reset the codec:

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

During i2c driver unbind, rt5668_i2c_remove() runs before devres
unregisters the component, so it resets the chip while the sound card is
still bound and then rt5668_remove() runs and calls disable_irq() and
rt5668_reset() a second time. In addition, rt5668_i2c_probe() continues if
devm_request_threaded_irq() fails, leaving client->irq non-zero with
rt5668->irq_acquired false, so unbind or shutdown disables an IRQ line the
driver never acquired.

The commit message also says that the component .remove callback only
resets the chip and neither disables the IRQ nor cancels the delayed work,
which is no longer true after 965bad6195d0 ("ASoC: rt5668: cancel delayed
works on component removal").

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

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

[khj]

Thanks for the review, and apologies for the noise.

You're right - this is already fixed by commit 965bad6195d0 ("ASoC:
rt5668: cancel delayed works on component removal"), which I missed as I
prepared this against an older tree. Please disregard this patch.

Thanks,
Hyunjung

2026년 11월 10일 (화) 오전 2:41, <sashiko-bot@kernel.org>님이 작성:
>
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue to consider:
> - [Medium] ASoC: rt5668: unbalanced disable_irq() in
>   rt5668_i2c_shutdown()
> --
>
> commit 7ffec355074a97397bd6323b0aff3802254c40bf
> Author: Hyunjung Ko <hj351016@gmail.com>
>
> ASoC: rt5668: cancel jack detect work on removal to avoid use-after-free
>
> This commit disables the jack IRQ and cancels the jack detection delayed
> works in rt5668_i2c_shutdown(), and hooks that path into a new i2c
> .remove callback.
>
> > diff --git a/sound/soc/codecs/rt5668.c b/sound/soc/codecs/rt5668.c
> > index 29d70802a046a..c56bbe3fc7bea 100644
> > --- a/sound/soc/codecs/rt5668.c
> > +++ b/sound/soc/codecs/rt5668.c
> > @@ -2584,9 +2584,19 @@ static void rt5668_i2c_shutdown(struct i2c_client *client)
> >  {
> >       struct rt5668_priv *rt5668 = i2c_get_clientdata(client);
> >
> > +     if (client->irq)
> > +             disable_irq(client->irq);
> > +     cancel_delayed_work_sync(&rt5668->jack_detect_work);
> > +     cancel_delayed_work_sync(&rt5668->jd_check_work);
> > +
> >       rt5668_reset(rt5668->regmap);
> >  }
> >
> > +static void rt5668_i2c_remove(struct i2c_client *client)
> > +{
> > +     rt5668_i2c_shutdown(client);
> > +}
>
> [Severity: Medium]
> Does calling disable_irq(client->irq) unconditionally here conflict with
> 965bad6195d0 ("ASoC: rt5668: cancel delayed works on component removal")?
>
> That commit updated the component remove callback rt5668_remove() to track
> IRQ ownership and state via rt5668->irq_acquired and rt5668->irq_disabled,
> cancel both delayed works, and reset the codec:
>
> sound/soc/codecs/rt5668.c:rt5668_remove() {
>         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);
> }
>
> During i2c driver unbind, rt5668_i2c_remove() runs before devres
> unregisters the component, so it resets the chip while the sound card is
> still bound and then rt5668_remove() runs and calls disable_irq() and
> rt5668_reset() a second time. In addition, rt5668_i2c_probe() continues if
> devm_request_threaded_irq() fails, leaving client->irq non-zero with
> rt5668->irq_acquired false, so unbind or shutdown disables an IRQ line the
> driver never acquired.
>
> The commit message also says that the component .remove callback only
> resets the chip and neither disables the IRQ nor cancels the delayed work,
> which is no longer true after 965bad6195d0 ("ASoC: rt5668: cancel delayed
> works on component removal").
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20261109172134.1850973-1-hj351016@gmail.com?part=1