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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dmaengine: misleading commit message and invalid Fixes tag
--

commit bd206aee322a8bc9aa73f1f81b842b751453d7b0
Author: Vignesh Raghavendra <vigneshr@ti.com>

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

This commit zero-initializes the ring_init_data structure to prevent a
sporadic crash caused by garbage data on the stack.

> ring_init_data is declared on stack and thus can contain garbage which
> can lead to k3_ringacc_dmarings use junk address for ringrt base leading
> to below crash during ring pop
[ ... ]
> Fixes: 017794739702 ("dmaengine: ti: k3-udma: Initial support for K3 BCDMA")

[Severity: Medium]
Is this description and Fixes tag accurate for the code at this specific 
commit?

The commit message states this fixes a crash caused by a junk address for the
ringrt base. However, the base_rt field doesn't appear to be added to
struct k3_ringacc_init_data until Patch 12 of this series.

Could this lead to ineffective stable backports, given that the patch appears
to act as a placebo prior to Patch 12 being applied?

> diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c
> index 78a67cb9d6e00..8457644cb9132 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: Medium]
Does this zero-initialization actually change any behavior here?

Looking at udma_probe(), every existing field of this struct is explicitly
assigned immediately after the declaration:

		ring_init_data.tisci = ud->tisci_rm.tisci;
		ring_init_data.tisci_dev_id = ud->tisci_rm.tisci_dev_id;
		if (ud->match_data->type == DMA_TYPE_BCDMA) {
			ring_init_data.num_rings = ud->bchan_cnt + ...
		} else {
			ring_init_data.num_rings = ud->rflow_cnt + ...
		}

Since all fields are explicitly initialized, wouldn't the uninitialized memory
issue only manifest after Patch 12 adds the base_rt field?

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