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 <[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
