Re: [PATCH v3 4/5] ASoC: fsl_asrc: expose individual DAIs per conversion path

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

Pre-existing issues:
- [High] Latent use-after-free / state corruption bug triggered by resource exhaustion: an hw_params error unconditionally releases a hardware pair actively used by another stream.
- [High] Changing Pair A's channel count via a DAI re-configuration instantly corrupts Pair B's active conversion.
--

--- Patch [4]: [PATCH v3 4/5] ASoC: fsl_asrc: expose individual DAIs per conversion path ---
commit 958c34d5b39a813baf5b15d76b95e94f278f4bf9
Author: Shengjiu Wang <[email protected]>

ASoC: fsl_asrc: expose individual DAIs per conversion path

The patch replaces the single fsl_asrc_dai instance with an array of three
DAI drivers. This allows up to three independent conversion paths to be
active at the same time.

> diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
> index 2755ee57c6a17..a5d6c4c1e9292 100644
> --- a/sound/soc/fsl/fsl_asrc.c
> +++ b/sound/soc/fsl/fsl_asrc.c
> @@ -792,27 +792,76 @@ static const struct snd_soc_dai_ops fsl_asrc_dai_ops = {
[ ... ]
> -static struct snd_soc_dai_driver fsl_asrc_dai = {
> -	.playback = {
[ ... ]
> +static struct snd_soc_dai_driver fsl_asrc_dai[] = {
> +	{
> +		.name = "asrc-0",
[ ... ]
> +	},
> +	{
> +		.name = "asrc-1",
[ ... ]
> +	},
> +	{
> +		.name = "asrc-2",

[Severity: High]
This isn't a bug introduced by this patch, but exposing 3 independent DAIs
allows multiple streams to run concurrently, making channel exhaustion and
-EBUSY in hw_params trivially reachable.

If a stream fails inside fsl_asrc_dai_hw_params() due to exhaustion, does
the cleanup path inadvertently disable another active stream?

Looking at fsl_asrc_dma_startup(), it requests a dummy pair and sets
pair->index (usually to 0), but does not clear it when released:

sound/soc/fsl/fsl_asrc_dma.c:fsl_asrc_dma_startup() {
    ...
    ret = asrc->request_pair(1, pair);
    ...
    if (pair)
        fsl_asrc_release_pair(pair);
}

If hw_params subsequently fails, the ALSA core calls fsl_asrc_dai_hw_free():

sound/soc/fsl/fsl_asrc.c:fsl_asrc_dai_hw_free() {
    ...
    if (pair)
        fsl_asrc_release_pair(pair);
}

Since pair->index is still 0 from the dummy request, wouldn't this blindly
disable Pair A, setting asrc->pair[0] = NULL and corrupting the global
channel_avail counter, even if Pair A is actively used by another DAI?


[Severity: High]
This is also a pre-existing issue, but allowing up to three independent
conversion paths to be active at the same time appears to violate a hardware
constraint regarding Pair B.

The driver documentation notes that Pair B depends on Pair A:

sound/soc/fsl/fsl_asrc.c:fsl_asrc_request_pair()
 * It assigns pair by the order of A->C->B because allocation of pair B,
 * within range [ANCA, ANCA+ANCB-1], depends on the channels of pair A

If Pair A is stopped and restarted with a different channel count while
Pair B is actively running, what prevents Pair B from being corrupted?

When fsl_asrc_config_pair() updates REG_ASRCNCR to change the ANCA value for
Pair A, won't Pair B's internal hardware channel offsets immediately shift
and disrupt its active conversion?

> +		.ops = &fsl_asrc_dai_ops,
>  	},
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4