[PATCH 0/2] dmaengine: fix dead empty checks in mpc512x and rz-dmac

[PATCH 0/2] dmaengine: fix dead empty checks in mpc512x and rz-dmac

[Maoyi Xie]

Two dmaengine drivers use list_first_entry() and then test the
returned pointer against NULL. list_first_entry() never returns
NULL, so the NULL check is dead code. The author intent at both
sites was clear from the existing recovery path. Switch to
list_first_entry_or_null() so the existing NULL path runs.

The two sites were raised in an inquiry on 2026-05-20. Frank Li
confirmed and asked for a patch.

Maoyi Xie (2):
  dmaengine: mpc512x: fix dead empty check in mpc_dma_prep_slave_sg()
  dmaengine: rz-dmac: fix dead empty check in rz_dmac_chan_get_residue()

 drivers/dma/mpc512x_dma.c | 4 ++--
 drivers/dma/sh/rz-dmac.c  | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

--
2.34.1

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

[Maoyi Xie]

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.

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@gmail.com>
---
 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

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@nxp.com>
>
> 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@gmail.com>
> ---
>  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
>

[PATCH v2] dmaengine: mpc512x: fix dead empty check in mpc_dma_prep_slave_sg()

[Maoyi Xie]

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.

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.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Maoyi Xie <maoyixie.tju@gmail.com>
---
v2: Trim two paragraphs from the commit message per Frank Li's
    nit on v1. No code change. Carry forward Frank's Reviewed-by.
    Drop the rz-dmac patch from v1: Geert pointed out that
    Claudiu's "[PATCH v5 09/17] dmaengine: sh: rz-dmac: Use
    virt-dma APIs for channel descriptor processing" rewrites
    rz_dmac_chan_get_residue() through vchan_find_desc() and
    removes ld_active, which supersedes the fix.
    https://lore.kernel.org/r/20260512121219.216159-10-claudiu.beznea.uj@bp.renesas.com
v1: https://lore.kernel.org/r/20260521144755.3476353-2-maoyixie.tju@gmail.com

 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

Re: [PATCH v2] dmaengine: mpc512x: fix dead empty check in mpc_dma_prep_slave_sg()

[Maoyi Xie]

Thanks all. I'm dropping this one. Using list_first_entry_or_null activates
the recovery path, and calling mpc_dma_process_completed() from prep context
isn't safe. It runs client callbacks inline and can roll completed_cookie
backwards. The empty free list is real but rare (all 64 descriptors in
flight). The safe fix is to return NULL there without that call and let the
tasklet reclaim, but I can't test it on hardware, so I'll leave it to you.

Best,
Maoyi

[PATCH 2/2] dmaengine: rz-dmac: fix dead empty check in rz_dmac_chan_get_residue()

[Maoyi Xie]

rz_dmac_chan_get_residue() reads channel->ld_active with
list_first_entry() and then tests the returned pointer against
NULL. list_first_entry() never returns NULL. On an empty list it
returns container_of(&channel->ld_active, struct rz_dmac_desc,
node), an aliased pointer derived from the list head. The "return
0" shortcut is dead code.

If ld_active is ever empty here, current_desc points at
&channel->ld_active. The subsequent cookie and status processing
then reads bogus values from the head's neighbouring memory.

ld_active can be empty when a residue query races with descriptor
completion on another path. The author intent was clear from the
existing comment on the next-following check, which already
acknowledges that the descriptor "could now be complete". The
empty case is the limit of that race.

Use list_first_entry_or_null() so the empty case returns NULL and
the existing "return 0" path runs.

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@gmail.com>
---
 drivers/dma/sh/rz-dmac.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
index 625ff29024de..3dd76615881f 100644
--- a/drivers/dma/sh/rz-dmac.c
+++ b/drivers/dma/sh/rz-dmac.c
@@ -723,8 +723,8 @@ static u32 rz_dmac_chan_get_residue(struct rz_dmac_chan *channel,
 	u32 crla, crtb, i;
 
 	/* Get current processing virtual descriptor */
-	current_desc = list_first_entry(&channel->ld_active,
-					struct rz_dmac_desc, node);
+	current_desc = list_first_entry_or_null(&channel->ld_active,
+						struct rz_dmac_desc, node);
 	if (!current_desc)
 		return 0;
 
-- 
2.34.1

Re: [PATCH 2/2] dmaengine: rz-dmac: fix dead empty check in rz_dmac_chan_get_residue()

[Geert Uytterhoeven]

Hi Maoyi,

On Thu, 21 May 2026 at 16:48, Maoyi Xie <maoyixie.tju@gmail.com> wrote:
> rz_dmac_chan_get_residue() reads channel->ld_active with
> list_first_entry() and then tests the returned pointer against
> NULL. list_first_entry() never returns NULL. On an empty list it
> returns container_of(&channel->ld_active, struct rz_dmac_desc,
> node), an aliased pointer derived from the list head. The "return
> 0" shortcut is dead code.
>
> If ld_active is ever empty here, current_desc points at
> &channel->ld_active. The subsequent cookie and status processing
> then reads bogus values from the head's neighbouring memory.
>
> ld_active can be empty when a residue query races with descriptor
> completion on another path. The author intent was clear from the
> existing comment on the next-following check, which already
> acknowledges that the descriptor "could now be complete". The
> empty case is the limit of that race.
>
> Use list_first_entry_or_null() so the empty case returns NULL and
> the existing "return 0" path runs.
>
> 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@gmail.com>

Thanks for your patch!

> --- a/drivers/dma/sh/rz-dmac.c
> +++ b/drivers/dma/sh/rz-dmac.c
> @@ -723,8 +723,8 @@ static u32 rz_dmac_chan_get_residue(struct rz_dmac_chan *channel,
>         u32 crla, crtb, i;
>
>         /* Get current processing virtual descriptor */
> -       current_desc = list_first_entry(&channel->ld_active,
> -                                       struct rz_dmac_desc, node);
> +       current_desc = list_first_entry_or_null(&channel->ld_active,
> +                                               struct rz_dmac_desc, node);
>         if (!current_desc)
>                 return 0;
>

Note that proposed "[PATCH v5 09/17] dmaengine: sh: rz-dmac: Use
virt-dma APIs for channel descriptor processing" would remove this code.
https://lore.kernel.org/20260512121219.216159-10-claudiu.beznea.uj@bp.renesas.com

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/2] dmaengine: rz-dmac: fix dead empty check in rz_dmac_chan_get_residue()

[Maoyi Xie]

Hi Geert,

Thanks for the pointer. I had not seen Claudiu's v5 09/17 series.
Looking at it, rz_dmac_chan_get_residue() is rewritten through
virt_dma APIs (vchan_find_desc + channel->desc) and ld_active is
removed. The fix I sent is superseded.

I will drop the rz-dmac patch from v2 of this series and send only
the mpc512x patch.

Thanks,
Maoyi