Re: [PATCH v2 07/37] drm/bridge: cros-ec-anx7688: Add atomic bridge state handlers

[sashiko-bot@kernel.org]

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 <mripard@kernel.org>

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/20260707-drm-all-atomic-bridges-v2-0-21d03cbca446@kernel.org?part=7