Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] NULL pointer dereference in `drm_state_dump` if the initial atomic bridge state allocation fails. -- --- Patch [7]: [PATCH v2 07/37] drm/bridge: cros-ec-anx7688: Add atomic bridge state handlers --- commit 45bb4ccee956e78effd5cb908d42d26a9cefaebb Author: Maxime Ripard <[email protected]> drm/bridge: cros-ec-anx7688: Add atomic bridge state handlers This commit adds atomic state handlers to the cros-ec-anx7688 bridge. This is done to remove the legacy bridge callback support from the DRM bridge core. > diff --git a/drivers/gpu/drm/bridge/cros-ec-anx7688.c > b/drivers/gpu/drm/bridge/cros-ec-anx7688.c > --- a/drivers/gpu/drm/bridge/cros-ec-anx7688.c > +++ b/drivers/gpu/drm/bridge/cros-ec-anx7688.c [ ... ] > @@ -92,6 +93,9 @@ static bool cros_ec_anx7688_bridge_mode_fixup(struct > drm_bridge *bridge, > } > > static const struct drm_bridge_funcs cros_ec_anx7688_bridge_funcs = { > + .atomic_create_state = drm_atomic_helper_bridge_create_state, [Severity: High] This isn't a bug introduced by this patch, but adding atomic state handlers causes drm_bridge_is_atomic() to return true, which exposes a pre-existing NULL pointer dereference in the DRM core. When drm_bridge_attach() calls drm_atomic_private_obj_init(), the object is added to privobj_list before the state is successfully allocated: drivers/gpu/drm/drm_atomic.c:drm_atomic_private_obj_init() { list_add_tail(&obj->head, &dev->mode_config.privobj_list); state = obj->funcs->atomic_create_state(obj); if (IS_ERR(state)) return PTR_ERR(state); obj->state = state; } If atomic_create_state() fails, drm_bridge_attach() ignores the error: drivers/gpu/drm/drm_bridge.c:drm_bridge_attach() { if (drm_bridge_is_atomic(bridge)) drm_atomic_private_obj_init(bridge->dev, &bridge->base, &drm_bridge_priv_state_funcs); return 0; } This leaves the object on privobj_list with a NULL obj->state. If drm_state_dump() is later triggered, it will crash when it unconditionally dereferences the NULL state: drivers/gpu/drm/drm_atomic.c:drm_atomic_private_obj_print_state() { struct drm_private_obj *obj = state->obj; } Could drm_bridge_attach() be updated to check the return value of drm_atomic_private_obj_init(), or should drm_atomic_private_obj_init() remove the object from the list on failure? > + .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, > + .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state, > .mode_fixup = cros_ec_anx7688_bridge_mode_fixup, > }; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=7
