[PATCH] drm/panel: novatek-nt35950: fix potential use-after-free on attach failure

[PATCH] drm/panel: novatek-nt35950: fix potential use-after-free on attach failure

[Guangshuo Li]

nt35950_probe() registers the panel before attaching it to the DSI
hosts. If mipi_dsi_attach() fails, the probe error path returns without
removing the panel from the DRM panel registry.

The panel is allocated with devm_drm_panel_alloc(), so its storage is
released after probe returns an error. Leaving the panel registered
therefore leaves the global panel list pointing at freed memory, which
can result in a potential use-after-free when the stale entry is later
accessed.

There is an additional cleanup issue for dual-DSI panels. If attaching
DSI0 succeeds but attaching DSI1 fails, the error path unregisters the
secondary DSI device without detaching the already attached DSI0.

The failure sequence is:

  drm_panel_add()
        |
        v
  attach DSI0
        |
        | success
        v
  attach DSI1
        |
        | failure
        v
  probe cleanup
        |
        +-- DSI0 remains attached
        |
        +-- panel remains registered
        |
        v
  probe returns error
        |
        v
  devm panel storage is released
        |
        v
  panel registry contains a dangling pointer

Detach any DSI hosts that were successfully attached before the
failure and remove the panel from the DRM panel registry before
returning the probe error.

This issue was found by manual code inspection.

Fixes: 623a3531e9cf ("drm/panel: Add driver for Novatek NT35950 DSI DriverIC panels")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/gpu/drm/panel/panel-novatek-nt35950.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-novatek-nt35950.c b/drivers/gpu/drm/panel/panel-novatek-nt35950.c
index 94aa6489d99f..8edd0aa363ce 100644
--- a/drivers/gpu/drm/panel/panel-novatek-nt35950.c
+++ b/drivers/gpu/drm/panel/panel-novatek-nt35950.c
@@ -514,12 +514,19 @@ static int nt35950_probe(struct mipi_dsi_device *dsi)
 
 		ret = mipi_dsi_attach(nt->dsi[i]);
 		if (ret < 0) {
-			/* If we fail to attach to either host, we're done */
+			dev_err_probe(dev, ret,
+				      "Cannot attach to DSI%d host.\n", i);
+
+			/* Detach from all previously attached DSI hosts */
+			while (i--)
+				mipi_dsi_detach(nt->dsi[i]);
+
 			if (num_dsis == 2)
 				mipi_dsi_device_unregister(nt->dsi[1]);
 
-			return dev_err_probe(dev, ret,
-					     "Cannot attach to DSI%d host.\n", i);
+			drm_panel_remove(&nt->panel);
+
+			return ret;
 		}
 	}
 
-- 
2.43.0

Re: [PATCH] drm/panel: novatek-nt35950: fix potential use-after-free on attach failure

[sashiko-bot]

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

Pre-existing issues:
- [High] Panel is published globally before initialization is complete, exposing a race condition that leads to Use-After-Free of DSI devices and devm-managed resources if an error occurs.
--

commit 06a7a4ea81475e17e3e167d525f0a49d30e5896a
Author: Guangshuo Li <lgs201920130244@gmail.com>
    
drm/panel: novatek-nt35950: fix potential use-after-free on attach failure
    
This commit attempts to fix a potential use-after-free issue by detaching
DSI hosts and removing the panel from the DRM panel registry if
mipi_dsi_attach() fails during probe.

> diff --git a/drivers/gpu/drm/panel/panel-novatek-nt35950.c b/drivers/gpu/drm/panel/panel-novatek-nt35950.c
> index 94aa6489d99fc..8edd0aa363cec 100644
> --- a/drivers/gpu/drm/panel/panel-novatek-nt35950.c
> +++ b/drivers/gpu/drm/panel/panel-novatek-nt35950.c
> @@ -514,12 +514,19 @@ static int nt35950_probe(struct mipi_dsi_device *dsi)
>  
>  		ret = mipi_dsi_attach(nt->dsi[i]);
>  		if (ret < 0) {
> -			/* If we fail to attach to either host, we're done */
> +			dev_err_probe(dev, ret,
> +				      "Cannot attach to DSI%d host.\n", i);
> +
> +			/* Detach from all previously attached DSI hosts */
> +			while (i--)
> +				mipi_dsi_detach(nt->dsi[i]);
> +
>  			if (num_dsis == 2)
>  				mipi_dsi_device_unregister(nt->dsi[1]);
>  
> -			return dev_err_probe(dev, ret,
> -					     "Cannot attach to DSI%d host.\n", i);
> +			drm_panel_remove(&nt->panel);
> +
> +			return ret;

[Severity: High]
This is a pre-existing issue, but does this still leave a race condition since
the panel is published globally before mipi_dsi_attach() is called?

Earlier in nt35950_probe(), the panel is published globally:

	drm_panel_add(&nt->panel);

If a concurrent thread (like a display controller) discovers the panel via
of_drm_find_panel() and acquires a reference, and then mipi_dsi_attach()
fails, this error path executes:

	if (num_dsis == 2)
		mipi_dsi_device_unregister(nt->dsi[1]);

	drm_panel_remove(&nt->panel);

The concurrent thread will keep the panel structure alive. If that thread
subsequently initiates a display modeset, it will call drm_panel_prepare()
which invokes nt35950_on() and dereferences the now-freed secondary DSI
device:

	nt->dsi[1]->mode_flags |= MIPI_DSI_MODE_LPM;

Could drm_panel_add() be moved to the very end of the probe function, after
all initializations (including mipi_dsi_attach) have successfully completed?

>  		}
>  	}
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260913131849.1561742-1-lgs201920130244@gmail.com?part=1