<[email protected]> 于2026年7月15日周三 10:06写道： > > 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.
Intentional design, consistent with cdns-dsi-core.c cdns_dsi_detach() (lines 1082-1091) which does not clear output->dev. Clearing output_bridge would allow re-attach, causing devm_drm_of_get_bridge() to allocate new devres without freeing the previous one, resulting in a real memory leak. The DRM bridge framework does not support dynamic bridge chain reconfiguration. No use-after-free: the -EBUSY guard returns before dereferencing output_bridge, and bridge_attach is not called by the SoC encoder after drm_bridge_remove(). > - [High] Sysfs attribute `lt7911exc_firmware` is accessible before the driver > has finished initialization in `probe`, leading to a kernel panic. False positive. The driver core creates dev_groups sysfs attributes AFTER probe() returns successfully. In drivers/base/dd.c really_probe(), call_driver_probe() (line 706) executes before device_add_groups(dev, drv->dev_groups) (line 725). By the time the sysfs file is visible to userspace, all mutexes, workqueue, and regmap are fully initialized. There is no window where userspace can access the sysfs attribute before initialization completes. > - [High] Asynchronous firmware upgrade worker accesses `bridge.dev` without > holding a reference, causing a Use-After-Free if the DRM display controller > unbinds. This is a known DRM bridge framework limitation, not specific to this driver. drm_bridge_remove() does not clear bridge.dev, so all bridge drivers calling drm_kms_helper_hotplug_event() share this risk. A proper fix requires framework-level bridge-to-drm_device lifetime tracking. Triggering requires root sysfs access to start firmware upgrade while simultaneously unbinding the DRM master driver — an administrator operation outside normal usage. > - [Medium] Driver illegitimately mutates the state of a downstream bridge > object by explicitly modifying `pre_enable_prev_first`. Hardware constraint. The DSI output stream (register 0xe0b0) must be enabled after the panel is powered on. The override is safe because transfer() is a stub — panel DCS commands are silently discarded and panel initialization is handled entirely by the chip's internal firmware. The panel driver's prepare_prev_first preference is therefore irrelevant for this chip. The comment above the assignment documents this rationale. > - [Low] The `lt7911exc->removed` flag is read without a lock or memory > barrier in the firmware upgrade work, leading to a data race. Valid. > -- > > --- Patch [2]: [PATCH v17 2/2] drm/bridge: Add Lontium LT7911EXC eDP to MIPI > DSI bridge --- > commit f9cec7dee2928b841f4104ddf68c1ebfeffba41a > Author: Sunyun Yang <[email protected]> > > 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, <7911exc->ocm_lock) > > lt7911exc_inside_mcu_run(lt7911exc); > > > > out_release_fw: > > release_firmware(fw); > > > > out_unlock_upgrade: > > scoped_guard(mutex, <7911exc->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(<7911exc->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(<7911exc->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(<7911exc->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, > > <7911exc_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, <7911exc->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, <7911exc->upgrade_lock); > > [ ... ] > > -- > Sashiko AI review · > https://sashiko.dev/#/patchset/[email protected]?part=2
