Re: [PATCH v7 01/19] dmaengine: ti: k3-udma: Fix sporadic crash on AM62x

[sashiko-bot@kernel.org]

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

New issues:
- [High] The commit claims to fix a sporadic crash by zero-initializing `ring_init_data`, but this is a placebo that masks the true root cause.

Pre-existing issues:
- [High] Incorrect use of `INIT_DELAYED_WORK_ONSTACK` on heap-allocated channel objects causes a debug objects leak.
- [Medium] Permanent memory and DMA mapping leak on probe error paths for `udma_setup_rx_flush` buffers.
--

--- Patch [1]: [PATCH v7 01/19] dmaengine: ti: k3-udma: Fix sporadic crash on AM62x ---
commit 5866c70a265652a85a02f5cc7fc41a1ae2f5bfae
Author: Vignesh Raghavendra <vigneshr@ti.com>

dmaengine: ti: k3-udma: Fix sporadic crash on AM62x

This commit attempts to fix a sporadic crash during ring pop on AM62x by
zero-initializing ring_init_data on the stack, stating that uninitialized
garbage led to junk addresses.

> diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c
> index fb21e0df5ab7b..48f4db777d924 100644
> --- a/drivers/dma/ti/k3-udma.c
> +++ b/drivers/dma/ti/k3-udma.c
> @@ -5484,7 +5484,7 @@ static int udma_probe(struct platform_device *pdev)
>  	if (ud->match_data->type == DMA_TYPE_UDMA) {
>  		ud->ringacc = of_k3_ringacc_get_by_phandle(dev->of_node, "ti,ringacc");
>  	} else {
> -		struct k3_ringacc_init_data ring_init_data;
> +		struct k3_ringacc_init_data ring_init_data = { 0 };
>  
>  		ring_init_data.tisci = ud->tisci_rm.tisci;
>  		ring_init_data.tisci_dev_id = ud->tisci_rm.tisci_dev_id;

[Severity: High]
Is zero-initializing this structure actually addressing the root cause, or 
could it be masking a stack layout issue?

Since struct k3_ringacc_init_data is exactly 16 bytes on 64-bit platforms
with no padding, and every single field is explicitly assigned right after
declaration, it shouldn't be structurally possible for it to contain
uninitialized garbage that is actually read by the subsequent initialization
functions. 

Could this change merely be shifting the stack layout and hiding an
out-of-bounds access or uninitialized pointer elsewhere in the driver?

[Severity: Medium]
This is a pre-existing issue, but are there missing DMA unmap calls on the
probe failure paths?

When udma_setup_rx_flush() succeeds, it maps memory for DMA:

drivers/dma/ti/k3-udma.c:udma_setup_rx_flush() {
    ...
    rx_flush->buffer_paddr = dma_map_single(dev, rx_flush->buffer_vaddr,
                                            rx_flush->buffer_size,
                                            DMA_TO_DEVICE);
    ...
}

If a later initialization step fails in udma_probe(), such as device
registration, the driver returns immediately without cleaning up those
mappings:

drivers/dma/ti/k3-udma.c:udma_probe() {
    ...
    ret = dma_async_device_register(&ud->ddev);
    if (ret) {
        dev_err(dev, "failed to register slave DMA engine: %d\n", ret);
        return ret;
    }
    ...
}

Will this result in a permanent resource leak of the mapped DMA space if 
probe aborts?

[Severity: High]
This is a pre-existing issue, but does using the _ONSTACK variant for
heap-allocated channels create a leak of debug objects?

During channel allocation for both bcdma and pktdma, the work item is
initialized using INIT_DELAYED_WORK_ONSTACK():

drivers/dma/ti/k3-udma.c:bcdma_alloc_chan_resources() {
    ...
    INIT_DELAYED_WORK_ONSTACK(&uc->tx_drain.work,
                              udma_check_tx_completion);
    ...
}

The channel structure is heap-allocated via devm_kcalloc(). Using the
_ONSTACK macro on heap memory registers a debug object that strictly
requires a matching destroy_work_on_stack() call to free. 

Since destroy_work_on_stack() is never called in udma_free_chan_resources(),
doesn't this permanently leak a debug object every time a channel is
requested by a client?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724102115.2854-1-s-adivi@ti.com?part=1