Re: [PATCH v17 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 5 potential issue(s) to consider:
- [High] Missing cleanup of `output_bridge` in `mipi_dsi_host_detach` prevents DSI device rebinds and causes use-after-free.
- [High] Sysfs attribute `lt7911exc_firmware` is accessible before the driver has finished initialization in `probe`, leading to a kernel panic.
- [High] Asynchronous firmware upgrade worker accesses `bridge.dev` without holding a reference, causing a Use-After-Free if the DRM display controller unbinds.
- [Medium] Driver illegitimately mutates the state of a downstream bridge object by explicitly modifying `pre_enable_prev_first`.
- [Low] The `lt7911exc->removed` flag is read without a lock or memory barrier in the firmware upgrade work, leading to a data race.
--

--- Patch [2]: [PATCH v17 2/2] drm/bridge: Add Lontium LT7911EXC eDP to MIPI DSI bridge ---
commit f9cec7dee2928b841f4104ddf68c1ebfeffba41a
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
> index 0000000000000..5b7a184e96194
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/lontium-lt7911exc.c

[ ... ]

> @@ -458,6 +458,25 @@ static void lt7911exc_firmware_upgrade_work(struct work_struct *work)
>  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;
>  	}
>  
>  	/* Notify DRM to re-trigger modeset after firmware upgrade */
>  	if (!lt7911exc->removed && lt7911exc->bridge.dev)

[Severity: Low]
Is there a data race on the removed flag here?

The lt7911exc->removed flag is set under ocm_lock in lt7911exc_remove(), but
it is read here outside of any lock or memory barrier.

While cancel_work_sync() in the remove path prevents a logical use-after-free,
could this still trigger unmarked data race warnings from KCSAN?

[Severity: High]
Can this code trigger a use-after-free if the DRM display controller unbinds?

The firmware upgrade work accesses bridge.dev asynchronously. The bridge.dev
pointer is assigned when the DRM master attaches, but the I2C bridge driver
holds no reference to it.

If a user starts a firmware upgrade via sysfs and then unbinds the DRM
master driver before the async work completes, the drm_device is freed.

Does this leave a dangling pointer that the worker will then dereference in
drm_kms_helper_hotplug_event()?

>  		drm_kms_helper_hotplug_event(lt7911exc->bridge.dev);
>  
>  	mutex_unlock(&lt7911exc->upgrade_lock);
>  }

[ ... ]

> @@ -515,10 +515,31 @@ static int lt7911exc_dsi_host_attach(struct mipi_dsi_host *host, struct mipi_dsi_device *dev)
>  	lt7911exc->output_bridge = bridge;
>  
>  	/*
>  	 * Force panel-first enable order: the DSI output stream (0xe0b0)
>  	 * must not be enabled before the panel is powered on.
>  	 * This override is safe because LT7911EXC's transfer() is a stub —
>  	 * panel DCS commands are silently discarded and panel init is done
>  	 * by the chip's internal firmware. The panel driver's
>  	 * prepare_prev_first preference is therefore irrelevant here.
>  	 */
>  	lt7911exc->output_bridge->pre_enable_prev_first = false;

[Severity: Medium]
Is it safe to explicitly mutate the state of a downstream bridge object?

The downstream bridge object belongs to the panel driver. Overriding its
pre_enable_prev_first flag violates object ownership and could disrupt the
panel's expected power-on sequence.

Could this cause the panel to fail to power on correctly?

>  
>  	drm_bridge_add(&lt7911exc->bridge);
>  
>  	return 0;
>  }
>  
>  static int lt7911exc_dsi_host_detach(struct mipi_dsi_host *host, struct mipi_dsi_device *dev)
>  {
>  	struct lt7911exc *lt7911exc = dsi_host_to_lt7911exc(host);
>  
>  	drm_bridge_remove(&lt7911exc->bridge);
>  
>  	return 0;
>  }

[Severity: High]
Does this missing cleanup prevent DSI device rebinds and cause a
use-after-free?

In lt7911exc_dsi_host_attach(), output_bridge is initialized, but it is never
cleared back to NULL in this detach function.

If the downstream DSI panel is unbound via sysfs, the old wrapper remains,
pointing to a freed panel.

If the panel is subsequently rebound, the check for output_bridge in
lt7911exc_dsi_host_attach() will falsely trigger and return -EBUSY,
permanently breaking the display pipeline. Should output_bridge be set to
NULL here?

[ ... ]

> @@ -662,11 +662,20 @@ static int lt7911exc_probe(struct i2c_client *client)
>  	lt7911exc = devm_drm_bridge_alloc(dev, struct lt7911exc, bridge, &lt7911exc_bridge_funcs);
>  	if (IS_ERR(lt7911exc))
>  		return dev_err_probe(dev, PTR_ERR(lt7911exc), "drm bridge alloc failed.\n");
>  
>  	dev_set_drvdata(dev, lt7911exc);
>  
>  	lt7911exc->dev = dev;
>  	lt7911exc->upgrade = false;
>  	lt7911exc->removed = false;
>  
>  	ret = devm_mutex_init(dev, &lt7911exc->ocm_lock);

[Severity: High]
Could a concurrent sysfs write during probe trigger a kernel panic?

Because lt7911exc_attr_groups is attached via driver.dev_groups, the driver
core creates the sysfs files before probe() runs.

Here, dev_set_drvdata() exposes the struct before ocm_lock, upgrade_lock,
and the work struct are initialized.

If a user writes to the sysfs file during this window,
lt7911exc_firmware_store() will retrieve the partially initialized struct and
attempt to lock the uninitialized mutex. Should the drvdata be set after
initialization is complete?

>  	if (ret)
>  		return dev_err_probe(dev, ret, "failed to init mutex\n");
>  
>  	ret = devm_mutex_init(dev, &lt7911exc->upgrade_lock);

[ ... ]

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