Re: [PATCH 1/2] dmaengine: mpc512x: fix dead empty check in mpc_dma_prep_slave_sg()

[Frank Li]

On Thu, May 21, 2026 at 10:47:54PM +0800, Maoyi Xie wrote:
> mpc_dma_prep_slave_sg() reads mchan->free with list_first_entry()
> and then tests the returned pointer against NULL. list_first_entry()
> never returns NULL. On an empty free list it returns
> container_of(&mchan->free, struct mpc_dma_desc, node), an aliased
> pointer derived from the list head. The recovery path (drop lock,
> scan completed list, return NULL) is dead code.
>
> If the free list is ever empty here, the aliased mdesc points at
> &mchan->free. The list_del(&mdesc->node) that follows then runs on
> the head itself, corrupting mchan->free.next and mchan->free.prev.
>
> The free list is reachable empty when the descriptor pool is
> exhausted. The author intent was clear from the recovery path:
> release the lock, scan the completed list to free descriptors, and
> return NULL so the caller can retry.

Nit: You can skip above two parapraph. This problem is quite straight forwards

Reviewed-by: Frank Li <Frank.Li@xxxxxxx>
>
> Use list_first_entry_or_null() so the empty case returns NULL and
> the existing recovery path runs as intended.
>
> The same shape has been cleaned up elsewhere, for example in
> commit fbb8bc408027 ("net: qed: Remove redundant NULL checks after list_first_entry()"),
> commit c708d3fad421 ("crypto: atmel - use list_first_entry_or_null to simplify find_dev"),
> and commit 10379171f346 ("ksmbd: use list_first_entry_or_null for opinfo_get_list()").
> This site was missed by those cleanups.
>
> Signed-off-by: Maoyi Xie <maoyixie.tju@xxxxxxxxx>
> ---
>  drivers/dma/mpc512x_dma.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
> index 0adc8e01057e..f5934136efc4 100644
> --- a/drivers/dma/mpc512x_dma.c
> +++ b/drivers/dma/mpc512x_dma.c
> @@ -706,8 +706,8 @@ mpc_dma_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
>  	for_each_sg(sgl, sg, sg_len, i) {
>  		spin_lock_irqsave(&mchan->lock, iflags);
>
> -		mdesc = list_first_entry(&mchan->free,
> -						struct mpc_dma_desc, node);
> +		mdesc = list_first_entry_or_null(&mchan->free,
> +						 struct mpc_dma_desc, node);
>  		if (!mdesc) {
>  			spin_unlock_irqrestore(&mchan->lock, iflags);
>  			/* Try to free completed descriptors */
> --
> 2.34.1
>