Re: [PATCH] dmaengine: axi-dmac: fix incorrect segment size calculation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] dmaengine: axi-dmac: 32-bit overflow in axi_dmac_prep_slave_sg()
--

commit 6fad587f5aff185e8ba37fde1bcd2a484b6fa19d
Author: Sergey Matsievskiy <matsievskiysv@gmail.com>

dmaengine: axi-dmac: fix incorrect segment size calculation

This commit switches segment count calculations in axi-dmac from DIV_ROUND_UP
to DIV_ROUND_UP_ULL to avoid 32-bit unsigned integer overflow when
chan->max_length is 0xffffffff.

> diff --git a/drivers/dma/dma-axi-dmac.c b/drivers/dma/dma-axi-dmac.c
> index d47ff27e1408f..bfa83ccfd7dc0 100644
> --- a/drivers/dma/dma-axi-dmac.c
> +++ b/drivers/dma/dma-axi-dmac.c
> @@ -706,7 +706,7 @@ static struct axi_dmac_sg *axi_dmac_fill_linear_sg(struct axi_dmac_chan *chan,
>  	unsigned int len;
>  
>  	/* Split into multiple equally sized segments if necessary */
> -	num_segments = DIV_ROUND_UP(period_len, chan->max_length);
> +	num_segments = DIV_ROUND_UP_ULL(period_len, chan->max_length);

[Severity: High]
Can axi_dmac_prep_slave_sg() under-allocate desc->sg[] and write out of
bounds on 32-bit architectures when chan->max_length is 0xffffffff?

While axi_dmac_fill_linear_sg(), axi_dmac_prep_peripheral_dma_vec(), and
axi_dmac_prep_dma_cyclic() now use DIV_ROUND_UP_ULL(), axi_dmac_prep_slave_sg()
still calculates num_sgs via sg_nents_for_dma() before calling
axi_dmac_alloc_desc() and axi_dmac_fill_linear_sg():

drivers/dma/dma-axi-dmac.c:axi_dmac_prep_slave_sg() {
	...
	num_sgs = sg_nents_for_dma(sgl, sg_len, chan->max_length);
	desc = axi_dmac_alloc_desc(chan, num_sgs);
	if (!desc)
		return NULL;

	dsg = desc->sg;

	for_each_sg(sgl, sg, sg_len, i) {
		...
		dsg = axi_dmac_fill_linear_sg(chan, direction, sg_dma_address(sg), 1,
			sg_dma_len(sg), dsg);
	}
	...
}

In lib/scatterlist.c, sg_nents_for_dma() uses DIV_ROUND_UP() with unsigned
int sg_dma_len(sg) and size_t len:

lib/scatterlist.c:sg_nents_for_dma() {
	...
	for_each_sg(sgl, sg, sglen, i)
		nents += DIV_ROUND_UP(sg_dma_len(sg), len);
	return nents;
}

On 32-bit architectures where size_t is 32 bits wide, passing len = 0xffffffff
causes DIV_ROUND_UP(sg_dma_len(sg), len) to overflow 32-bit arithmetic and
evaluate to 0 for any scatterlist entry with length >= 2.

When that happens, axi_dmac_alloc_desc(chan, num_sgs) either dereferences
desc->sg[num_sgs - 1].hw out of bounds when num_sgs is 0:

drivers/dma/dma-axi-dmac.c:axi_dmac_alloc_desc() {
	...
	/* The last hardware descriptor will trigger an interrupt */
	desc->sg[num_sgs - 1].hw->flags = AXI_DMAC_HW_FLAG_LAST | AXI_DMAC_HW_FLAG_IRQ;
	...
}

or allocates fewer sg entries than needed while the updated
axi_dmac_fill_linear_sg() computes num_segments = 1 via DIV_ROUND_UP_ULL()
and advances dsg past the end of desc->sg[].

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105131547.305510-1-matsievskiysv@gmail.com?part=1