Re: [PATCH v3 1/3] ASoC: qcom: audioreach: compute active channel maps from channel_map

[sashiko-bot@kernel.org]

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

New issues:
- [High] The new `active_channels_mask` computation in `audioreach_codec_dma_set_media_format` introduces regressions by unconditionally iterating over all `AR_PCM_MAX_NUM_CHANNEL` elements.

Pre-existing issues:
- [High] Kernel stack memory is leaked to the DSP and evaluated in control logic because `cfg.channel_map` is not zero-initialized.
--

--- Patch [1]: [PATCH v3 1/3] ASoC: qcom: audioreach: compute active channel maps from channel_map ---
commit 893598480a0705f69c55eefe5bf4e2b969edc6d8
Author: Neil Armstrong <neil.armstrong@linaro.org>
Subject: ASoC: qcom: audioreach: compute active channel maps from channel_map

This patch updates the Audioreach DSP code to compute the active channel
mask and logical channel mappings using the physical channel mapping array
(channel_map) rather than a simple bitmask derived from the channel count.
This enables WSA2 speaker usage on specific hardware by correctly setting
only the upper bits of the channel mask.

> diff --git a/sound/soc/qcom/qdsp6/audioreach.c b/sound/soc/qcom/qdsp6/audioreach.c
> index e6e9eb2e85aa1..0cc840aca69dd 100644
> --- a/sound/soc/qcom/qdsp6/audioreach.c
> +++ b/sound/soc/qcom/qdsp6/audioreach.c
> @@ -741,7 +742,12 @@ static int audioreach_codec_dma_set_media_format(struct q6apm_graph *graph,
>  
>  	intf_cfg->cfg.lpaif_type = module->hw_interface_type;
>  	intf_cfg->cfg.intf_index = module->hw_interface_idx;
> -	intf_cfg->cfg.active_channels_mask = (1 << cfg->num_channels) - 1;
> +	intf_cfg->cfg.active_channels_mask = 0;
> +	/* Convert the physical channel mapping into a bit field */
> +	for (i = 0; i < AR_PCM_MAX_NUM_CHANNEL; i++)
> +		if (cfg->channel_map[i])
> +			intf_cfg->cfg.active_channels_mask |= BIT(i);
> +

[Severity: High]
If a stream is configured with 4 channels and later reconfigured to 2
channels, audioreach_set_default_channel_mapping() in q6dma_hw_params() only
overwrites the first 2 entries, leaving the remaining entries intact.

Since this loop iterates unconditionally up to AR_PCM_MAX_NUM_CHANNEL, could
it pick up those stale entries and activate unintended channels in the DSP?

Additionally, for channel counts greater than 4 (like 6 or 8),
audioreach_set_default_channel_mapping() can leave the map as all zeros. Could
this result in an active_channels_mask of 0 and break the DMA configuration?

[Severity: High]
This is a pre-existing issue, but struct audioreach_module_config cfg is
allocated on the stack in q6apm_dai_prepare() without being initialized:

q6apm_dai_prepare() {
    ...
    struct audioreach_module_config cfg;
    ...
}

When an unsupported channel count is used,
audioreach_set_default_channel_mapping() leaves the array untouched,
containing stack garbage.

While the previous code copied this memory blindly, the new loop here branches
on this uninitialized stack memory to construct the channel mask. Could this
result in unpredictable DSP configurations and leak stack contents in the IPC
payload?

>  	p += ic_sz;
>  
>  	pm_cfg = p;

[ ... ]

> @@ -1113,7 +1124,12 @@ static int audioreach_pcm_set_media_format(struct q6apm_graph *graph,
>  	media_cfg->num_channels = mcfg->num_channels;
>  	media_cfg->q_factor = mcfg->bit_width - 1;
>  	media_cfg->bits_per_sample = mcfg->bit_width;
> -	memcpy(media_cfg->channel_mapping, mcfg->channel_map, mcfg->num_channels);
> +	/* Convert the physical mapping to a logical mapping of the channels */
> +	for (i = 0, j = 0; i < AR_PCM_MAX_NUM_CHANNEL && j < mcfg->num_channels; i++) {
> +		if (!mcfg->channel_map[i])
> +			continue;
> +		media_cfg->channel_mapping[j++] = mcfg->channel_map[i];
> +	}

[Severity: High]
This is a pre-existing issue, but similar to the above concern, could this
loop branch on uninitialized stack memory if mcfg->channel_map was left
uninitialized in q6apm_dai_prepare()?

This would cause the code to evaluate stack garbage in its control flow and
copy it into the DSP IPC payload.

>  
>  	return q6apm_send_cmd_sync(graph->apm, pkt, 0);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728-topic-sm8650-ayaneo-pocket-s2-wsa2-fix-v3-0-b29f44720178@linaro.org?part=1