Re: [PATCH 2/3] drm/panel: himax-hx83121a: pass the panel pointer when creating BL

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing device link to secondary DSI host can lead to Use-After-Free if the host driver is unbound.
- [High] The backlight device is exposed to userspace before the DSI device is attached to the host, allowing premature DCS transfers.
- [High] Unprotected MIPI DSI hardware access due to missing backlight disable allows DCS commands to a powered-off panel.
--

commit 4b6dd00d8fec3ba9b10693589e0413bbbce2ec2f
Author: Icenowy Zheng <zhengxingda@iscas.ac.cn>

drm/panel: himax-hx83121a: pass the panel pointer when creating BL

This commit modifies the backlight creation function to accept the entire
panel context instead of just the DSI device. This prepares the driver for
future backlight powering on/off support.

> diff --git a/drivers/gpu/drm/panel/panel-himax-hx83121a.c b/drivers/gpu/drm/panel/panel-himax-hx83121a.c
> index bed79aa06f46a..1a7e0125bced8 100644
> --- a/drivers/gpu/drm/panel/panel-himax-hx83121a.c
> +++ b/drivers/gpu/drm/panel/panel-himax-hx83121a.c
> @@ -193,10 +193,11 @@ static const struct drm_panel_funcs himax_panel_funcs = {
>  
>  static int himax_bl_update_status(struct backlight_device *bl)
>  {
> -	struct mipi_dsi_device *dsi = bl_get_data(bl);
> +	struct himax *ctx = bl_get_data(bl);
>  	u16 brightness = backlight_get_brightness(bl);
>  	/* TODO: brightness to raw map table */
> -	return mipi_dsi_dcs_set_display_brightness_large(dsi, brightness);
> +	return mipi_dsi_dcs_set_display_brightness_large(to_primary_dsi(ctx),
> +							 brightness);

[Severity: High]
This isn't a bug introduced by this patch, but does this function leave the
MIPI DSI hardware vulnerable to unprotected access?

Because ctx->backlight is not assigned to ctx->panel.backlight during
probe, the DRM core does not automatically manage its state. In
himax_unprepare(), the panel's regulators are disabled and the reset GPIO
is asserted, but there is no corresponding backlight_disable() call.

If userspace writes to the backlight sysfs interface after the panel is
powered off, this function will unconditionally send DCS commands via
mipi_dsi_dcs_set_display_brightness_large() to the off panel. This
typically causes the MIPI DSI host to experience transfer timeouts and
potential hardware errors.

[ ... ]

> @@ -646,7 +647,7 @@ static int himax_probe(struct mipi_dsi_device *dsi)
>  	ctx->panel.prepare_prev_first = true;
>  
>  	if (desc->has_dcs_backlight) {
> -		ctx->backlight = himax_create_backlight(to_primary_dsi(ctx));
> +		ctx->backlight = himax_create_backlight(ctx);

[Severity: High]
This is a pre-existing issue, but does himax_create_backlight() expose the
device to userspace too early?

In himax_probe(), this registers the backlight device and immediately exposes
its sysfs interface. However, this happens before devm_mipi_dsi_attach() is
called later in the probe routine.

If userspace writes to the brightness attribute in this window,
himax_bl_update_status() could send DCS commands over the MIPI DSI bus
before the hardware is actually attached to the host, which could result in
transfers on uninitialized hardware.

[Severity: High]
This is also a pre-existing issue, but does himax_probe() need to establish
a device link to the secondary DSI host?

Earlier in himax_probe(), the secondary DSI host is looked up and the
device is created:

    dsi1_host = of_find_mipi_dsi_host_by_node(dsi1);
    ...
    ctx->dsi[1] = devm_mipi_dsi_device_register_full(dev, dsi1_host, &dsi_info);

Without establishing a device link (e.g., using device_link_add()) to
ensure proper driver unbind synchronization, if the secondary DSI host driver
is unbound, its mipi_dsi_host and the associated DSI device are unregistered
and freed.

Since the panel driver remains bound and retains ctx->dsi[1], a subsequent
DCS command or modeset could access this freed memory, leading to a
use-after-free.

>  		if (IS_ERR(ctx->backlight))
>  			return dev_err_probe(dev, PTR_ERR(ctx->backlight),
>  					     "Failed to create backlight\n");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608162622.403713-1-zhengxingda@iscas.ac.cn?part=2