Re: [PATCH v4 04/10] dmaengine: dw-axi-dmac: Use bus width capability helpers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] The interrupt handler is registered with IRQF_SHARED before channel registers are initialized, risking a fatal NULL pointer dereference.
- [High] Out-of-bounds write in axi_req_irqs due to unbounded irq_count from platform metadata.
- [High] Missing check for negative error code from platform_irq_count silently aborts IRQ registration but falsely returns success.
--

commit 547df6badf50a7c40c7c108bf82e51bb78feaed7
Author: Nuno Sá <nuno.sa@analog.com>

dmaengine: dw-axi-dmac: Use bus width capability helpers

Advertise the supported bus widths through dma_bus_width_set_many()
instead of assigning the legacy u32 masks directly. This keeps the
driver using the new bitmap representation while preserving legacy
fields during the transition.

> 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 eebed2474210d..ca0ab8dc5f596 100644
> --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
[ ... ]
> @@ -1482,6 +1469,20 @@ static int dw_probe(struct platform_device *pdev)
>  	unsigned int flags;
>  	u32 i;
>  	int ret;
> +	/*
> +	 * The set of bus widths supported by the DMA controller. DW AXI DMAC
> +	 * supports master data bus width up to 512 bits (for both AXI master
> +	 * interfaces), but it depends on IP block configuration.
> +	 */
> +	enum dma_slave_buswidth buswidths[] = {
> +		DMA_SLAVE_BUSWIDTH_1_BYTE,
> +		DMA_SLAVE_BUSWIDTH_2_BYTES,
> +		DMA_SLAVE_BUSWIDTH_4_BYTES,
> +		DMA_SLAVE_BUSWIDTH_8_BYTES,
> +		DMA_SLAVE_BUSWIDTH_16_BYTES,
> +		DMA_SLAVE_BUSWIDTH_32_BYTES,
> +		DMA_SLAVE_BUSWIDTH_64_BYTES,
> +	};
>  
>  	chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
>  	if (!chip)

[Severity: High]
This is a pre-existing issue, but does axi_req_irqs() properly handle a
negative error code from platform_irq_count()?

If platform_irq_count() returns a negative error (like -EPROBE_DEFER):

drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:axi_req_irqs() {
    ...
    int irq_count = platform_irq_count(pdev);

    for (i = 0; i < irq_count; i++) {
        ...
    }
    return 0;
}

The loop will evaluate to false immediately, and the function falsely
returns 0. This silently aborts IRQ registration and causes dw_probe() to
continue without deferring probe or registering interrupts.


[Severity: High]
This is a pre-existing issue, but can the loop in axi_req_irqs() cause an
out-of-bounds write?

drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:axi_req_irqs() {
    ...
    int irq_count = platform_irq_count(pdev);

    for (i = 0; i < irq_count; i++) {
        chip->irq[i] = platform_get_irq(pdev, i);
        ...
    }
}

The chip->irq array has a fixed size of DMAC_MAX_CHANNELS (32). If
platform_irq_count() returns an unbounded value greater than 32 from the
platform metadata, this will write past the end of the array and corrupt
adjacent memory.


[Severity: High]
This is a pre-existing issue, but could there be a NULL pointer dereference
if a shared interrupt fires early during dw_probe()?

In axi_req_irqs(), the interrupt is registered with IRQF_SHARED:

drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:axi_req_irqs() {
    ...
        ret = devm_request_irq(chip->dev, chip->irq[i], dw_axi_dma_interrupt,
                               IRQF_SHARED, KBUILD_MODNAME, chip);
    ...
}

However, the channel registers are initialized after this call returns,
in dw_probe():

drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:dw_probe() {
    ...
    ret = axi_req_irqs(pdev, chip);
    if (ret)
        return ret;

    INIT_LIST_HEAD(&dw->dma.channels);
    for (i = 0; i < hdata->nr_channels; i++) {
        struct axi_dma_chan *chan = &dw->chan[i];
        ...
        chan->chan_regs = chip->regs + COMMON_REG_LEN + i * CHAN_REG_LEN;
    ...
}

If another device sharing the same IRQ line triggers an interrupt before
the channel registers are initialized, dw_axi_dma_interrupt() will read
from the uninitialized (NULL) chan->chan_regs:

drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:dw_axi_dma_interrupt() {
    ...
    for (i = 0; i < dw->hdata->nr_channels; i++) {
        chan = &dw->chan[i];
        status = axi_chan_irq_read(chan);
        ...
    }
}

Will this cause a panic if the handler tries to access chan->chan_regs?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911-dmaengine-support-wider-dma-masks-v4-0-72b8fd4cc78e@analog.com?part=4