Re: [PATCH] drm/mipi-dsi: fix UAF on device_add() failure

[sashiko-bot@kernel.org]

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

New issues:
- [High] The patch introduces inconsistent ownership semantics for the `info->node` reference on error paths, creating an unresolvable reference counting ambiguity for callers.

Pre-existing issues:
- [High] Double-put of `device_node` references in DSI device callers leading to Use-After-Free.
--

--- Patch [1]: [PATCH] drm/mipi-dsi: fix UAF on device_add() failure ---
commit a9cfa4f20408a5cdd4a82d99b49a2b8290aff096
Author: Linkai Gong <gonglinkai@kylinos.cn>

drm/mipi-dsi: fix UAF on device_add() failure

This commit modifies the error handling in mipi_dsi_device_register_full()
when mipi_dsi_device_add() fails. It replaces kfree() with put_device()
to ensure mipi_dsi_dev_release() is called to clean up the device resources.

> diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
> index 3ac1dd5ad640a..1b4c0acbd30d2 100644
> --- a/drivers/gpu/drm/drm_mipi_dsi.c
> +++ b/drivers/gpu/drm/drm_mipi_dsi.c
> @@ -233,7 +233,7 @@ mipi_dsi_device_register_full(struct mipi_dsi_host *host,
>  	ret = mipi_dsi_device_add(dsi);
>  	if (ret) {
>  		dev_err(host->dev, "failed to add DSI device %d\n", ret);
> -		kfree(dsi);
> +		put_device(&dsi->dev);

[Severity: High]
Does this create an inconsistent ownership contract for the caller?

If mipi_dsi_device_register_full() fails early (e.g., info->channel > 3, or
mipi_dsi_device_alloc() fails), it returns ERR_PTR() without putting
the info->node reference.

If it fails late at mipi_dsi_device_add(), the new put_device() call triggers
mipi_dsi_dev_release() which consumes the info->node reference via
of_node_put().

Since both cases return an opaque error pointer, the caller has no way to
know whether the node reference was consumed or not. If the caller drops the
reference on error, it causes a double-put on late failures. If the caller
retains it, it leaks on early failures.

[Severity: High]
This is a pre-existing issue, but this patch makes it manifest on the error
path. Are callers actually holding an extra reference to info->node when
calling mipi_dsi_device_register_full()?

Looking at existing drivers, many pass an unowned reference. For example,
in drivers/gpu/drm/panel/panel-raydium-rm69380.c:rm69380_probe():

	dsi_sec_host = of_find_mipi_dsi_host_by_node(dsi_sec);
	of_node_put(dsi_sec);
	...
	ctx->dsi[1] = devm_mipi_dsi_device_register_full(dev, dsi_sec_host, &info);

The driver explicitly drops the reference before registration. When
mipi_dsi_dev_release() calls of_node_put(dev->of_node) unconditionally,
this causes a use-after-free.

Similarly, in drivers/gpu/drm/bridge/lontium-lt9611.c:lt9611_attach_dsi():

	const struct mipi_dsi_device_info info = { "lt9611", 0, lt9611->dev->of_node};
	...
	dsi = devm_mipi_dsi_device_register_full(dev, host, &info);

The node is passed without acquiring an extra reference, which will also
result in a double-put when the device is released.

>  		return ERR_PTR(ret);
>  	}
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806075452.2361689-1-gonglinkai@kylinos.cn?part=1