Re: [PATCH 01/11] ASoC: fsl_asrc: Use guard() for spin locks

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Unvalidated cleanup in fsl_asrc_release_pair() unconditionally releases a pair without an ownership check, causing hardware state corruption and channel count tracking errors if fsl_asrc_request_pair() fails.
--

commit f44cdf6b851bca6e946cdc01dc94cd1b9c6b9570
Author: bui duc phuc <phucduc.bui@gmail.com>

ASoC: fsl_asrc: Use guard() for spin locks

Clean up the code using guard() for spin locks. Merely code refactoring, 
and no behavior change.

> diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
> index 5fda9b647c704..0b28bcfa47fed 100644
> --- a/sound/soc/fsl/fsl_asrc.c
> +++ b/sound/soc/fsl/fsl_asrc.c

[ ... ]

> @@ -265,19 +262,16 @@ static void fsl_asrc_release_pair(struct fsl_asrc_pair *pair)
>  {
>  	struct fsl_asrc *asrc = pair->asrc;
>  	enum asrc_pair_index index = pair->index;

[Severity: High]
This is a pre-existing issue, but does fsl_asrc_release_pair() unconditionally 
trust the pair's index without verifying ownership?

If fsl_asrc_request_pair() fails (for example, due to insufficient channels), 
it returns an error but leaves pair->index and pair->channels unmodified. 

The ALSA core handles this configuration failure by invoking the hw_free 
callback on the error path:

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

Since ownership isn't verified here, fsl_asrc_release_pair() blindly uses the 
stale pair->index, forcefully disables the hardware register for that pair, and 
adds the stale pair->channels back to asrc->channel_avail.

Could this abruptly destroy the active audio session of another process that 
owns the pair, and artificially inflate the available channel count?

> -	unsigned long lock_flags;
>  
>  	/* Make sure the pair is disabled */
>  	regmap_update_bits(asrc->regmap, REG_ASRCTR,
>  			   ASRCTR_ASRCEi_MASK(index), 0);
>  
> -	spin_lock_irqsave(&asrc->lock, lock_flags);
> +	guard(spinlock_irqsave)(&asrc->lock);
>  
>  	asrc->channel_avail += pair->channels;
>  	asrc->pair[index] = NULL;
>  	pair->error = 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612132639.78086-1-phucduc.bui@gmail.com?part=1