Re: [PATCH v2 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] Lockless access to the `asrc->pair` array in `fsl_asrc_isr` can lead to Use-After-Free and reading uninitialized memory.
--

commit 4b6342441bc61476e92408bc85fb134b9ad8e9f7
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;
> -	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;

[Severity: High]
This isn't a bug introduced by this patch, but can the lockless access to the
asrc->pair array in fsl_asrc_isr() lead to a use-after-free or reading
uninitialized memory?

In fsl_asrc_release_pair(), the pair is disabled and removed via
asrc->pair[index] = NULL under the lock. However, fsl_asrc_isr() iterates
over the asrc->pair array and dereferences its elements locklessly:

sound/soc/fsl/fsl_asrc.c:fsl_asrc_isr() {
    ...
		if (!asrc->pair[index])
			continue;

		if (status & ASRSTR_ATQOL) {
			asrc->pair[index]->error |= ASRC_TASK_Q_OVERLOAD;
    ...
}

Because the ISR does not take asrc->lock, an ISR executing on another CPU can
read the pointer just before it is nullified. ALSA subsequently calls
fsl_asrc_dma_shutdown(), which frees the memory:

sound/soc/fsl/fsl_asrc_dma.c:fsl_asrc_dma_shutdown() {
    ...
	kfree(pair);
    ...
}

Since there is no synchronize_irq() or lock-based synchronization, the
concurrent ISR can proceed to write to pair->error after the memory is freed,
causing a use-after-free in hardirq context.

Additionally, fsl_asrc_request_pair() publishes the newly allocated pair via a
plain store:

		asrc->pair[index] = pair;

Without a memory barrier (like smp_store_release()), the lockless ISR may see
the pointer before the initialization of the pair structure is visible to its
CPU.

>  	pair->error = 0;
> -
> -	spin_unlock_irqrestore(&asrc->lock, lock_flags);
>  }
>  
>  /**

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