Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] drm: double list_del() on private obj init failure --
commit 4bcc9dc639c9860732e1906f0390e6fdeeb98b4e Author: Maxime Ripard <[email protected]> drm/atomic: Fix private obj error handling drm_atomic_private_obj_init() adds the object to the privobj_list before calling atomic_create_state(). If atomic_create_state() fails, this patch adds an error path that removes the object from the list to avoid a dangling entry. > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index 9bb761eda59f4..9973becb75aee 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -1029,6 +1029,8 @@ int drm_atomic_private_obj_init(struct drm_device *dev, > const struct drm_private_state_funcs *funcs) > { > struct drm_private_state *state; > + int ret; > + > memset(obj, 0, sizeof(*obj)); > > drm_modeset_lock_init(&obj->lock); > @@ -1038,12 +1040,19 @@ int drm_atomic_private_obj_init(struct drm_device > *dev, > 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); > + if (IS_ERR(state)) { > + ret = PTR_ERR(state); > + goto err_list_del; > + } > > obj->state = state; > > return 0; > + > +err_list_del: > + list_del(&obj->head); [Severity: High] Does this error path introduce a double list_del() panic during driver teardown? Several callers of drm_atomic_private_obj_init(), such as vc4_ctm_obj_init() and drm_dp_mst_topology_mgr_init(), ignore its return value. They unconditionally register their teardown functions, which perform their own list_del(&obj->head) via drm_atomic_private_obj_fini(). For instance, in drivers/gpu/drm/vc4/vc4_kms.c:vc4_ctm_obj_init(): drm_atomic_private_obj_init(&vc4->base, &vc4->ctm_manager, &vc4_ctm_state_funcs); return drmm_add_action_or_reset(&vc4->base, vc4_ctm_obj_fini, NULL); If atomic_create_state() fails here, we delete the list head and return an error, but the caller ignores it. Then, at teardown, drm_atomic_private_obj_fini() executes a second list_del(&obj->head) on the poisoned list head, causing a panic. Will this unwinding cause crashes for these existing callers if initialization fails? > + drm_modeset_lock_fini(&obj->lock); > + return ret; > } > EXPORT_SYMBOL(drm_atomic_private_obj_init); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
