Re: [PATCH v4 01/24] dmaengine: dw-edma: Add dw_edma_core_ll_cur_idx() to get current LL entry index

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The linked-list index computation lacks bounds checking and underflows on PCIe link loss, returning a massive positive integer that bypasses negative error checks.
--

--- Patch [1]: [PATCH v4 01/24] dmaengine: dw-edma: Add dw_edma_core_ll_cur_idx() to get current LL entry index ---
commit 97ea1bc0d0cff65116038aa18e1ba530b5bbe7c9
Author: Frank Li <Frank.Li@nxp.com>

dmaengine: dw-edma: Add dw_edma_core_ll_cur_idx() to get current LL entry index

This commit adds dw_edma_core_ll_cur_idx() to retrieve the current LL entry
index, preparing for dynamic addition of DMA requests while the DMA engine
is running.

> diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> index abc1bd4c0be2e..7b9a95ecedbb6 100644
> --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c

[ ... ]

> @@ -616,6 +616,24 @@ static void dw_edma_v0_core_ch_doorbell(struct dw_edma_chan *chan)
>  		  FIELD_PREP(EDMA_V0_DOORBELL_CH_MASK, chan->id));
>  }
>  
> +static int dw_edma_v0_core_ll_cur_idx(struct dw_edma_chan *chan)
> +{
> +	u32 base, val;
> +
> +	val = GET_CH_32(chan->dw, chan->dir, chan->id, llp.lsb);
> +	base = lower_32_bits(dw_edma_core_get_ll_paddr(chan));
> +
> +	/*
> +	 * LL regions stay within one 4 GiB address window. If the low word is
> +	 * zero, use the high word to distinguish a nonzero boundary address
> +	 * from an unprogrammed all-zero context.
> +	 */
> +	if (!val && !GET_CH_32(chan->dw, chan->dir, chan->id, llp.msb))
> +		return -EINVAL;
> +
> +	return (val - base) / EDMA_LL_SZ;

[Severity: High]
If a PCIe link loss or hardware error occurs, the MMIO read for llp.lsb could
return all-Fs (0xFFFFFFFF). Could this cause the unsigned subtraction of
(val - base) to underflow into a massive positive integer?

If val is 0xFFFFFFFF, the !val check is bypassed. After division by EDMA_LL_SZ
(24), this yields a very large positive index that fits comfortably within a
signed 32-bit int. Will this bypass negative error checks in callers and lead
to an out-of-bounds memory access when the index is used?

> +}

[ ... ]

> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> index 36ee72efcd317..d849220f3b5d3 100644
> --- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c

[ ... ]

> @@ -384,6 +384,24 @@ static void dw_hdma_v0_core_ch_doorbell(struct dw_edma_chan *chan)
>  	SET_CH_32(dw, chan->dir, chan->id, doorbell, HDMA_V0_DOORBELL_START);
>  }
>  
> +static int dw_hdma_v0_core_ll_cur_idx(struct dw_edma_chan *chan)
> +{
> +	u32 base, val;
> +
> +	val = GET_CH_32(chan->dw, chan->dir, chan->id, llp.lsb);
> +	base = lower_32_bits(dw_edma_core_get_ll_paddr(chan));
> +
> +	/*
> +	 * LL regions stay within one 4 GiB address window. If the low word is
> +	 * zero, use the high word to distinguish a nonzero boundary address
> +	 * from an unprogrammed all-zero context.
> +	 */
> +	if (!val && !GET_CH_32(chan->dw, chan->dir, chan->id, llp.msb))
> +		return -EINVAL;
> +
> +	return (val - base) / EDMA_LL_SZ;

[Severity: High]
Like the v0 core implementation, does this HDMA arithmetic need bounds checking
to prevent a stale hardware pointer where val < base, or a PCIe link loss,
from returning a large positive integer instead of a negative error code?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729143036.3087722-1-den@valinux.co.jp?part=1