[PATCH] dma: loongson2-apb-cmc: fix NULL deref in residue computation

Stepan Ionichev posted 1 patch 1 month, 1 week ago
There is a newer version of this series
drivers/dma/loongson/loongson2-apb-cmc-dma.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] dma: loongson2-apb-cmc: fix NULL deref in residue computation
Posted by Stepan Ionichev 1 month, 1 week ago
loongson2_cmc_dma_desc_residue() takes a "desc" parameter that is the
descriptor whose residue should be computed. The body uses it
correctly via "desc->num_sgs" and "desc->sg_req[i].len", but the
cyclic check incorrectly looks at the channel's stale current
descriptor instead:

	if (lchan->desc->cyclic && next_sg == 0)
		return residue;

This breaks when the function is called from the vdesc fallback path
of loongson2_cmc_dma_tx_status():

	if (lchan->desc && cookie == lchan->desc->vdesc.tx.cookie)
		state->residue = ...desc_residue(lchan, lchan->desc, ...);
	else if (vdesc)
		state->residue = ...desc_residue(lchan, to_lmdma_desc(vdesc), 0);

The else-if branch is taken precisely when "lchan->desc" is NULL or
points to a different descriptor than the one being queried, so
dereferencing "lchan->desc->cyclic" inside the helper either NULL-
derefs or reads the wrong descriptor's flag.

smatch flags this:

  drivers/dma/loongson/loongson2-apb-cmc-dma.c:516
  loongson2_cmc_dma_tx_status() error: we previously assumed
  'lchan->desc' could be null (see line 512)

Use the "desc" parameter, matching how the rest of the function
already accesses fields of the descriptor under inspection.

Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
---
 drivers/dma/loongson/loongson2-apb-cmc-dma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/loongson/loongson2-apb-cmc-dma.c b/drivers/dma/loongson/loongson2-apb-cmc-dma.c
index 1c9a542ed..3b02bcd75 100644
--- a/drivers/dma/loongson/loongson2-apb-cmc-dma.c
+++ b/drivers/dma/loongson/loongson2-apb-cmc-dma.c
@@ -487,7 +487,7 @@ static size_t loongson2_cmc_dma_desc_residue(struct loongson2_cmc_dma_chan *lcha
 	ndtr = loongson2_cmc_dma_read(lddev, LOONGSON2_CMCDMA_CNDTR, lchan->id);
 	residue = ndtr << width;
 
-	if (lchan->desc->cyclic && next_sg == 0)
+	if (desc->cyclic && next_sg == 0)
 		return residue;
 
 	for (i = next_sg; i < desc->num_sgs; i++)
-- 
2.43.0
Re: [PATCH] dma: loongson2-apb-cmc: fix NULL deref in residue computation
Posted by Frank Li 1 month, 1 week ago
On Thu, May 07, 2026 at 07:31:53AM +0500, Stepan Ionichev wrote:
> loongson2_cmc_dma_desc_residue() takes a "desc" parameter that is the
> descriptor whose residue should be computed. The body uses it
> correctly via "desc->num_sgs" and "desc->sg_req[i].len", but the
> cyclic check incorrectly looks at the channel's stale current
> descriptor instead:
>
> 	if (lchan->desc->cyclic && next_sg == 0)
> 		return residue;
>
> This breaks when the function is called from the vdesc fallback path
> of loongson2_cmc_dma_tx_status():
>
> 	if (lchan->desc && cookie == lchan->desc->vdesc.tx.cookie)
> 		state->residue = ...desc_residue(lchan, lchan->desc, ...);
> 	else if (vdesc)
> 		state->residue = ...desc_residue(lchan, to_lmdma_desc(vdesc), 0);
>
> The else-if branch is taken precisely when "lchan->desc" is NULL or
> points to a different descriptor than the one being queried, so
> dereferencing "lchan->desc->cyclic" inside the helper either NULL-
> derefs or reads the wrong descriptor's flag.
>
> smatch flags this:
>
>   drivers/dma/loongson/loongson2-apb-cmc-dma.c:516
>   loongson2_cmc_dma_tx_status() error: we previously assumed

remove "we previously assumed"

>   'lchan->desc' could be null (see line 512)
>
> Use the "desc" parameter, matching how the rest of the function
> already accesses fields of the descriptor under inspection.
>

fix tags here.

Frank

> Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
> ---
>  drivers/dma/loongson/loongson2-apb-cmc-dma.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/dma/loongson/loongson2-apb-cmc-dma.c b/drivers/dma/loongson/loongson2-apb-cmc-dma.c
> index 1c9a542ed..3b02bcd75 100644
> --- a/drivers/dma/loongson/loongson2-apb-cmc-dma.c
> +++ b/drivers/dma/loongson/loongson2-apb-cmc-dma.c
> @@ -487,7 +487,7 @@ static size_t loongson2_cmc_dma_desc_residue(struct loongson2_cmc_dma_chan *lcha
>  	ndtr = loongson2_cmc_dma_read(lddev, LOONGSON2_CMCDMA_CNDTR, lchan->id);
>  	residue = ndtr << width;
>
> -	if (lchan->desc->cyclic && next_sg == 0)
> +	if (desc->cyclic && next_sg == 0)
>  		return residue;
>
>  	for (i = next_sg; i < desc->num_sgs; i++)
> --
> 2.43.0
>
[PATCH v2] dma: loongson2-apb-cmc: fix NULL deref in residue computation
Posted by Stepan Ionichev 1 month, 1 week ago
loongson2_cmc_dma_desc_residue() takes a "desc" parameter that is the
descriptor whose residue should be computed. The body uses it
correctly via "desc->num_sgs" and "desc->sg_req[i].len", but the
cyclic check incorrectly looks at the channel's stale current
descriptor instead:

	if (lchan->desc->cyclic && next_sg == 0)
		return residue;

This breaks when the function is called from the vdesc fallback path
of loongson2_cmc_dma_tx_status():

	if (lchan->desc && cookie == lchan->desc->vdesc.tx.cookie)
		state->residue = ...desc_residue(lchan, lchan->desc, ...);
	else if (vdesc)
		state->residue = ...desc_residue(lchan, to_lmdma_desc(vdesc), 0);

The else-if branch is taken precisely when "lchan->desc" is NULL or
points to a different descriptor than the one being queried, so
dereferencing "lchan->desc->cyclic" inside the helper either NULL-
derefs or reads the wrong descriptor's flag.

smatch flags the inconsistency:

  drivers/dma/loongson/loongson2-apb-cmc-dma.c:516
  loongson2_cmc_dma_tx_status() error: 'lchan->desc' could be
  null (see line 512)

Use the "desc" parameter, matching how the rest of the function
already accesses fields of the descriptor under inspection.

Fixes: 1c0028e725f1 ("dmaengine: loongson: New driver for the Loongson Multi-Channel DMA controller")
Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
---
v2:
- Drop "we previously assumed" from the smatch quote (Frank Li).
- Add Fixes: tag.

 drivers/dma/loongson/loongson2-apb-cmc-dma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/loongson/loongson2-apb-cmc-dma.c b/drivers/dma/loongson/loongson2-apb-cmc-dma.c
index 1c9a542ed..3b02bcd75 100644
--- a/drivers/dma/loongson/loongson2-apb-cmc-dma.c
+++ b/drivers/dma/loongson/loongson2-apb-cmc-dma.c
@@ -487,7 +487,7 @@ static size_t loongson2_cmc_dma_desc_residue(struct loongson2_cmc_dma_chan *lcha
 	ndtr = loongson2_cmc_dma_read(lddev, LOONGSON2_CMCDMA_CNDTR, lchan->id);
 	residue = ndtr << width;

-	if (lchan->desc->cyclic && next_sg == 0)
+	if (desc->cyclic && next_sg == 0)
 		return residue;

 	for (i = next_sg; i < desc->num_sgs; i++)
--
2.43.0
Re: [PATCH v2] dma: loongson2-apb-cmc: fix NULL deref in residue computation
Posted by Binbin Zhou 1 month, 1 week ago
Hi Stepan.

Thanks for your patch.

On 2026/5/8 01:50, Stepan Ionichev wrote:
> loongson2_cmc_dma_desc_residue() takes a "desc" parameter that is the
> descriptor whose residue should be computed. The body uses it
> correctly via "desc->num_sgs" and "desc->sg_req[i].len", but the
> cyclic check incorrectly looks at the channel's stale current
> descriptor instead:
> 
> 	if (lchan->desc->cyclic && next_sg == 0)
> 		return residue;
> 
> This breaks when the function is called from the vdesc fallback path
> of loongson2_cmc_dma_tx_status():
> 
> 	if (lchan->desc && cookie == lchan->desc->vdesc.tx.cookie)
> 		state->residue = ...desc_residue(lchan, lchan->desc, ...);
> 	else if (vdesc)
> 		state->residue = ...desc_residue(lchan, to_lmdma_desc(vdesc), 0);
> 
> The else-if branch is taken precisely when "lchan->desc" is NULL or
> points to a different descriptor than the one being queried, so
> dereferencing "lchan->desc->cyclic" inside the helper either NULL-
> derefs or reads the wrong descriptor's flag.
> 
> smatch flags the inconsistency:
> 
>    drivers/dma/loongson/loongson2-apb-cmc-dma.c:516
>    loongson2_cmc_dma_tx_status() error: 'lchan->desc' could be
>    null (see line 512)
> 
> Use the "desc" parameter, matching how the rest of the function
> already accesses fields of the descriptor under inspection.
> 
> Fixes: 1c0028e725f1 ("dmaengine: loongson: New driver for the Loongson Multi-Channel DMA controller")
> Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
LGTM.

Reviewed-by: Binbin Zhou <zhoubinbin@loongson.cn>
> ---
> v2:
> - Drop "we previously assumed" from the smatch quote (Frank Li).
> - Add Fixes: tag.
> 
>   drivers/dma/loongson/loongson2-apb-cmc-dma.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/loongson/loongson2-apb-cmc-dma.c b/drivers/dma/loongson/loongson2-apb-cmc-dma.c
> index 1c9a542ed..3b02bcd75 100644
> --- a/drivers/dma/loongson/loongson2-apb-cmc-dma.c
> +++ b/drivers/dma/loongson/loongson2-apb-cmc-dma.c
> @@ -487,7 +487,7 @@ static size_t loongson2_cmc_dma_desc_residue(struct loongson2_cmc_dma_chan *lcha
>   	ndtr = loongson2_cmc_dma_read(lddev, LOONGSON2_CMCDMA_CNDTR, lchan->id);
>   	residue = ndtr << width;
> 
> -	if (lchan->desc->cyclic && next_sg == 0)
> +	if (desc->cyclic && next_sg == 0)
>   		return residue;
> 
>   	for (i = next_sg; i < desc->num_sgs; i++)
> --
> 2.43.0

-- 
Thanks.
Binbin