Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] Missing Error Handling / Null Pointer Dereference in `amdgpu_dm_crtc_init`. - [High] Use After Free / List Corruption on plane initialization error paths. -- commit b7f19de1d51c67315c5de464e67807b88c862ea5 Author: Maxime Ripard <[email protected]> drm/amdgpu_dm: Convert to atomic_create_state This commit converts the plane reset implementation to use atomic_create_state. It allocates and initializes a pristine state without resetting hardware, updating the plane functions and state initialization paths to explicitly return errors on allocation failure. > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c > index 824ef3ce5de0a..f9655f534d3b6 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c [ ... ] > @@ -2278,9 +2277,16 @@ int amdgpu_dm_plane_init(struct amdgpu_display_manager > *dm, > return res; > #endif > > - /* Create (reset) the plane state */ > - if (plane->funcs->reset) > - plane->funcs->reset(plane); > + /* Create the plane state */ > + if (plane->funcs->atomic_create_state) { > + struct drm_plane_state *plane_state; > + > + plane_state = plane->funcs->atomic_create_state(plane); > + if (IS_ERR(plane_state)) > + return PTR_ERR(plane_state); [Severity: High] This is a pre-existing issue, but returning this explicit error code exposes an ignored return value in one of the callers. In amdgpu_dm_crtc_init(), the error returned here for the cursor plane is overwritten without being checked: cursor_plane->type = DRM_PLANE_TYPE_CURSOR; res = amdgpu_dm_plane_init(dm, cursor_plane, 0, NULL); acrtc = kzalloc_obj(struct amdgpu_crtc); if (!acrtc) goto fail; res = drm_crtc_init_with_planes( ... ); If amdgpu_dm_plane_init() fails, does the overwritten res variable mean the cursor plane will be left with a NULL state and trigger a kernel panic in the DRM atomic helpers later? [Severity: High] This is also a pre-existing issue, but this new error return path triggers improper cleanup in the callers when atomic_create_state() fails. If amdgpu_dm_plane_init() returns an error here after drm_universal_plane_init() has already registered the plane, the callers handle the failure by simply calling kfree() on the plane: initialize_plane() in amdgpu_dm.c: ret = amdgpu_dm_plane_init(dm, plane, possible_crtcs, plane_cap); if (ret) { drm_err(adev_to_drm(dm->adev), "KMS: Failed to initialize plane\n"); kfree(plane); return ret; } amdgpu_dm_crtc_init() in amdgpu_dm_crtc.c: fail: kfree(acrtc); kfree(cursor_plane); return res; Since the plane is not properly cleaned up with drm_plane_cleanup() first, could this leak a dangling pointer into the mode_config.plane_list and cause list corruption when the DRM plane list is iterated? > + > + plane->state = plane_state; > + } > > return 0; > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=25
