Re: [PATCH v10] ASoC: wm8994: drain delayed works on remove and probe error

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ASoC: wm8994: mic_work can be re-queued by wm1811_jackdet_bootstrap()
--

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

ASoC: wm8994: drain delayed works on remove and probe error

This commit cancels pending delayed works in wm8994_component_probe() error
handling and wm8994_component_remove() to prevent works from executing after
resources are freed.

> diff --git a/sound/soc/codecs/wm8994.c b/sound/soc/codecs/wm8994.c
> index c65a7769a9d25..c95a7a4dffb19 100644
> --- a/sound/soc/codecs/wm8994.c
> +++ b/sound/soc/codecs/wm8994.c
> @@ -4574,6 +4574,11 @@ 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);
> +	cancel_delayed_work_sync(&wm8994->mic_complete_work);
> +	cancel_delayed_work_sync(&wm8994->open_circuit_work);
>  
>  	return ret;
>  }
> @@ -4615,6 +4620,11 @@ 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);
> +	cancel_delayed_work_sync(&wm8994->mic_complete_work);
> +	cancel_delayed_work_sync(&wm8994->open_circuit_work);

[Severity: High]
Can wm8994->mic_work be re-queued here if wm8994->jackdet_bootstrap runs
after cancel_delayed_work_sync(&wm8994->mic_work) completes?

In sound/soc/codecs/wm8994.c, wm1811_jackdet_bootstrap() calls
wm1811_jackdet_irq(), which queues wm8994->mic_work when a jack is present:

static void wm1811_jackdet_bootstrap(struct work_struct *work)
{
	struct wm8994_priv *wm8994 = container_of(work,
						struct wm8994_priv,
						jackdet_bootstrap.work);
	wm1811_jackdet_irq(0, wm8994);
}

and inside wm1811_jackdet_irq():

	if (present) {
		...
		delay = control->pdata.micdet_delay;
		queue_delayed_work(system_power_efficient_wq,
				   &wm8994->mic_work,
				   msecs_to_jiffies(delay));
	}

On WM1811 with jack detection enabled, powering up CLK_SYS triggers
clk_sys_event(), which schedules wm8994->jackdet_bootstrap.

Because wm1811_mic_work() can schedule wm8994->jackdet_bootstrap (via
snd_soc_dapm_force_enable_pin() -> clk_sys_event()) and
wm1811_jackdet_bootstrap() can in turn schedule wm8994->mic_work, the two
works have a cyclic dependency. If wm1811_jackdet_bootstrap() is already
executing or its timer fires between cancel_delayed_work_sync(&wm8994->mic_work)
and cancel_delayed_work_sync(&wm8994->jackdet_bootstrap), it will re-arm
wm8994->mic_work, leaving wm1811_mic_work() pending after
wm8994_component_remove() (or the wm8994_component_probe() error path) returns
and frees the component and wm8994_priv memory.

Would it make sense to use disable_delayed_work_sync() on these works during
teardown, or cancel wm8994->mic_work a second time after
cancel_delayed_work_sync(&wm8994->jackdet_bootstrap) finishes?

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