[PATCH v4 0/2] ASoC: codecs: nau8360: Fix DSP routing and deadlock issues

[PATCH v4 0/2] ASoC: codecs: nau8360: Fix DSP routing and deadlock issues

[Neo Chang]

This patch series addresses two issues in the nau8360 codec driver
regarding the DAC mux control:

1. An invalid hardware routing state that occurs when the DSP firmware
   fails to load but the userspace attempts to switch to the DSP path.

2. A pre-existing AB-BA deadlock triggered during concurrent stream
   startups or ALSA mixer updates.

---
Changes in v4:
- Patch 1/2: Changed the return value to -EBUSY instead of 0 to properly
  reject the invalid request and prevent ALSA state desync.
- Patch 1/2: Used dev_warn_ratelimited() instead of dev_warn().
- Patch 2/2: Added a fix for the AB-BA deadlock in nau8360_dac_mux_put_enum().
  (Reported by Sashiko AI)

Changes in v3:
- Block the DSP path selection if the firmware fails to load to prevent
  invalid routing states.

Changes in v2:
- Use snd_soc_component_update_bits() to prevent register overwriting.
  (Reported by Sashiko AI)
- Fix missing bit shift for default PEQ band. (Reported by Sashiko AI)

Neo Chang (2):
  ASoC: codecs: nau8360: Block DSP path selection when firmware load
    fails
  ASoC: codecs: nau8360: Fix AB-BA deadlock in mux update

 sound/soc/codecs/nau8360.c | 19 +++++++++++++------
 sound/soc/codecs/nau8360.h |  6 ++++++
 2 files changed, 19 insertions(+), 6 deletions(-)

-- 
2.25.1

[PATCH v4 1/2] ASoC: codecs: nau8360: Block DSP path selection when firmware load fails

[Neo Chang]

Block the DSP path selection if the firmware fails to load to prevent
invalid routing states.

If the firmware is not ready, nau8360_dac_mux_put_enum() will print
a rate-limited warning and return -EBUSY. This rejects the invalid
userspace request and maintains the original hardware state without
causing audio wrong status.

Signed-off-by: Neo Chang <YLCHANG2@nuvoton.com>
---
 sound/soc/codecs/nau8360.c | 7 ++++++-
 sound/soc/codecs/nau8360.h | 6 ++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/sound/soc/codecs/nau8360.c b/sound/soc/codecs/nau8360.c
index 89b8ee80d6c8..277aa982c341 100644
--- a/sound/soc/codecs/nau8360.c
+++ b/sound/soc/codecs/nau8360.c
@@ -715,10 +715,15 @@ static int nau8360_dac_mux_put_enum(struct snd_kcontrol *kcontrol,
 	int ret = 0;
 
 	if (snd_soc_dapm_get_bias_level(dapm) > SND_SOC_BIAS_STANDBY) {
-		dev_warn(nau8360->dev, "changing path is not allowed during playback");
+		dev_warn_ratelimited(nau8360->dev, "changing path is not allowed during playback");
 		return ret;
 	}
 
+	if (item[0] == NAU8360_DAC_SRC_DSP && !nau8360->load_fw_done) {
+		dev_warn_ratelimited(nau8360->dev, "Cannot enable DSP: Firmware not ready or disabled\n");
+		return -EBUSY;
+	}
+
 	mutex_lock(&nau8360->lock);
 
 	ret = snd_soc_dapm_put_enum_double(kcontrol, ucontrol);
diff --git a/sound/soc/codecs/nau8360.h b/sound/soc/codecs/nau8360.h
index cc640ba8c838..71396747c0fa 100644
--- a/sound/soc/codecs/nau8360.h
+++ b/sound/soc/codecs/nau8360.h
@@ -870,6 +870,12 @@ enum {
 	NAU8360_TDM_TXN,
 };
 
+/* DAC Source Path*/
+enum {
+	NAU8360_DAC_SRC_HW1 = 0,
+	NAU8360_DAC_SRC_DSP,
+};
+
 /* PLL Source */
 enum {
 	NAU8360_PLL_MCLK,
-- 
2.25.1

Re: [PATCH v4 1/2] ASoC: codecs: nau8360: Block DSP path selection when firmware load fails

[sashiko-bot]

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

Pre-existing issues:
- [High] Time-of-check to time-of-use (TOCTOU) race when checking DAPM bias level, allowing path changes during active playback.
--

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

ASoC: codecs: nau8360: Block DSP path selection when firmware load fails
    
This commit blocks DSP path selection if the firmware fails to load,
preventing invalid routing states. It ensures invalid userspace requests
are rejected with -EBUSY to maintain the original hardware state.

> diff --git a/sound/soc/codecs/nau8360.c b/sound/soc/codecs/nau8360.c
> --- a/sound/soc/codecs/nau8360.c
> +++ b/sound/soc/codecs/nau8360.c
> @@ -715,10 +715,15 @@ static int nau8360_dac_mux_put_enum(struct snd_kcontrol *kcontrol,
>  	int ret = 0;
>  
>  	if (snd_soc_dapm_get_bias_level(dapm) > SND_SOC_BIAS_STANDBY) {
> -		dev_warn(nau8360->dev, "changing path is not allowed during playback");
> +		dev_warn_ratelimited(nau8360->dev, "changing path is not allowed during playback");
>  		return ret;
>  	}
>  
> +	if (item[0] == NAU8360_DAC_SRC_DSP && !nau8360->load_fw_done) {
> +		dev_warn_ratelimited(nau8360->dev, "Cannot enable DSP: Firmware not ready or disabled\n");
> +		return -EBUSY;
> +	}
> +
>  	mutex_lock(&nau8360->lock);
>  
>  	ret = snd_soc_dapm_put_enum_double(kcontrol, ucontrol);

[Severity: High]
This is a pre-existing issue, but does nau8360_dac_mux_put_enum() have a
time-of-check to time-of-use (TOCTOU) race when checking the DAPM bias level?

The follow-up fix in this series ("ASoC: codecs: nau8360: Fix AB-BA deadlock
in mux update") adds dapm_mutex locking to this function but fails to resolve
the race condition.

If a concurrent userspace mixer update and a PCM stream start occur, the mixer
update thread can check that the bias level is <= SND_SOC_BIAS_STANDBY.

Once the lock is dropped (or before it is held, in the current patch state),
the PCM stream can start, acquire the dapm_mutex, transition the bias level to
SND_SOC_BIAS_ON, and release the lock.

Then, snd_soc_dapm_put_enum_double() re-acquires the lock internally and
blindly applies the path change while playback is active.

Can this sequence cause the hardware routing path to be mutated while audio
is active, potentially leading to invalid hardware routing states or audio
glitches?

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

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

[Neo Chang]

nau8360_dac_mux_put_enum() acquires nau8360->lock before the DAPM mutex,
causing an AB-BA lock inversion. This deadlocks with concurrent stream
startups or mixer updates, which acquire locks in the reverse order.

Fix this by replacing nau8360->lock with the DAPM mutex to protect
pre-checks, releasing it before calling snd_soc_dapm_put_enum_double().

Reported-by: Sashiko AI <sashiko-bot@kernel.org>
Link: https://sashiko.dev/#/patchset/20260908030342.222655-1-YLCHANG2@nuvoton.com?part=1
Signed-off-by: Neo Chang <YLCHANG2@nuvoton.com>
---
 sound/soc/codecs/nau8360.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/sound/soc/codecs/nau8360.c b/sound/soc/codecs/nau8360.c
index 277aa982c341..7f83c737e8af 100644
--- a/sound/soc/codecs/nau8360.c
+++ b/sound/soc/codecs/nau8360.c
@@ -689,6 +689,8 @@ static void nau8360_dsp_switch(struct snd_soc_component *component, bool enable)
 	struct regmap *regmap = nau8360->regmap;
 	int value = NAU8360_PEQ_BAND_8;
 
+	mutex_lock(&nau8360->lock);
+
 	/* If DSP is enabled, unstall HW3 engine and DSP, loading DSP firmware,
 	 * and configure PEQ after dsp reset.
 	 */
@@ -702,6 +704,7 @@ static void nau8360_dsp_switch(struct snd_soc_component *component, bool enable)
 	regmap_update_bits(regmap, NAU8360_R9D_PEQ_CTL, NAU8360_PEQ_BAND_MASK,
 		value << NAU8360_PEQ_BAND_SFT);
 
+	mutex_unlock(&nau8360->lock);
 }
 
 static int nau8360_dac_mux_put_enum(struct snd_kcontrol *kcontrol,
@@ -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;
 	}
 
 	if (item[0] == NAU8360_DAC_SRC_DSP && !nau8360->load_fw_done) {
 		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);
 	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;
 }
 
-- 
2.25.1

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

[sashiko-bot]

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