Re: [PATCH v2 6/6] ASoC: qcom: lpass-va-macro: Add SoundWire microphone mux support

[sashiko-bot@kernel.org]

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

New issues:
- [High] The HPF cutoff frequency change request bit (`CDC_VA_TX_HPF_CUTOFF_FREQ_CHANGE_REQ`) is permanently left asserted for SMIC sources.
- [High] The VA SMIC and VA DMIC mux put handlers conflict over the `CDC_VA_TX_PATH_ADC_DMIC_SEL_MASK` hardware bit, leading to silent audio routing failures.

Pre-existing issues:
- [High] The kcontrol put callback `va_macro_put_dec_enum` modifies hardware register state outside the DAPM mutex and prior to validation, introducing a TOCTOU race condition and state desynchronization.
--

commit 65c41e47c0a34211271b24ec96e2c5b75c4b06b0
Author: Mohammad Rafi Shaik <mohammad.rafi.shaik@oss.qualcomm.com>

ASoC: qcom: lpass-va-macro: Add SoundWire microphone mux support
    
The VA decimators can currently capture audio only from DMIC inputs.
Add support for routing audio from SoundWire microphone (SMIC) inputs
as well.

> diff --git a/sound/soc/codecs/lpass-va-macro.c b/sound/soc/codecs/lpass-va-macro.c
> index 8353921f718e3..46f35a984321c 100644
> --- a/sound/soc/codecs/lpass-va-macro.c
> +++ b/sound/soc/codecs/lpass-va-macro.c

[ ... ]

> @@ -756,10 +757,16 @@ static int va_macro_put_dec_enum(struct snd_kcontrol *kcontrol,
>  		return -EINVAL;
>  	}
>  
> -	if (val != 0)
> -		snd_soc_component_update_bits(component, mic_sel_reg,
> +	if (val != 0) {
> +		if (strstr(widget->name, "SMIC"))
> +			snd_soc_component_update_bits(component, mic_sel_reg,
> +					      CDC_VA_TX_PATH_ADC_DMIC_SEL_MASK,
> +					      CDC_VA_TX_PATH_ADC_DMIC_SEL_ADC);
> +		else
> +			snd_soc_component_update_bits(component, mic_sel_reg,
>  					      CDC_VA_TX_PATH_ADC_DMIC_SEL_MASK,
>  					      CDC_VA_TX_PATH_ADC_DMIC_SEL_DMIC);
> +	}
>  
>  	return snd_soc_dapm_put_enum_double(kcontrol, ucontrol);
>  }

[Severity: High]
This is a pre-existing issue, but does modifying the hardware register here in
va_macro_put_dec_enum() happen outside the DAPM mutex? 

Since snd_soc_dapm_put_enum_double() acquires the DAPM mutex and performs
bounds checking later, could an out-of-bounds enum index malicious update the
hardware but get rejected by the DAPM core, causing the hardware and software
states to permanently desynchronize?

[Severity: High]
Does this code also unconditionally overwrite the shared hardware mux selection
based on whichever control was touched last?

If userspace toggles the target DEC source (VA DECx MUX) between DMIC and SMIC
without also re-triggering the corresponding input mux, could this cause the
hardware register to remain incorrectly stuck on the previous selection? The
DAPM graph would properly power the new SMIC path, but the hardware register
would be incorrectly left on DMIC, potentially resulting in silent audio
routing failures.

[ ... ]

> @@ -994,21 +1011,20 @@ static int va_macro_enable_dec(struct snd_soc_dapm_widget *w,
>  						      CF_MIN_3DB_150HZ << 5);
>  
>  			snd_soc_component_update_bits(comp, hpf_gate_reg,
> -				      CDC_VA_TX_HPF_CUTOFF_FREQ_CHANGE_MASK,
> -				      CDC_VA_TX_HPF_CUTOFF_FREQ_CHANGE_REQ);
> +					      CDC_VA_TX_HPF_CUTOFF_FREQ_CHANGE_MASK,
> +					      CDC_VA_TX_HPF_CUTOFF_FREQ_CHANGE_REQ);
>  
>  			/*
>  			 * Minimum 1 clk cycle delay is required as per HW spec
>  			 */
>  			usleep_range(1000, 1010);
>  
> -			snd_soc_component_update_bits(comp,
> -				hpf_gate_reg,
> -				CDC_VA_TX_HPF_CUTOFF_FREQ_CHANGE_MASK,
> -				0x0);
> +			if (!is_amic_enabled(comp, decimator))
> +				snd_soc_component_update_bits(comp, hpf_gate_reg,
> +						      CDC_VA_TX_HPF_CUTOFF_FREQ_CHANGE_MASK,
> +						      0x0);
>  		}

[Severity: High]
If an SMIC path is active (!is_amic_enabled evaluates to false), does the
frequency change request bit CDC_VA_TX_HPF_CUTOFF_FREQ_CHANGE_REQ remain
permanently asserted?

The hardware state machine for the HPF typically requires a 0->1->0 transition.
If this bit is conditionally left at 1 when an SMIC source is active and the
cutoff frequency differs from CF_MIN_3DB_150HZ, could it become stuck and
prevent any future HPF cutoff frequency changes from completing?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910-add_shikra_support-v2-0-fdd2129c696c@oss.qualcomm.com?part=6