* Re: [PATCH] dmaengine: dw-axi-dmac: fix __le32 on set of CH_CTL_H_LLI_VALID
2026-06-17 8:49 [PATCH] dmaengine: dw-axi-dmac: fix __le32 on set of CH_CTL_H_LLI_VALID Ben Dooks
@ 2026-06-17 9:02 ` sashiko-bot
2026-06-17 14:12 ` Frank Li
1 sibling, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-06-17 9:02 UTC (permalink / raw)
To: Ben Dooks; +Cc: vkoul, dmaengine, Frank.Li
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] The `status_lo` field of `struct axi_dma_lli` is a little-endian `__le32` value, but it is passed directly without byte-swapping to `axi_chan_irq_clear()`. Furthermore, `status_lo` contains block transfer status, not an interrupt clear mask.
- [High] `axi_chan_block_xfer_complete` uses the global channel-wide allocation counter `chan->descs_allocated` to iterate over the local `desc->hw_desc` array, leading to a slab out-of-bounds access.
- [High] `dw_axi_dma_chan_prep_cyclic` produces incorrect segment lengths if `period_len` is not evenly divisible by `num_segments`, causing cyclic buffer overflows and missed callbacks.
--
commit 7955a4e962eae80371df48e83bbcd1b7445eaf76
Author: Ben Dooks <ben.dooks@codethink.co.uk>
dmaengine: dw-axi-dmac: fix __le32 on set of CH_CTL_H_LLI_VALID
This patch fixes an endianness issue in the dw-axi-dmac driver by using
cpu_to_le32() when setting the CH_CTL_H_LLI_VALID bit in the LLI control
high register.
> diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> @@ -1123,7 +1123,7 @@ static void axi_chan_block_xfer_complete(struct axi_dma_chan *chan)
> hw_desc = &desc->hw_desc[i];
[Severity: High]
This is a pre-existing issue, but since we are indexing desc->hw_desc
here, can this loop read out of bounds?
The loop limit count is derived from the global channel-wide allocation:
dw-axi-dmac-platform.c:axi_chan_block_xfer_complete()
int count = atomic_read(&chan->descs_allocated);
However, the array desc->hw_desc is sized specifically for the current
transaction (desc->nr_hw_descs). If multiple descriptors are prepared on
the same channel, won't descs_allocated be larger than desc->nr_hw_descs
and cause this loop to access memory past the end of the array?
> if (hw_desc->llp == llp) {
> axi_chan_irq_clear(chan, hw_desc->lli->status_lo);
[Severity: High]
This isn't a bug introduced by this patch, but addressing the question in
the commit message about whether passing lli->status_lo is a bug: yes,
is this writing garbage bits to the interrupt clear register?
The status_lo field is an __le32 value being passed directly to
axi_chan_irq_clear() which expects a CPU-endian u32. More importantly,
status_lo contains the hardware status writeback of the LLI, not an IRQ
bitmask.
Writing this to CH_INTCLEAR could clear unrelated interrupts, such as
critical error interrupts like DWAXIDMAC_IRQ_ALL_ERR, before they can
be handled.
> - hw_desc->lli->ctl_hi |= CH_CTL_H_LLI_VALID;
> + hw_desc->lli->ctl_hi |= cpu_to_le32(CH_CTL_H_LLI_VALID);
> desc->completed_blocks = i;
>
> if (((hw_desc->len * (i + 1)) % desc->period_len) == 0)
[Severity: High]
This is a pre-existing issue, but does this modulo condition fail to trigger
callbacks if the period length isn't evenly divisible?
In dw_axi_dma_chan_prep_cyclic(), the segment length is calculated with
DIV_ROUND_UP():
dw-axi-dmac-platform.c:dw_axi_dma_chan_prep_cyclic()
segment_len = DIV_ROUND_UP(period_len, num_segments);
If period_len is not a clean multiple of num_segments, the unconditionally
padded segment lengths mean the mathematical condition here will never
evaluate to 0, permanently breaking vchan_cyclic_callback() execution.
Additionally, would the padded segment lengths cause an out-of-bounds DMA
memory transfer exceeding the actual buffer length?
> vchan_cyclic_callback(vd);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617084944.705266-1-ben.dooks@codethink.co.uk?part=1
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] dmaengine: dw-axi-dmac: fix __le32 on set of CH_CTL_H_LLI_VALID
2026-06-17 8:49 [PATCH] dmaengine: dw-axi-dmac: fix __le32 on set of CH_CTL_H_LLI_VALID Ben Dooks
2026-06-17 9:02 ` sashiko-bot
@ 2026-06-17 14:12 ` Frank Li
2026-06-17 14:16 ` Ben Dooks
1 sibling, 1 reply; 4+ messages in thread
From: Frank Li @ 2026-06-17 14:12 UTC (permalink / raw)
To: Ben Dooks; +Cc: Eugeniy Paltsev, Vinod Koul, Frank Li, dmaengine, linux-kernel
On Wed, Jun 17, 2026 at 09:49:43AM +0100, Ben Dooks wrote:
>
> When writing the lli->ctl_hi, this is an __le32 type so the
> value being orred should be convered to __le32 by cpu_to_le32.
Not sure if it can pass sparse warning check.
Frank
>
> Fixes 1deb96c0fa58a ("dmaegine: dw-axi-dmac: Support device_prep_dma_cyclic()")
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> --
> Note, the call to axi_chan_irq_clear() is passing lli->status_lo
> through which is also an __le32 but it does not seem to be set
> anywhere. Is this also a bug?
> ---
> drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> index 4d53f077e9d2..8311df2f11bb 100644
> --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> @@ -1123,7 +1123,7 @@ static void axi_chan_block_xfer_complete(struct axi_dma_chan *chan)
> hw_desc = &desc->hw_desc[i];
> if (hw_desc->llp == llp) {
> axi_chan_irq_clear(chan, hw_desc->lli->status_lo);
> - hw_desc->lli->ctl_hi |= CH_CTL_H_LLI_VALID;
> + hw_desc->lli->ctl_hi |= cpu_to_le32(CH_CTL_H_LLI_VALID);
> desc->completed_blocks = i;
>
> if (((hw_desc->len * (i + 1)) % desc->period_len) == 0)
> --
> 2.37.2.352.g3c44437643
>
^ permalink raw reply [flat|nested] 4+ messages in thread