Re: [PATCH v5 03/17] dmaengine: sh: rz-dmac: Use list_first_entry_or_null()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Pre-existing severe race condition exposing uninitialized descriptors to hardware execution queues, causing transfers to permanently hang.
--

commit 47f89a64a0228eb1a1e5b15d4e627cac741a15c7
Author: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

dmaengine: sh: rz-dmac: Use list_first_entry_or_null()

This commit simplifies descriptor allocation by replacing an open-coded
list_empty() check and list_first_entry() call with a single call to
list_first_entry_or_null().

> diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
> index 6d80cb668957f..1717b407ab9e9 100644
> --- a/drivers/dma/sh/rz-dmac.c
> +++ b/drivers/dma/sh/rz-dmac.c
> @@ -503,11 +503,10 @@ rz_dmac_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
>  		__func__, channel->index, &src, &dest, len);
>  
>  	scoped_guard(spinlock_irqsave, &channel->vc.lock) {
> -		if (list_empty(&channel->ld_free))
> +		desc = list_first_entry_or_null(&channel->ld_free, struct rz_dmac_desc, node);
> +		if (!desc)
>  			return NULL;
>  
> -		desc = list_first_entry(&channel->ld_free, struct rz_dmac_desc, node);
> -
>  		desc->type = RZ_DMAC_DESC_MEMCPY;
>  		desc->src = src;
>  		desc->dest = dest;

This isn't a bug introduced by this commit, but there appears to be a
pre-existing race condition in the functionally modified block.

At the end of the scoped_guard block for channel->vc.lock, the descriptor is
moved from ld_free to ld_queue, and then the lock is dropped. Outside the
lock, vchan_tx_prep() is called.

If rz_dmac_irq_handler_thread() runs in the exact window between dropping the
lock and vchan_tx_prep(), could it prematurely pull the descriptor from
ld_queue?

It would then call rz_dmac_xfer_desc(), which checks vchan_next_desc().
Because the descriptor hasn't been fully initialized or submitted to virt-dma
yet, vchan_next_desc() would return NULL, causing rz_dmac_xfer_desc() to
return 0 without programming the hardware.

The IRQ handler would then erroneously move the descriptor to ld_active. Later,
when the client finally calls rz_dmac_issue_pending(), it would find ld_queue
empty and do nothing.

Would this cause the transfer to remain stuck in ld_active and never execute?

Note that this issue is fixed later in the patch series by commit "dmaengine:
sh: rz-dmac: Use virt-dma APIs for channel descriptor processing".

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260512121219.216159-1-claudiu.beznea.uj@bp.renesas.com?part=3