Re: [PATCH v4 2/2] ASoC: codecs: nau8360: Fix AB-BA deadlock in mux update

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Use-after-free (UAF) due to unbalanced lifecycle operations between devres component unregistration and background work cancellation.
- [High] Time-of-check to time-of-use (TOCTOU) race when verifying the DAPM bias level constraint before updating DAPM paths.
- [Medium] Returning 0 instead of an error code when rejecting a mux change during playback.
- [Medium] Bogus locking and data race when checking `load_fw_done` flag.
--

commit e1e283a6d218bffef7bd342646768fdbef3b2320
Author: Neo Chang <YLCHANG2@nuvoton.com>

ASoC: codecs: nau8360: Fix AB-BA deadlock in mux update

This commit fixes an AB-BA lock inversion by replacing nau8360->lock with
the DAPM mutex to protect pre-checks. It drops the lock before calling
snd_soc_dapm_put_enum_double() to avoid deadlocks with concurrent stream
startups or mixer updates.

Link: https://sashiko.dev/#/patchset/20260908030342.222655-1-YLCHANG2@nuvoton.com?part=1

> diff --git a/sound/soc/codecs/nau8360.c b/sound/soc/codecs/nau8360.c
> index 277aa982c341e..7f83c737e8af8 100644
> --- a/sound/soc/codecs/nau8360.c
> +++ b/sound/soc/codecs/nau8360.c
[ ... ]
> @@ -714,27 +717,26 @@ static int nau8360_dac_mux_put_enum(struct snd_kcontrol *kcontrol,
>  	unsigned int *item = ucontrol->value.enumerated.item;
>  	int ret = 0;
>  
> +	snd_soc_dapm_mutex_lock(dapm);
>  	if (snd_soc_dapm_get_bias_level(dapm) > SND_SOC_BIAS_STANDBY) {
>  		dev_warn_ratelimited(nau8360->dev, "changing path is not allowed during playback");
> +		snd_soc_dapm_mutex_unlock(dapm);
>  		return ret;

[Severity: Medium]
This is a pre-existing issue, but does this return success to the ALSA
framework when the change was actually rejected?

Since ret is initialized to 0, userspace might incorrectly believe the request
was accepted or benignly ignored when it attempts to change the control during
playback.

>  	}
>  
>  	if (item[0] == NAU8360_DAC_SRC_DSP && !nau8360->load_fw_done) {

[Severity: Medium]
This isn't a bug introduced by this patch, but does the dapm_mutex actually
protect this load_fw_done flag?

Since nau8360_load_fw_work() asynchronously sets nau8360->load_fw_done = true
without acquiring dapm_mutex, it appears the read here remains a plain C data
race and the lock provides no actual synchronization.

>  		dev_warn_ratelimited(nau8360->dev, "Cannot enable DSP: Firmware not ready or disabled\n");
> +		snd_soc_dapm_mutex_unlock(dapm);
>  		return -EBUSY;
>  	}
> -
> -	mutex_lock(&nau8360->lock);
> +	snd_soc_dapm_mutex_unlock(dapm);
>  
>  	ret = snd_soc_dapm_put_enum_double(kcontrol, ucontrol);

[Severity: High]
This is a pre-existing issue, but could dropping the lock here before calling
snd_soc_dapm_put_enum_double() create a race condition?

Since snd_soc_dapm_put_enum_double() re-acquires the same dapm mutex
internally, there is a window where the lock is released. A concurrent PCM
stream startup could acquire the DAPM lock in this window, increment the bias
level, and start playback.

Once the stream starts, snd_soc_dapm_put_enum_double() acquires the lock and
blindly applies the mux routing change during active playback, circumventing
the driver's intended safety constraint checked earlier.

>  	if (ret <= 0)
> -		goto unlock;
> +		return ret;
>  
>  	nau8360_dsp_switch(component, snd_soc_enum_item_to_val(e, item[0]));
>  
> -unlock:
> -	mutex_unlock(&nau8360->lock);
> -
>  	return ret;
>  }

[Severity: Critical]
This is a pre-existing issue, but is there a use-after-free risk between
devres component unregistration and background work cancellation?

In nau8360_i2c_remove(), the background work is cancelled:

    cancel_work_sync(&nau8360->load_fw_work);

However, the ASoC component is registered using devm_snd_soc_register_component().
Due to devres cleanup ordering, the component remains registered after
nau8360_i2c_remove() finishes, until the devres release unregisters it.

During this window, an asynchronous sound card bind (e.g., from deferred probe)
could successfully acquire the module reference and call nau8360_codec_probe(),
which reschedules the work:

    schedule_work(&nau8360->load_fw_work);

The devres release would then continue, unregistering the component and freeing
the nau8360 struct without canceling the newly scheduled work.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909025947.287120-1-YLCHANG2@nuvoton.com?part=2