Re: [PATCH v11 11/19] dmaengine: ti: k3-udma: move inclusion of k3-udma-private.c to k3-udma-common.c

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] mm: UAF in of_xudma_dev_get() due to dereference after put_device()
- [High] mm: UAF in xudma_dev_put() due to devm_ unbind race
- [Medium] Commit message claims 'No functional changes intended' but adds features
--

commit d7ff936b088ca2c00890bff99f72a6c55660576f
Author: Sai Sree Kartheek Adivi <s-adivi@ti.com>

dmaengine: ti: k3-udma: move inclusion of k3-udma-private.c to k3-udma-common.c

Relocate the #include directive for k3-udma-private.c to k3-udma-common.c
to share code between UDMA variants and prepare for variant-specific
implementations.

>     No functional changes intended.

[Severity: Medium]
The commit message explicitly asserts "No functional changes intended", but the
patch actively introduces functional module pinning logic (try_module_get).
Could the commit message be updated to accurately reflect these lifecycle
changes?

> diff --git a/drivers/dma/ti/k3-udma-private.c b/drivers/dma/ti/k3-udma-private.c
> index 44c097fff5ee6..d2501e074bcfb 100644
> --- a/drivers/dma/ti/k3-udma-private.c
> +++ b/drivers/dma/ti/k3-udma-private.c
> @@ -54,10 +54,26 @@ struct udma_dev *of_xudma_dev_get(struct device_node *np, const char *property)
>  		return ERR_PTR(-EPROBE_DEFER);
>  	}
>  
> +	/*
> +	 * Callers keep using ud (via function pointers stored in it, e.g.
> +	 * ud->psil_pair) after this call returns, bypassing the dmaengine
> +	 * core's own try_module_get() protection. Pin the owning driver's
> +	 * module here so it can't be unloaded out from under a live glue
> +	 * client; xudma_dev_put() releases it.
> +	 */
> +	if (!try_module_get(ud->ddev.owner))

[Severity: High]
Does this code introduce a use-after-free race?

Earlier in of_xudma_dev_get(), put_device(&pdev->dev) is called, which drops
the reference to the platform device:

	ud = platform_get_drvdata(pdev);
	put_device(&pdev->dev);
	if (!ud) {
		pr_debug("UDMA has not been probed\n");
		return ERR_PTR(-EPROBE_DEFER);
	}

Immediately after this, the new code executes try_module_get(ud->ddev.owner).

If a concurrent device unbind (e.g., via sysfs) occurs in this tight window,
the devres-managed ud object can be freed. The subsequent access to
ud->ddev.owner dereferences the freed pointer.

Should put_device() be called after we are done accessing ud?

> +		return ERR_PTR(-ENODEV);
> +
>  	return ud;
>  }
>  EXPORT_SYMBOL(of_xudma_dev_get);
>  
> +void xudma_dev_put(struct udma_dev *ud)
> +{
> +	module_put(ud->ddev.owner);

[Severity: High]
Can this dereference a freed pointer upon device unbind?

The ud pointer is implicitly managed by devres. When of_xudma_dev_get() returns
it to the client, it explicitly drops the device reference without establishing
a device link. The module pinning added here does not prevent sysfs device
unbinding.

If the UDMA device is unbound, devres frees ud. Later, when the client cleans
up, it calls this new xudma_dev_put(), which dereferences the already-freed
ud->ddev.owner.

> +}

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