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 <[email protected]> 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/[email protected]?part=2
