Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] Dangling pointer (Use-After-Free) vulnerability in vmwgfx component reset hooks on memory allocation failure. - [High] Missing NULL pointer checks in amdgpu duplicate_state hooks lead to NULL pointer dereference on allocation failure. -- commit 0f8629594bf5eacdad20c483597706d609e02d01 Author: Evgenii Burenchev <[email protected]> drm/amd/display: Fix dangling pointer in plane reset function amdgpu_dm_plane_drm_plane_reset() frees the old state before allocating a new one. If kzalloc_obj() fails, the function returns without updating the state pointer, leaving a dangling pointer to already freed memory. Fix this by allocating the new state first. On allocation failure, the old state remains untouched and the function safely returns. Found by Linux Verification Center (linuxtesting.org) with SVACE. Fixes: 5d945cbcd4b1 ("drm/amd/display: Create a file dedicated to planes") Signed-off-by: Evgenii Burenchev <[email protected]> > 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 e957657b06c78..a698c5835bea6 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 > @@ -1488,17 +1488,15 @@ static const struct drm_plane_helper_funcs > dm_primary_plane_helper_funcs = { > > 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); > + struct dm_plane_state *amdgpu_state; > > amdgpu_state = kzalloc_obj(*amdgpu_state); > - WARN_ON(amdgpu_state == NULL); > - > if (!amdgpu_state) > return; [Severity: High] This is a pre-existing issue, but returning here when plane->state is initially NULL means plane->state remains NULL. Later, when the DRM core attempts to duplicate the state, it calls the driver's duplicate hook. In amdgpu_dm_plane_drm_plane_duplicate_state(), there is no check if plane->state is NULL: amdgpu_dm_plane_drm_plane_duplicate_state() { ... dm_plane_state = kzalloc_obj(*dm_plane_state); if (!dm_plane_state) return NULL; __drm_atomic_helper_plane_duplicate_state(plane, &dm_plane_state->base); ... } Will this cause a NULL pointer dereference in __drm_atomic_helper_plane_duplicate_state() since it will crash when trying to memcpy from a NULL plane->state? The same pattern appears in amdgpu_dm_connector_atomic_duplicate_state(), where to_dm_connector_state(connector->state) will resolve to an invalid pointer offset from NULL, causing kmemdup to panic: amdgpu_dm_connector_atomic_duplicate_state() { ... struct dm_connector_state *state = to_dm_connector_state(connector->state); struct dm_connector_state *new_state = kmemdup(state, sizeof(*state), GFP_KERNEL); ... } Could these duplicate_state hooks be updated to handle a NULL state? [Severity: High] This isn't a bug introduced by this patch, but I noticed the exact same dangling pointer pattern this patch fixes also exists in the vmwgfx driver. In vmw_du_connector_reset(), if kzalloc_obj() fails, it returns without updating the state pointer, leaving a dangling pointer: vmw_du_connector_reset() { ... kfree(vmw_connector_state_to_vcs(connector->state)); } vcs = kzalloc_obj(*vcs); if (!vcs) { DRM_ERROR("Cannot allocate vmw_connector_state\n"); return; ... } The identical issue also occurs in vmw_du_crtc_reset() and vmw_du_plane_reset(). Since unhandled use-after-free bugs in core DRM state structures can cause severe stability issues, do these vmwgfx component reset hooks need a similar fix to allocate the new state before freeing the old one? > + if (plane->state) > + plane->funcs->atomic_destroy_state(plane, plane->state); > + > __drm_atomic_helper_plane_reset(plane, &amdgpu_state->base); [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
