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

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

[Hyunjung Ko]

rt5659_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 dereferences the driver data via container_of().

struct rt5659_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
rt5659_priv while jack_detect_work can still be queued, so the pending
work then runs against freed memory (use-after-free).

Disable the IRQ and cancel the delayed work 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
RT5658/RT5659 hardware).

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

diff --git a/sound/soc/codecs/rt5659.c b/sound/soc/codecs/rt5659.c
index 0000000..0000000 100644
--- a/sound/soc/codecs/rt5659.c
+++ b/sound/soc/codecs/rt5659.c
@@ -4319,9 +4319,18 @@ static void rt5659_i2c_shutdown(struct i2c_client *client)
 {
 	struct rt5659_priv *rt5659 = i2c_get_clientdata(client);

+	if (client->irq)
+		disable_irq(client->irq);
+	cancel_delayed_work_sync(&rt5659->jack_detect_work);
+
 	regmap_write(rt5659->regmap, RT5659_RESET, 0);
 }

+static void rt5659_i2c_remove(struct i2c_client *client)
+{
+	rt5659_i2c_shutdown(client);
+}
+
 #ifdef CONFIG_OF
 static const struct of_device_id rt5659_of_match[] = {
 	{ .compatible = "realtek,rt5658", },
@@ -4347,6 +4356,7 @@ static struct i2c_driver rt5659_i2c_driver = {
 		.acpi_match_table = ACPI_PTR(rt5659_acpi_match),
 	},
 	.probe = rt5659_i2c_probe,
+	.remove = rt5659_i2c_remove,
 	.shutdown = rt5659_i2c_shutdown,
 	.id_table = rt5659_i2c_id,
 };
--
2.43.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] ASoC: rt5659: unguarded cancel_delayed_work_sync() in
  rt5659_i2c_shutdown()
- [Medium] ASoC: rt5659: unguarded disable_irq() in
  rt5659_i2c_shutdown()
--

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

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

This commit disables the client IRQ and cancels jack_detect_work in
rt5659_i2c_shutdown(), and wires rt5659_i2c_shutdown() into a new i2c
.remove callback.

> diff --git a/sound/soc/codecs/rt5659.c b/sound/soc/codecs/rt5659.c
> index 68f7d5673e190..721c742c0efda 100644
> --- a/sound/soc/codecs/rt5659.c
> +++ b/sound/soc/codecs/rt5659.c
> @@ -4337,9 +4337,18 @@ static void rt5659_i2c_shutdown(struct i2c_client *client)
>  {
>  	struct rt5659_priv *rt5659 = i2c_get_clientdata(client);
>  
> +	if (client->irq)
> +		disable_irq(client->irq);

[Severity: Medium]
Can this cause an unbalanced double disable_irq() on client->irq, or
disable an IRQ that this driver never acquired?

The component remove callback rt5659_remove(), added in ca3efbaa8aeb
("ASoC: rt5659: cancel jack_detect_work on component removal"), already
disables the IRQ:

sound/soc/codecs/rt5659.c:rt5659_remove() {
	...
	if (rt5659->irq_acquired) {
		disable_irq(i2c->irq);
		rt5659->irq_disabled = true;
	}
	...
}

When unbinding an i2c device that is bound to a sound card,
rt5659_i2c_remove() -> rt5659_i2c_shutdown() calls disable_irq(), and
devres teardown of devm_snd_soc_register_component() then invokes
rt5659_remove(), calling disable_irq() a second time.  Conversely, if the
card was already unbound before i2c unbind or system shutdown,
rt5659_remove() has already set irq_disabled = true and
rt5659_i2c_shutdown() disables client->irq again.

In addition, devm_request_threaded_irq() failure in rt5659_i2c_probe() is
non-fatal, leaving client->irq non-zero while rt5659->irq_acquired is
false, so checking client->irq alone disables a line the driver does not
own.

> +	cancel_delayed_work_sync(&rt5659->jack_detect_work);

[Severity: Medium]
Can this call cancel_delayed_work_sync() on an uninitialized
rt5659->jack_detect_work?

In rt5659_i2c_probe(), INIT_DELAYED_WORK(&rt5659->jack_detect_work, ...)
and rt5659->jack_detect_work_inited = true only run when pdata.jd_src is
RT5659_JD3 or RT5659_JD_HDA_HEADER.  When pdata.jd_src is RT5659_JD_NULL
(for example when the optional realtek,jd-src property is omitted),
jack_detect_work is never initialized and unbinding or shutting down the
device triggers WARN_ON(!work->func) in __flush_work().

Guarding this call with if (rt5659->jack_detect_work_inited), as
rt5659_remove() does, avoids canceling uninitialized work.

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

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

[khj]

Thanks, and apologies - this is already fixed by ca3efbaa8aeb ("ASoC:
rt5659: cancel jack_detect_work on component removal"), which I missed
as I was working against an older tree. Please disregard.

Thanks,
Hyunjung

2026년 11월 10일 (화) 오전 2:43, <sashiko-bot@kernel.org>님이 작성:
>
> Thank you for your contribution! Sashiko AI review found 2 potential
> issues to consider:
> - [Medium] ASoC: rt5659: unguarded cancel_delayed_work_sync() in
>   rt5659_i2c_shutdown()
> - [Medium] ASoC: rt5659: unguarded disable_irq() in
>   rt5659_i2c_shutdown()
> --
>
> commit cfae689e1e0a124463e941d5a4ef73f9a8712aac
> Author: Hyunjung Ko <hj351016@gmail.com>
>
> ASoC: rt5659: cancel jack detect work on removal to avoid use-after-free
>
> This commit disables the client IRQ and cancels jack_detect_work in
> rt5659_i2c_shutdown(), and wires rt5659_i2c_shutdown() into a new i2c
> .remove callback.
>
> > diff --git a/sound/soc/codecs/rt5659.c b/sound/soc/codecs/rt5659.c
> > index 68f7d5673e190..721c742c0efda 100644
> > --- a/sound/soc/codecs/rt5659.c
> > +++ b/sound/soc/codecs/rt5659.c
> > @@ -4337,9 +4337,18 @@ static void rt5659_i2c_shutdown(struct i2c_client *client)
> >  {
> >       struct rt5659_priv *rt5659 = i2c_get_clientdata(client);
> >
> > +     if (client->irq)
> > +             disable_irq(client->irq);
>
> [Severity: Medium]
> Can this cause an unbalanced double disable_irq() on client->irq, or
> disable an IRQ that this driver never acquired?
>
> The component remove callback rt5659_remove(), added in ca3efbaa8aeb
> ("ASoC: rt5659: cancel jack_detect_work on component removal"), already
> disables the IRQ:
>
> sound/soc/codecs/rt5659.c:rt5659_remove() {
>         ...
>         if (rt5659->irq_acquired) {
>                 disable_irq(i2c->irq);
>                 rt5659->irq_disabled = true;
>         }
>         ...
> }
>
> When unbinding an i2c device that is bound to a sound card,
> rt5659_i2c_remove() -> rt5659_i2c_shutdown() calls disable_irq(), and
> devres teardown of devm_snd_soc_register_component() then invokes
> rt5659_remove(), calling disable_irq() a second time.  Conversely, if the
> card was already unbound before i2c unbind or system shutdown,
> rt5659_remove() has already set irq_disabled = true and
> rt5659_i2c_shutdown() disables client->irq again.
>
> In addition, devm_request_threaded_irq() failure in rt5659_i2c_probe() is
> non-fatal, leaving client->irq non-zero while rt5659->irq_acquired is
> false, so checking client->irq alone disables a line the driver does not
> own.
>
> > +     cancel_delayed_work_sync(&rt5659->jack_detect_work);
>
> [Severity: Medium]
> Can this call cancel_delayed_work_sync() on an uninitialized
> rt5659->jack_detect_work?
>
> In rt5659_i2c_probe(), INIT_DELAYED_WORK(&rt5659->jack_detect_work, ...)
> and rt5659->jack_detect_work_inited = true only run when pdata.jd_src is
> RT5659_JD3 or RT5659_JD_HDA_HEADER.  When pdata.jd_src is RT5659_JD_NULL
> (for example when the optional realtek,jd-src property is omitted),
> jack_detect_work is never initialized and unbinding or shutting down the
> device triggers WARN_ON(!work->func) in __flush_work().
>
> Guarding this call with if (rt5659->jack_detect_work_inited), as
> rt5659_remove() does, avoids canceling uninitialized work.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20261109172155.1851386-1-hj351016@gmail.com?part=1