Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [High] Unconditional NULL pointer dereference for bridges missing atomic state callbacks. Pre-existing issues: - [High] Lockless modifications of `encoder->bridge_chain` and `dev->mode_config.privobj_list` cause list corruption. -- --- Patch [37]: [PATCH v2 37/37] drm/bridge: Remove legacy bridge callback support --- commit 7f35866581b3c5c247d4a1cdd1535127f3999993 Author: Maxime Ripard <[email protected]> drm/bridge: Remove legacy bridge callback support This commit removes legacy bridge callback support, as all bridge drivers are expected to have been converted to atomic variants. The private object state is now initialized unconditionally at attach time. > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c > index 6ead9b75ae3de..6fb443984dff3 100644 > --- a/drivers/gpu/drm/drm_bridge.c > +++ b/drivers/gpu/drm/drm_bridge.c > @@ -629,9 +624,8 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct > drm_bridge *bridge, > goto err_reset_bridge; > } > > - if (drm_bridge_is_atomic(bridge)) > - drm_atomic_private_obj_init(bridge->dev, &bridge->base, > - &drm_bridge_priv_state_funcs); > + drm_atomic_private_obj_init(bridge->dev, &bridge->base, > + &drm_bridge_priv_state_funcs); [Severity: High] Does this unconditionally crash for bridges that don't implement atomic state callbacks? If a bridge (like aspire_ec_bridge_funcs in drivers/platform/arm64/acer-aspire1-ec.c) is missing .atomic_create_state, drm_atomic_private_obj_init() will call drm_bridge_atomic_create_priv_state() which blindly executes bridge->funcs->atomic_create_state(bridge). Could this result in a NULL pointer dereference during initialization? > return 0; [ ... ] > @@ -664,8 +658,7 @@ void drm_bridge_detach(struct drm_bridge *bridge) > if (WARN_ON(!bridge->dev)) > return; > > - if (drm_bridge_is_atomic(bridge)) > - drm_atomic_private_obj_fini(&bridge->base); > + drm_atomic_private_obj_fini(&bridge->base); [Severity: High] This is a pre-existing issue, but does drm_bridge_detach() remove the bridge from the encoder's bridge chain list locklessly? Looking at drm_bridge_detach() just below this block: list_del(&bridge->chain_node); It performs this deletion without acquiring encoder->bridge_chain_mutex. Could this race with list iterations like drm_atomic_bridge_chain_disable() that do hold the mutex? Additionally, drm_atomic_private_obj_fini() modifies dev->mode_config.privobj_list without synchronization. Can this cause list corruption if a bridge driver is unbound via sysfs during active modesets? > if (bridge->funcs->detach) > bridge->funcs->detach(bridge); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=37
