Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] Freed `regmap` and `backlight` resources are accessed during `atomic_enable` and `atomic_disable` if the driver is unbound while the bridge is still attached. - [High] The bridge's atomic_enable and atomic_disable callbacks execute sleeping functions, violating atomic context requirements. -- --- Patch [25]: [PATCH v2 25/37] drm/bridge: waveshare-dsi: Switch to atomic bridge callbacks --- commit 5ef73541c5abfd37236a332fd681aeaf873d8239 Author: Maxime Ripard <[email protected]> drm/bridge: waveshare-dsi: Switch to atomic bridge callbacks The waveshare-dsi bridge uses the legacy non-atomic enable and disable bridge callbacks. In order to remove the legacy bridge callback support from the DRM bridge core, switch to their atomic counterparts and add the bridge atomic state handlers. > diff --git a/drivers/gpu/drm/bridge/waveshare-dsi.c > b/drivers/gpu/drm/bridge/waveshare-dsi.c > index ded57f298d640..9c9825c9b7c59 100644 > --- a/drivers/gpu/drm/bridge/waveshare-dsi.c > +++ b/drivers/gpu/drm/bridge/waveshare-dsi.c [ ... ] > @@ -92,7 +93,8 @@ static int ws_bridge_bridge_attach(struct drm_bridge > *bridge, > &ws->bridge, flags); > } > > -static void ws_bridge_bridge_enable(struct drm_bridge *bridge) > +static void ws_bridge_bridge_enable(struct drm_bridge *bridge, > + struct drm_atomic_commit *commit) > { > struct ws_bridge *ws = bridge_to_ws_bridge(bridge); > > regmap_write(ws->reg_map, 0xad, 0x01); > backlight_enable(ws->backlight); [Severity: High] This isn't a bug introduced by this patch, but does accessing ws->reg_map and ws->backlight here risk a use-after-free if the driver is unbound? The bridge object is managed by DRM and stays alive as long as it's attached to the encoder. However, the regmap and backlight resources are device-managed (devm) and are freed immediately upon driver unbind (e.g., manual unbind via sysfs or hardware hot-unplug). If a modeset or DPMS change occurs after unbind, the DRM core calls the bridge's atomic_enable callback, which dereferences the freed ws->reg_map and ws->backlight. [Severity: High] This is a pre-existing issue, but does calling sleeping functions from this atomic callback violate context requirements? According to the DRM subsystem guidelines, drm_atomic_helper_commit_tail() (which calls these bridge callbacks) executes in atomic context where sleeping is forbidden. However, ws_bridge_bridge_enable() calls regmap_write() (which uses an I2C transfer and acquires a mutex) and backlight_enable() (which acquires a mutex), both of which can sleep. > } > > -static void ws_bridge_bridge_disable(struct drm_bridge *bridge) > +static void ws_bridge_bridge_disable(struct drm_bridge *bridge, > + struct drm_atomic_commit *commit) > { > struct ws_bridge *ws = bridge_to_ws_bridge(bridge); > > backlight_disable(ws->backlight); > regmap_write(ws->reg_map, 0xad, 0x00); [Severity: High] This is a pre-existing issue, but similar to the enable path, could these calls result in a use-after-free or violate atomic context requirements? If a modeset or DPMS change occurs after the driver is unbound, atomic_disable will dereference the freed ws->reg_map and ws->backlight. Additionally, calling backlight_disable() and regmap_write() executes sleeping functions in atomic context. > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=25
