Re: [PATCH 2/4] ASoC: rt5659: cancel jack_detect_work on component remove

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] ASoC: rt5659: uninitialized delayed_work cancel in rt5659_remove()
- [High] ASoC: rt5659: uninitialized jack_detect_work used when jd_src is unset
--

commit ca81fa64a51ff1ffcf9ce836fb012f5147653078
Author: Tristan Madani <tristan@talencesecurity.com>

ASoC: rt5659: cancel jack_detect_work on component remove

This commit disables the codec IRQ and cancels jack_detect_work
synchronously in rt5659_remove() before resetting the codec, and
re-enables the IRQ in rt5659_probe() on component rebind.

> diff --git a/sound/soc/codecs/rt5659.c b/sound/soc/codecs/rt5659.c
> index af8051fa69f89..d9daede7310ea 100644
> --- a/sound/soc/codecs/rt5659.c
> +++ b/sound/soc/codecs/rt5659.c
[ ... ]
> @@ -3684,7 +3690,14 @@ 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;
> +	}
>  
> +	cancel_delayed_work_sync(&rt5659->jack_detect_work);

[Severity: High]
Is rt5659->jack_detect_work guaranteed to be initialized before
rt5659_remove() calls cancel_delayed_work_sync()?

In sound/soc/codecs/rt5659.c, rt5659_i2c_probe() allocates rt5659 with
devm_kzalloc() and only calls INIT_DELAYED_WORK() when
rt5659->pdata.jd_src is RT5659_JD3 or RT5659_JD_HDA_HEADER:

sound/soc/codecs/rt5659.c:rt5659_i2c_probe() {
	...
	switch (rt5659->pdata.jd_src) {
	case RT5659_JD3:
		...
		INIT_DELAYED_WORK(&rt5659->jack_detect_work,
			rt5659_jack_detect_work);
		break;
	case RT5659_JD_HDA_HEADER:
		...
		INIT_DELAYED_WORK(&rt5659->jack_detect_work,
			rt5659_jack_detect_intel_hd_header);
		rt5659_intel_hd_header_probe_setup(rt5659);
		break;
	default:
		break;
	}
	...
}

When the optional realtek,jd-src property is omitted or set to 0
(RT5659_JD_NULL), the default branch is taken and
rt5659->jack_detect_work remains zero-initialized with work.func == NULL
and an uninitialized timer.

When snd_soc_component_remove() -> rt5659_remove() then runs on card
unbind or driver removal, cancel_delayed_work_sync() ->
__cancel_work_sync() calls timer_delete() on the uninitialized timer and
__flush_work() hits WARN_ON(!work->func).

Additionally, because rt5659_i2c_probe() still registers rt5659_irq when
i2c->irq is present and rt5659_set_jack_detect() invokes rt5659_irq(), an
interrupt or jack detection setup with pdata.jd_src == RT5659_JD_NULL will
call queue_delayed_work() on the uninitialized rt5659->jack_detect_work.

Should cancel_delayed_work_sync() (as well as IRQ registration or
work queueing) be guarded by checking whether rt5659->pdata.jd_src
configured jack_detect_work?

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