Re: [PATCH] dmaengine: ppc4xx: fix format specifiers for size_t arguments

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing check for dma_map_page() failure in device probe.
- [High] Calling dma_free_coherent() with an uninitialized (NULL) device pointer in the ioremap error path.
- [High] Missing free_irq() for adev->err_irq in the error path of ppc440spe_adma_setup_irqs.
--

commit 6506d0727c09a20b6e812b6d3009a7ffe42bfacf
Author: Rosen Penev <rosenp@gmail.com>

dmaengine: ppc4xx: fix format specifiers for size_t arguments

Several dev_err, dev_dbg, pr_debug, and printk calls use %d or %u format
specifiers for size_t arguments. Fix them to use %zu to avoid warnings
on 64-bit builds.

> diff --git a/drivers/dma/ppc4xx/adma.c b/drivers/dma/ppc4xx/adma.c
> index 279a431ccae38..bcc54d584e2bf 100644
> --- a/drivers/dma/ppc4xx/adma.c
> +++ b/drivers/dma/ppc4xx/adma.c
[ ... ]
> @@ -4077,7 +4077,7 @@ static int ppc440spe_adma_probe(struct platform_device *ofdev)
>  					adev->pool_size, &adev->dma_desc_pool,
>  					GFP_KERNEL);
>  	if (adev->dma_desc_pool_virt == NULL) {
> -		dev_err(&ofdev->dev, "failed to allocate %d bytes of coherent "
> +		dev_err(&ofdev->dev, "failed to allocate %zu bytes of coherent "
>  			"memory for hardware descriptors\n",
>  			adev->pool_size);
>  		initcode = PPC_ADMA_INIT_COHERENT;

[Severity: High]
This isn't a bug introduced by this patch, but while looking at
ppc440spe_adma_probe(), I noticed we map the destination pages without
checking for mapping errors:

    chan->pdest = dma_map_page(&ofdev->dev, chan->pdest_page, 0,
                               PAGE_SIZE, DMA_BIDIRECTIONAL);
    chan->qdest = dma_map_page(&ofdev->dev, chan->qdest_page, 0,
                               PAGE_SIZE, DMA_BIDIRECTIONAL);

If the mapping fails, it returns an error address. Does this mean the
hardware might be configured with an invalid DMA destination, potentially
causing memory corruption or IOMMU faults?

Should we be validating the return values with dma_mapping_error()?

[Severity: High]
This is another pre-existing issue in ppc440spe_adma_probe(), but I'm
curious about the error handling if ioremap() fails:

    regs = ioremap(res.start, resource_size(&res));
    if (!regs) {
        dev_err(&ofdev->dev, "failed to ioremap regs!\n");
        ret = -ENOMEM;
        goto err_regs_alloc;
    }

This jumps to err_regs_alloc before adev->dev is initialized, where we do:

err_regs_alloc:
    dma_free_coherent(adev->dev, adev->pool_size,
                      adev->dma_desc_pool_virt,
                      adev->dma_desc_pool);

Since adev->dev is still NULL at this point, could this cause a crash
in dma_free_coherent() when trying to resolve the DMA operations?

[Severity: High]
A third pre-existing issue I noticed is in the error path of
ppc440spe_adma_setup_irqs(), which is called from this probe function.

If we fail to find the device tree node after successfully registering
adev->err_irq:

    np = of_find_compatible_node(NULL, NULL, "ibm,i2o-440spe");
    if (!np) {
        pr_err("%s: can't find I2O device tree node\n", __func__);
        ret = -ENODEV;
        goto err_req2;
    }

We jump to err_req2, which frees adev->irq but misses freeing adev->err_irq:

err_req2:
    free_irq(adev->irq, chan);
err_req1:
    irq_dispose_mapping(adev->irq);
err_irq_map:
    if (adev->err_irq > 0) {
        if (atomic_dec_and_test(&ppc440spe_adma_err_irq_ref))
            irq_dispose_mapping(adev->err_irq);
    }

Since the probe function later frees the chan structure on failure,
could this leave a dangling interrupt handler pointing to the freed
chan pointer?

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