Re: [PATCH v4 5/8] ASoC: cs35l36: Implement set_tdm_slot to program RX and TX slots

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ASoC: cs35l36: DAPM 'Input Mux' conflicts with set_tdm_slot over ASP_RX1_SLOT
--

commit b9a33ee349c8716b47c6a0d1a24434c14d2f72d5
Author: David Heidelberg <david@ixit.cz>

ASoC: cs35l36: Implement set_tdm_slot to program RX and TX slots

Program the ASP RX and TX slot registers from the TDM masks passed by
the machine driver. Each set bit in a mask names a slot; codec channels
are assigned to those slots in order, ASPRX1 taking the first RX slot
and ASPTX1..TX8 the first eight TX slots.

> diff --git a/sound/soc/codecs/cs35l36.c b/sound/soc/codecs/cs35l36.c
> index 170588be4dbe3..5fc8ec292e3c4 100644
> --- a/sound/soc/codecs/cs35l36.c
> +++ b/sound/soc/codecs/cs35l36.c
[ ... ]
> @@ -947,6 +947,49 @@ static const struct cs35l36_pll_config *cs35l36_get_clk_config(
>  	return NULL;
>  }
>  
> +static void cs35l36_mask_to_slots(struct cs35l36_private *cs35l36,
> +				  unsigned long mask, unsigned int base_reg,
> +				  unsigned int nchan)
> +{
> +	unsigned int chan = 0, shift;
> +	int slot;
> +
> +	/* Two 6-bit slot fields per register, at bits 0 and 16 */
> +	for_each_set_bit(slot, &mask, BITS_PER_TYPE(mask)) {
> +		if (chan == nchan) {
> +			dev_warn(cs35l36->dev,
> +				 "Too many slots in TDM mask: %lx\n", mask);
> +			return;
> +		}
> +
> +		shift = (chan % 2) * CS35L36_ASP_TX2_SLOT_SHIFT;
> +		regmap_update_bits(cs35l36->regmap, base_reg + (chan / 2) * 4,
> +				   CS35L36_ASP_RX1_SLOT_MASK << shift,
> +				   slot << shift);
> +		chan++;
> +	}
> +}
> +
> +static int cs35l36_set_tdm_slot(struct snd_soc_dai *dai,
> +				unsigned int tx_mask, unsigned int rx_mask,
> +				int slots, int slot_width)
> +{
> +	struct cs35l36_private *cs35l36 =
> +			snd_soc_component_get_drvdata(dai->component);
> +
> +	/* Note: rx/tx is from point of view of the CPU end */
> +	if (!slots || !rx_mask)
> +		rx_mask = BIT(0);		/* ASPRX1 in slot 0 */
> +
> +	if (!slots || !tx_mask)
> +		tx_mask = GENMASK(7, 0);	/* ASPTX1..8 in slots 0..7 */
> +
> +	cs35l36_mask_to_slots(cs35l36, rx_mask, CS35L36_ASP_RX1_SLOT, 1);

[Severity: High]
Does overwriting CS35L36_ASP_RX1_SLOT here conflict with the DAPM "Input Mux"
control?

The CS35L36_ASP_RX1_SLOT register is already exposed to userspace as a DAPM
enum (chansel_enum).

If a machine driver sets the TDM slot using snd_soc_dai_set_tdm_slot(), it
overwrites this register via regmap_update_bits() in cs35l36_mask_to_slots().

However, if userspace later modifies the mixer control, DAPM will write back
to the register. Because the enum only covers 1 bit (options 'RX1' and 'RX2'),
it will overwrite bit 0, corrupting the TDM slot assigned by the machine
driver.

Can this cause the ALSA DAPM state to fall out of sync with the hardware and
allow unprivileged userspace to unknowingly break audio routing?

> +	cs35l36_mask_to_slots(cs35l36, tx_mask, CS35L36_ASP_TX1_TX2_SLOT, 8);
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924-pixel3-audio-v4-0-85b5d133aa5e@ixit.cz?part=5