Re: [PATCH v12 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] Dangling pointer created during MIPI DSI host detach if the downstream device is a bridge rather than a panel.
- [High] Firmware upgrade CRC mismatch due to unpadded partial page writes.
- [High] Firmware upgrade workqueue accesses `bridge.dev` without ensuring the DRM device is alive, leading to a Use-After-Free.
- [High] Regmap page selector cache desynchronization after device reset causes firmware upgrade commands to be routed to the wrong register page.
--

commit 43236ff612b99bc861cbbc490c9d4569107b4d46
Author: Sunyun Yang <syyang@lontium.com>
Subject: drm/bridge: Add Lontium LT7911EXC eDP to MIPI DSI bridge

This commit adds support for the Lontium LT7911EXC bridge chip, which converts 
eDP input to MIPI DSI output. It provides DRM bridge integration, MIPI DSI 
host interfaces, I2C firmware upgrade mechanisms, and GPIO resets.

> diff --git a/drivers/gpu/drm/bridge/lontium-lt7911exc.c b/drivers/gpu/drm/bridge/lontium-lt7911exc.c
> new file mode 100644
> index 0000000000000..de6953ed1e3b0
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/lontium-lt7911exc.c
[ ... ]
> +static int lt7911exc_write_data(struct lt7911exc *lt7911exc, const struct firmware *fw, u64 addr)
> +{
> +	struct device *dev = lt7911exc->dev;
> +	int ret;
> +	int page = 0, num = 0, page_len = 0;
> +	u64 size, offset;
> +	const u8 *data;
> +
> +	data = fw->data;
> +	size = fw->size;
> +	page = (size + LT_PAGE_SIZE - 1) / LT_PAGE_SIZE;
[ ... ]
> +	for (num = 0; num < page; num++) {
> +		offset = num * LT_PAGE_SIZE;
> +		page_len = (offset + LT_PAGE_SIZE <= size) ? LT_PAGE_SIZE : (size - offset);
> +		ret = lt7911exc_prog_init(lt7911exc, addr);
> +		if (ret)
> +			return ret;
> +
> +		ret = regmap_raw_write(lt7911exc->regmap, 0xe05d, &data[offset], page_len);

[Severity: High]
The software CRC in lt7911exc_firmware_upgrade_work() is calculated over a
buffer padded with 0xff up to FW_SIZE - 4. However, this function writes the
original unpadded fw->data.

If fw->size is not a multiple of the 32-byte page size, does the short write
leave residual data in the hardware's internal page buffer? This could cause
a CRC mismatch and firmware upgrade failure since the padded bytes are
actually flashed. Should this function write the padded buffer instead?

[ ... ]
> +static void lt7911exc_firmware_upgrade_work(struct work_struct *work)
> +{
[ ... ]
> +	lt7911exc_reset(lt7911exc);
> +	mutex_lock(&lt7911exc->ocm_lock);
> +	lt7911exc_hw_mcu_halt(lt7911exc);

[Severity: High]
When lt7911exc_reset() asserts the reset GPIO, the physical hardware's page
control register is reset to 0x00.

Does the regmap cache know about this external reset? If not, regmap might
assume the hardware is already on page 0xe0 and omit the page control write
during lt7911exc_hw_mcu_halt(). 

Can this route commands to the wrong page and corrupt hardware state? Should
the regmap cache be synchronized or marked dirty after a reset?

[ ... ]
> +out_unlock:
> +	mutex_lock(&lt7911exc->ocm_lock);
> +	lt7911exc_hw_mcu_run(lt7911exc);
> +	lt7911exc->fw_version = lt7911exc_read_version(lt7911exc);
> +	mutex_unlock(&lt7911exc->ocm_lock);
> +
> +	mutex_unlock(&lt7911exc->upgrade_lock);
> +	/* Notify DRM framework that hardware state changed/needs a modeset */
> +	if (lt7911exc->bridge.dev)
> +		drm_kms_helper_hotplug_event(lt7911exc->bridge.dev);

[Severity: High]
Is it possible for lt7911exc->bridge.dev to be a dangling pointer here?

The DRM core does not clear bridge->dev on bridge detachment. Since the sysfs
attribute is tied to the I2C device (which remains bound), could a user
trigger a firmware upgrade after the KMS driver unbinds? This would lead to a
use-after-free when drm_kms_helper_hotplug_event() executes.

[ ... ]
> +static int lt7911exc_dsi_host_detach(struct mipi_dsi_host *host, struct mipi_dsi_device *dev)
> +{
> +	struct lt7911exc *lt7911exc = dsi_host_to_lt7911exc(host);
> +
> +	if (!lt7911exc->output.dev)
> +		return 0;
> +
> +	if (lt7911exc->output.panel && lt7911exc->output.bridge) {
> +		drm_panel_bridge_remove(lt7911exc->output.bridge);
> +		lt7911exc->output.bridge = NULL;
> +		lt7911exc->output.panel = NULL;
> +	}
> +
> +	lt7911exc->output.dev = NULL;
> +
> +	return 0;
> +}

[Severity: High]
If the downstream device is a raw bridge instead of a panel, output.panel
will be NULL, causing this block to be skipped.

Does this leave output.bridge as a dangling pointer? If the bridge is freed
and the display pipeline is re-attached, lt7911exc_bridge_attach() might
dereference this stale pointer and cause a use-after-free.

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