Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] drm_bridge: ignored return from drm_atomic_private_obj_init causes crash - [Low] drm_bridge: degraded debug output for statically allocated bridges --
commit e5b998c537e0f9eb83be0fe1e66e8f70427ccfcc Author: Maxime Ripard <[email protected]> Subject: drm/bridge: Switch print_state to use bridge name This commit updates drm_bridge_atomic_print_priv_state() to print the human-readable bridge->name instead of the function pointer bridge->funcs. This is intended to make the state readout output more consistent and easier to interpret. > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c > index d474d60204110..447fe6d333b3b 100644 > --- a/drivers/gpu/drm/drm_bridge.c > +++ b/drivers/gpu/drm/drm_bridge.c > @@ -563,9 +563,9 @@ drm_bridge_atomic_print_priv_state(struct drm_printer *p, > struct drm_bridge *bridge = drm_priv_to_bridge(s->obj); > > if (bridge->of_node) > - drm_printf(p, "bridge: %ps (%pOFfc)\n", bridge->funcs, > bridge->of_node); > + drm_printf(p, "bridge: %s (%pOFfc)\n", bridge->name, > bridge->of_node); > else > - drm_printf(p, "bridge: %ps\n", bridge->funcs); > + drm_printf(p, "bridge: %s\n", bridge->name); [Severity: Low] In drivers/gpu/drm/drm_bridge.c:drm_bridge_atomic_print_priv_state(), for bridges that do not use devm_drm_bridge_alloc() (e.g., legacy or statically allocated bridges), bridge->name is never initialized and remains NULL. When bridge->name is NULL, vsprintf safely intercepts the NULL pointer and prints '(null)'. Does this degrade the debug output compared to the previous behavior (which printed the function pointer symbol for identification), defeating the goal for these bridges? [Severity: High] While looking into the bridge->name changes, I noticed a related issue in drivers/gpu/drm/drm_bridge.c:drm_bridge_attach(), where the return value of drm_atomic_private_obj_init() is ignored. A preceding patch in this series added a dynamically allocated name parameter to drm_atomic_private_obj_init() which fails with -ENOMEM if the allocation fails: drivers/gpu/drm/drm_atomic.c:drm_atomic_private_obj_init() { ... obj->name = kstrdup(name, GFP_KERNEL); if (!obj->name) { ret = -ENOMEM; goto err_lock_fini; } ... } Because bridge->name is NULL for statically allocated bridges, kstrdup(NULL) returns NULL, causing drm_atomic_private_obj_init() to unconditionally fail. Since drm_bridge_attach() ignores the return value: drivers/gpu/drm/drm_bridge.c:drm_bridge_attach() { ... drm_atomic_private_obj_init(bridge->dev, &bridge->base, bridge->name, &drm_bridge_priv_state_funcs); return 0; } The private object is left zeroed and uninitialized. Will this cause a deterministic NULL pointer dereference during teardown when drm_bridge_detach() calls drm_atomic_private_obj_fini(), which executes list_del(&obj->head) on a zeroed list_head? Does it also cause a crash during atomic commits when drm_atomic_helper_swap_state() attempts to dereference old_obj_state->state? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
