Re: [PATCH v12 12/20] dmaengine: ti: k3-udma: pin owning module while glue clients hold a udma_dev

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] dmaengine: k3-udma: Use-After-Free in of_xudma_dev_get() due to early put_device()
- [High] dmaengine: ti: UAF due to empty device release for devm-managed struct
--

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

dmaengine: ti: k3-udma: pin owning module while glue clients hold a udma_dev

This commit modifies the k3-udma driver to pin the owning module when a glue
client obtains a udma_dev pointer. It sets the ddev.owner field, uses
try_module_get() in of_xudma_dev_get(), and introduces xudma_dev_put() to
release the module reference when a channel is released.

> diff --git a/drivers/dma/ti/k3-udma-glue.c b/drivers/dma/ti/k3-udma-glue.c
> index 70eaf7ee57e68..f9e2a1e1e0a50 100644
> --- a/drivers/dma/ti/k3-udma-glue.c
> +++ b/drivers/dma/ti/k3-udma-glue.c
> @@ -466,6 +466,9 @@ void k3_udma_glue_release_tx_chn(struct k3_udma_glue_tx_channel *tx_chn)
>  		device_unregister(&tx_chn->common.chan_dev);
>  		tx_chn->common.chan_dev.parent = NULL;
>  	}
> +
> +	if (!IS_ERR_OR_NULL(tx_chn->common.udmax))
> +		xudma_dev_put(tx_chn->common.udmax);

[Severity: High]
Does device_unregister() here cause a use-after-free regression because tx_chn
is a devm-managed structure with an empty release function?

When tx_chn is allocated with devm_kzalloc() in k3_udma_glue_request_tx_chn(),
its embedded chan_dev is registered. The device class release function
k3_udma_chan_dev_release() is deliberately empty because the structure is
devm-managed.

If unprivileged userspace holds an active reference to chan_dev's sysfs
directory (e.g., keeping a file descriptor open) during driver unbind, the
kobject remains alive after device_unregister() returns. Devres will then free
tx_chn. When userspace finally closes the file descriptor, kobject_put() will
access the already-freed tx_chn memory.

Could this be fixed by managing the memory lifecycle through the kobject
release callback instead of devres?

[ ... ]

> 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]
Can this code cause a use-after-free regression by dereferencing
ud->ddev.owner?

Looking at of_xudma_dev_get(), put_device(&pdev->dev) is called right before
this block:

drivers/dma/ti/k3-udma-private.c:of_xudma_dev_get() {
...
	ud = platform_get_drvdata(pdev);
	put_device(&pdev->dev);
	if (!ud) {
...
	}
...

If put_device() drops the final reference to the device (for example, due to a
concurrent device removal or DT overlay teardown), the device is unregistered
and its devres-managed memory, including the ud structure, could be freed
immediately.

Calling try_module_get(ud->ddev.owner) would then dereference a freed pointer.
Should try_module_get() be called before put_device() to ensure ud remains
valid?

> +		return ERR_PTR(-ENODEV);
> +
>  	return ud;
>  }

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