Re: [PATCH 2/5] ASoC: fsl_asrc: fix pair ownership on repeated or failed

[Shengjiu Wang]

On Tue, Nov 10, 2026 at 8:08 PM Stefan Ghetu <stefan.ghetu@xxxxxxx> wrote:
>
> hw_params() can be called again without hw_free() in between, and
> hw_free() is also called after a failed hw_params().
>
> fsl_asrc_request_pair() always takes a new pair, so a second hw_params()
> leaves the stream in two asrc->pair[] entries. Only one is released, and
> after close the other points to freed memory, which is dereferenced by
> fsl_asrc_isr() and fsl_asrc_m2m_suspend(). After a failed hw_params(),
> fsl_asrc_release_pair() releases a stale index that may belong to
> another running stream.
>
> Keep the owned pair and only update its channels when it is requested
> again, and ignore a pair the stream does not own on release.
>
> Fixes: 3117bb3109dc ("ASoC: fsl_asrc: Add ASRC ASoC CPU DAI and platform drivers")
> Signed-off-by: Stefan Ghetu <stefan.ghetu@xxxxxxx>

Reviewed-by: Shengjiu Wang <shengjiu.wang@xxxxxxxxx>

best regards
shengjiu Wang
> ---
>  sound/soc/fsl/fsl_asrc.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>
> diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
> index faf348dd582d..5d97789b5c36 100644
> --- a/sound/soc/fsl/fsl_asrc.c
> +++ b/sound/soc/fsl/fsl_asrc.c
> @@ -216,6 +216,8 @@ static void fsl_asrc_sel_proc(int inrate, int outrate,
>   * 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
>   * while pair A and pair C are comparatively independent.
> + *
> + * If @pair already owns a pair, it is kept and only its channels are updated.
>   */
>  static int fsl_asrc_request_pair(int channels, struct fsl_asrc_pair *pair)
>  {
> @@ -226,6 +228,19 @@ static int fsl_asrc_request_pair(int channels, struct fsl_asrc_pair *pair)
>
>         guard(spinlock_irqsave)(&asrc->lock);
>
> +       if (asrc->pair[pair->index] == pair) {
> +               if (asrc->channel_avail + pair->channels < channels) {
> +                       dev_err(dev, "can't afford required channels: %d\n", channels);
> +                       return -EINVAL;
> +               }
> +
> +               asrc->channel_avail += pair->channels;
> +               asrc->channel_avail -= channels;
> +               pair->channels = channels;
> +
> +               return 0;
> +       }
> +
>         for (i = ASRC_PAIR_A; i < ASRC_PAIR_MAX_NUM; i++) {
>                 if (asrc->pair[i] != NULL)
>                         continue;
> @@ -257,12 +272,19 @@ static int fsl_asrc_request_pair(int channels, struct fsl_asrc_pair *pair)
>   * @pair: pair to release
>   *
>   * It clears the resource from asrc and releases the occupied channels.
> + * Nothing is done if @pair does not own a pair.
>   */
>  static void fsl_asrc_release_pair(struct fsl_asrc_pair *pair)
>  {
>         struct fsl_asrc *asrc = pair->asrc;
>         enum asrc_pair_index index = pair->index;
>
> +       /* Only the owner clears its entry, so it cannot change after this check */
> +       scoped_guard(spinlock_irqsave, &asrc->lock) {
> +               if (asrc->pair[index] != pair)
> +                       return;
> +       }
> +
>         /* Make sure the pair is disabled */
>         regmap_update_bits(asrc->regmap, REG_ASRCTR,
>                            ASRCTR_ASRCEi_MASK(index), 0);
> --
> 2.53.0
>