Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Missing drm_plane_cleanup() on the error path in amdgpu_dm_plane_init() causes a Use-After-Free of the plane object when the caller frees the plane. -- --- Patch [40]: [PATCH v2 40/61] drm/amdgpu_dm: Convert to atomic_create_state --- commit 6612d2468dd0e4946eb21b27b9d60498ca111985 Author: Maxime Ripard <[email protected]> drm/amdgpu_dm: Convert to atomic_create_state This commit converts the plane reset implementation to use the atomic_create_state hook. A custom state subclass is initialized without resetting any hardware. The conversion was performed via a Coccinelle semantic patch. > 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 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c [ ... ] > @@ -1993,9 +1992,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 I wanted to ask about this error path. If plane->funcs->atomic_create_state() fails and returns an error pointer, we return the error directly without calling drm_plane_cleanup(). Since drm_universal_plane_init() was called earlier in this function, the plane is already linked into the mode_config.plane_list. If we return an error here, the caller initialize_plane() will free the plane memory: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c:initialize_plane() { ... if (ret) { drm_err(adev_to_drm(dm->adev), "KMS: Failed to initialize plane\n"); kfree(plane); return ret; } ... } Does this leave a freed pointer on the plane list that could cause a use-after-free later during device teardown in drm_mode_config_cleanup()? While this problem already existed for dm_plane_init_colorops() failures, it appears this patch might expose it to a new allocation failure path. > + > + plane->state = plane_state; > + } > > return 0; > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=40
