Re: [PATCH v15 2/2] drm/bridge: Add Lontium LT7911EXC eDP to MIPI DSI bridge

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] `devm_drm_of_get_bridge()` in the hot-pluggable `lt7911exc_dsi_host_attach()` callback leaks devres memory and causes use-after-free upon panel unbind/rebind.
- [High] Double removal of the DRM bridge due to a flawed `list_empty` check in `lt7911exc_remove()`, leading to double mutex destruction and use-after-free.
- [High] Firmware upgrade clears the `upgrade` flag AFTER sending the hotplug event, causing potential modesets to fail silently.
- [Medium] Direct mutation of `pre_enable_prev_first` on an external `drm_bridge` object owned by a downstream driver.
--

--- Patch [2]: [PATCH v15 2/2] drm/bridge: Add Lontium LT7911EXC eDP to MIPI DSI bridge ---
commit 4c1fba2c711f35f533a59a3680b0b1fda9a82f94
Author: Sunyun Yang <syyang@lontium.com>

drm/bridge: Add Lontium LT7911EXC eDP to MIPI DSI bridge
    
Add support for the Lontium LT7911EXC bridge chip, which converts
eDP input to MIPI DSI output using an internal firmware-controlled
pipeline.

> diff --git a/drivers/gpu/drm/bridge/lontium-lt7911exc.c b/drivers/gpu/drm/bridge/lontium-lt7911exc.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/lontium-lt7911exc.c
[ ... ]
> +static void lt7911exc_firmware_upgrade_work(struct work_struct *work)
> +{
[ ... ]
> +	if (ret)
> +		dev_err(dev, "failed to read version after upgrade\n");
> +
> +	/* Notify DRM to re-trigger modeset after firmware upgrade */
> +	if (lt7911exc->bridge.dev)
> +		drm_kms_helper_hotplug_event(lt7911exc->bridge.dev);

[Severity: High]
Could this hotplug event fire before the upgrade flag is cleared?

Since drm_kms_helper_hotplug_event() triggers modesets synchronously
(or fast async), it might call lt7911exc_atomic_pre_enable() while
lt7911exc->upgrade is still true. If that happens, pre_enable() will
silently skip writing the hardware enable registers, resulting in a 
black screen.

Would it be safer to clear the upgrade flag before firing the hotplug
event?

> +
> +	goto out_release_fw;
> +
> +out_mcu_run:
> +	scoped_guard(mutex, &lt7911exc->ocm_lock)
> +		lt7911exc_inside_mcu_run(lt7911exc);
> +
> +out_release_fw:
> +	release_firmware(fw);
> +
> +out_unlock_upgrade:
> +	scoped_guard(mutex, &lt7911exc->ocm_lock) {
> +		if (!lt7911exc->removed)
> +			lt7911exc->upgrade = false;
> +	}
> +	mutex_unlock(&lt7911exc->upgrade_lock);
> +}
[ ... ]
> +static int lt7911exc_dsi_host_attach(struct mipi_dsi_host *host, struct mipi_dsi_device *dev)
> +{
> +	struct lt7911exc *lt7911exc = dsi_host_to_lt7911exc(host);
> +
> +	if (dev->lanes > 4) {
> +		dev_err(lt7911exc->dev, "unsupported number of data lanes(%u)\n", dev->lanes);
> +		return -EINVAL;
> +	}
> +
> +	lt7911exc->output_bridge = devm_drm_of_get_bridge(lt7911exc->dev, host->dev->of_node, 1, 0);

[Severity: High]
Could this devm allocation leak devres memory and cause a use-after-free?

The devm_drm_of_get_bridge() allocation is tied to the host device
(lt7911exc->dev), but it runs in lt7911exc_dsi_host_attach() which can
be called repeatedly if the downstream DSI panel is unbound and rebound.

Because the allocation is tied to the host device, it won't be freed until
the host driver unbinds. This leaks memory on every panel rebind. Also, the
retained drm_panel_bridge will hold a dangling pointer after the panel is
destroyed, risking a use-after-free if accessed later.

> +	if (IS_ERR(lt7911exc->output_bridge))
> +		return PTR_ERR(lt7911exc->output_bridge);
> +
> +	/*
> +	 * LT7911EXC requires the panel to be powered and ready before
> +	 * enabling MIPI DSI output. Keep default order (panel first)
> +	 * regardless of panel's prepare_prev_first setting, since the
> +	 * chip's internal firmware handles panel initialization.
> +	 */
> +	lt7911exc->output_bridge->pre_enable_prev_first = false;

[Severity: Medium]
Is it safe to directly mutate the state of an external bridge object?

This output_bridge is owned and managed by the downstream driver. Setting
its pre_enable_prev_first field from this host driver violates object
ownership and could override necessary configuration expected by the
downstream driver.

> +
> +	drm_bridge_add(&lt7911exc->bridge);
> +
> +	return 0;
> +}
[ ... ]
> +static void lt7911exc_remove(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	struct lt7911exc *lt7911exc = dev_get_drvdata(dev);
> +
> +	if (!lt7911exc)
> +		return;
> +
> +	scoped_guard(mutex, &lt7911exc->ocm_lock) {
> +		lt7911exc->removed = true;
> +		lt7911exc->upgrade = true;
> +	}
> +
> +	cancel_work_sync(&lt7911exc->work);
> +
> +	dev_set_drvdata(dev, NULL);
> +
> +	mipi_dsi_host_unregister(&lt7911exc->dsi_host);
> +
> +	if (!list_empty(&lt7911exc->bridge.list))
> +		drm_bridge_remove(&lt7911exc->bridge);

[Severity: High]
Will this result in a double free of the bridge?

During module unload, mipi_dsi_host_unregister() detaches all DSI children,
which calls lt7911exc_dsi_host_detach(). That explicitly calls
drm_bridge_remove() and moves the list node to the global
bridge_lingering_list.

When execution reaches this list_empty() check, it will evaluate to false
because the node is on the lingering list (and its next pointer isn't
pointing to itself). This causes drm_bridge_remove() to be erroneously
called a second time on the same bridge, leading to a double free and
double mutex_destroy().

> +
> +	gpiod_set_value_cansleep(lt7911exc->reset_gpio, 1);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714064822.2753289-1-syyang@lontium.com?part=2