Re: [PATCH v5 2/9] dmaengine: dma-axi-dmac: Use bus width capability helpers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] dmaengine: axi-dmac: Incorrect copy_align assignment causes UB for >= 32-byte widths
--

commit 32d178f66f02ed239c1f773e106395cd1e81ae9c
Author: Nuno Sá <nuno.sa@analog.com>
Subject: dmaengine: dma-axi-dmac: Use bus width capability helpers

This commit transitions the AXI DMA controller driver to use dma_bus_width_set()
to advertise source and destination bus widths. This allows the driver to express
widths of 32 bytes and above while maintaining legacy fields.

> diff --git a/drivers/dma/dma-axi-dmac.c b/drivers/dma/dma-axi-dmac.c
> index d47ff27e1408f..ca9cf1e0c5e46 100644
> --- a/drivers/dma/dma-axi-dmac.c
> +++ b/drivers/dma/dma-axi-dmac.c
[ ... ]
> @@ -1262,8 +1262,13 @@ static int axi_dmac_probe(struct platform_device *pdev)
>  	dma_dev->device_terminate_all = axi_dmac_terminate_all;
>  	dma_dev->device_synchronize = axi_dmac_synchronize;
>  	dma_dev->dev = &pdev->dev;
> -	dma_dev->src_addr_widths = BIT(dmac->chan.src_width);
> -	dma_dev->dst_addr_widths = BIT(dmac->chan.dest_width);
> +	ret = dma_bus_width_set(dma_dev->src_bus_widths, dmac->chan.src_width);
> +	if (ret)
> +		return ret;
> +
> +	ret = dma_bus_width_set(dma_dev->dst_bus_widths, dmac->chan.dest_width);

[Severity: High]
Does this migration to support widths of 32 bytes and above expose an
undefined behavior in the alignment checks?

Later in axi_dmac_probe(), the copy alignment is assigned the literal byte
width:

    dma_dev->copy_align = (dmac->chan.address_align_mask + 1);

However, dma_dev->copy_align expects a log2 alignment exponent (as defined
by enum dmaengine_alignment). The DMA core uses dmaengine_check_align(),
which calculates the alignment using a bitwise shift:

include/linux/dmaengine.h:dmaengine_check_align() {
    ...
    return !(((1 << align) - 1) & (off1 | off2 | len));
}

With this patch explicitly allowing widths of 32 bytes and above,
copy_align can now reach 32. This results in an operation of 1 << 32,
which triggers undefined behavior on 32-bit arithmetic, potentially
invalidating DMA transfer alignment checks.

> +	if (ret)
> +		return ret;
>  	dma_dev->directions = BIT(dmac->chan.direction);
>  	dma_dev->residue_granularity = DMA_RESIDUE_GRANULARITY_DESCRIPTOR;
>  	dma_dev->max_sg_burst = 31; /* 31 SGs maximum in one burst */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-dmaengine-support-wider-dma-masks-v5-0-4c4e63a7a1af@analog.com?part=2