Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Dangling pointers in `amdgpu_dm_crtc_reset_state` and `amdgpu_dm_connector_funcs_reset` upon memory allocation failure. -- commit a2f21c8f5312ce74d2f0b34c962db8d83e4d242f Author: Evgenii Burenchev <[email protected]> drm/amd/display: Fix dangling pointer in plane state reset on allocation failure In amdgpu_dm_plane_drm_plane_reset(), the old plane state is freed before allocating a new one. If kzalloc_obj() fails, the function returns without updating plane->state, leaving a dangling pointer to already freed memory. > 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 c7f8e08feaf4b..65c0aeeba090c 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 > @@ -1490,20 +1490,30 @@ static void amdgpu_dm_plane_drm_plane_reset(struct > drm_plane *plane) > { > struct dm_plane_state *amdgpu_state = NULL; > > - if (plane->state) > - plane->funcs->atomic_destroy_state(plane, plane->state); > - > amdgpu_state = kzalloc_obj(*amdgpu_state); > - WARN_ON(amdgpu_state == NULL); [Severity: High] This isn't a bug introduced by this patch, but does the same dangling pointer issue exist in other reset functions within amdgpu_dm? For example, in drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c within amdgpu_dm_crtc_reset_state(): if (crtc->state) amdgpu_dm_crtc_destroy_state(crtc, crtc->state); state = kzalloc_obj(*state); if (WARN_ON(!state)) return; If kzalloc_obj() fails, this returns early without setting crtc->state to NULL, leaving a dangling pointer to the state freed by amdgpu_dm_crtc_destroy_state(). Similarly, in drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c within amdgpu_dm_connector_funcs_reset(): kfree(state); state = kzalloc_obj(*state); if (state) { // ... __drm_atomic_helper_connector_reset(connector, &state->base); } If the allocation fails here, connector->state is never cleared and remains dangling. Could these locations also be updated to prevent a potential use-after-free under memory pressure? > > if (!amdgpu_state) > - return; > + goto err_alloc; > + > + /* Old state can now be safely destroyed. The new state is already > allocated and will be assigned */ [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
