Re: [PATCH] drm/gem: Handle shadow plane state allocation failure
| Newsgroups | org.freedesktop.lists.dri-devel |
|---|---|
| Message-ID | <[email protected]> |
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Returning early on allocation failure in `drm_gem_reset_shadow_plane` leaves the old plane state intact, violating DRM atomic reset semantics and causing state desynchronization. -- --- Patch [1]: [PATCH] drm/gem: Handle shadow plane state allocation failure --- commit 0d95f6bd3cf4e9b3b32636265a7263dcbf12db13 Author: Triet Hoang <[email protected]> drm/gem: Handle shadow plane state allocation failure Allocate the new shadow plane state before destroying the existing state, so that an allocation failure leaves the current state intact. > diff --git a/drivers/gpu/drm/drm_gem_atomic_helper.c b/drivers/gpu/drm/drm_gem_atomic_helper.c > --- a/drivers/gpu/drm/drm_gem_atomic_helper.c > +++ b/drivers/gpu/drm/drm_gem_atomic_helper.c > @@ -308,14 +308,16 @@ EXPORT_SYMBOL(__drm_gem_reset_shadow_plane); > */ > void drm_gem_reset_shadow_plane(struct drm_plane *plane) > { > - struct drm_shadow_plane_state *shadow_plane_state; > + struct drm_shadow_plane_state *shadow_plane_state = kzalloc_obj(*shadow_plane_state); > + > + if (!shadow_plane_state) > + return; [Severity: High] Does returning early here violate DRM atomic reset semantics? The .reset hook must completely clear the software state to mirror the hardware reset (which defaults to disabled). By returning early on allocation failure, the old plane->state is left intact. Could this cause the plane's software state to become desynchronized from both the hardware and other DRM objects (like CRTCs, which will have their state cleared)? Subsequent atomic commits might then operate on this broken software state (e.g., a plane pointing to a disabled CRTC), causing validation failures or crashes. > > if (plane->state) { > drm_gem_destroy_shadow_plane_state(plane, plane->state); > plane->state = NULL; /* must be set to NULL here */ > } -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1