Re: [PATCH v3 2/5] drm/panel: add Novatek NT36850 panel driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] drm/panel: double free/UAF of secondary DSI device on host unbind
- [Low] drm/panel: NULL pointer dereference in nt36850_disable() for single-DSI panels
- [Low] drm/panel: Unused backlight field in struct panel_info
--

commit c61a43c5fad101fbd326985c7fa4cea8c858d4bd
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

drm/panel: add Novatek NT36850 panel driver

Add support for the Truly WQHD dual-DSI command mode panel built around
the Novatek NT36850, found on the display card for the Qualcomm SDM845
and SM8150 HDK. The driver is based on panel-novatek-nt36523, the power
and command sequences follow the vendor device tree.

> diff --git a/drivers/gpu/drm/panel/panel-novatek-nt36850.c b/drivers/gpu/drm/panel/panel-novatek-nt36850.c
> new file mode 100644
> index 0000000000000..d3530e3d5754b
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-novatek-nt36850.c

[ ... ]

> +struct panel_info {
> +	struct drm_panel panel;
> +	struct mipi_dsi_device *dsi[2];
> +	const struct panel_desc *desc;
> +	enum drm_panel_orientation orientation;
> +
> +	struct gpio_desc *reset_gpio;
> +	struct backlight_device *backlight;

[Severity: Low]
Is this backlight pointer used anywhere else in the driver? It appears to be
declared here but never initialized or accessed since the backlight is handled
entirely by the drm_panel framework via drm_panel_of_backlight().

> +	struct regulator *vddio;
> +	struct regulator *avdd;
> +	struct regulator *avee;
> +};

[ ... ]

> +static int nt36850_disable(struct drm_panel *panel)
> +{
> +	struct panel_info *pinfo = to_panel_info(panel);
> +	struct mipi_dsi_device *dsi0 = pinfo->dsi[0];
> +	struct mipi_dsi_device *dsi1 = pinfo->dsi[1];
> +	struct mipi_dsi_multi_context dsi_ctx = { .dsi = NULL };
> +
> +	dsi0->mode_flags &= ~MIPI_DSI_MODE_LPM;
> +	dsi1->mode_flags &= ~MIPI_DSI_MODE_LPM;

[Severity: Low]
Could this cause a NULL pointer dereference if a single-DSI panel descriptor
is ever added?

The driver's probe function conditionally initializes dsi1 only when
is_dual_dsi is true, but nt36850_disable() unconditionally fetches and
dereferences it here, and then passes it to mipi_dsi_dual().

> +
> +	mipi_dsi_dual(mipi_dsi_dcs_set_display_off_multi, &dsi_ctx, dsi0, dsi1);
> +	mipi_dsi_msleep(&dsi_ctx, 120);
> +	mipi_dsi_dual(mipi_dsi_dcs_enter_sleep_mode_multi, &dsi_ctx, dsi0, dsi1);

[ ... ]

> +static int nt36850_probe(struct mipi_dsi_device *dsi)
> +{

[ ... ]

> +	/* If the panel is dual dsi, register DSI1 */
> +	if (pinfo->desc->is_dual_dsi) {
> +		struct mipi_dsi_device_info dsi_info = {
> +			.type = "dsi-secondary",
> +		};
> +
> +		dsi1 = of_graph_get_remote_node(dsi->dev.of_node, 1, -1);
> +		if (!dsi1) {
> +			dev_err(dev, "cannot get secondary DSI node.\n");
> +			return -ENODEV;
> +		}
> +
> +		dsi1_host = of_find_mipi_dsi_host_by_node(dsi1);
> +		of_node_put(dsi1);
> +		if (!dsi1_host)
> +			return dev_err_probe(dev, -EPROBE_DEFER, "cannot get secondary DSI host\n");
> +
> +		pinfo->dsi[1] = devm_mipi_dsi_device_register_full(dev, dsi1_host, &dsi_info);

[Severity: High]
Will this cause a double free or use-after-free during device unbind?

The devres cleanup action for the secondary DSI device is tied to the primary
DSI device (dev). However, the secondary DSI device is instantiated as a child
of dsi1_host->dev.

If the secondary DSI host is unbound via sysfs or module removal,
mipi_dsi_host_unregister() will iterate over all its children and
unconditionally call mipi_dsi_device_unregister(), freeing the secondary
DSI device.

Later, when the primary DSI device is unbound, its devres cleanup executes
and calls mipi_dsi_device_unregister() on the already-freed pointer. Should
this lifecycle be tied directly to the secondary host's unbind instead?

> +		if (IS_ERR(pinfo->dsi[1])) {
> +			dev_err(dev, "cannot get secondary DSI device\n");
> +			return PTR_ERR(pinfo->dsi[1]);
> +		}
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015-sdm845-hdk-v3-0-1c179cd0d16f@oss.qualcomm.com?part=2