Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Memory leak and missing plane state initialization due to ignored return value of atomic_create_state --
--- Patch [39]: [PATCH 39/60] drm/amdgpu_dm: Convert to atomic_create_state --- commit ffb311ea66ae63df87af953af043654eafe9802e Author: Maxime Ripard <[email protected]> drm/amdgpu_dm: Convert to atomic_create_state The plane reset implementation creates a custom state subclass, but only initializes a pristine state without resetting any hardware. This commit converts it to use the atomic_create_state callback. > 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 1b564cfe21202..64647f4a42908 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 > @@ -1515,22 +1515,21 @@ static const struct drm_plane_helper_funcs > dm_primary_plane_helper_funcs = { > .panic_flush = amdgpu_dm_plane_panic_flush, > }; > > -static void amdgpu_dm_plane_drm_plane_reset(struct drm_plane *plane) > +static struct drm_plane_state *amdgpu_dm_plane_drm_plane_create_state(struct > drm_plane *plane) > { > struct dm_plane_state *amdgpu_state; > > amdgpu_state = kzalloc_obj(*amdgpu_state); > if (!amdgpu_state) > - return; > - > - if (plane->state) > - plane->funcs->atomic_destroy_state(plane, plane->state); > + return ERR_PTR(-ENOMEM); > > - __drm_atomic_helper_plane_reset(plane, &amdgpu_state->base); > + __drm_atomic_helper_plane_state_init(&amdgpu_state->base, plane); [ ... ] > + return &amdgpu_state->base; > } [ ... ] > @@ -1994,8 +1993,8 @@ int amdgpu_dm_plane_init(struct amdgpu_display_manager > *dm, > #endif > > /* Create (reset) the plane state */ > - if (plane->funcs->reset) > - plane->funcs->reset(plane); > + if (plane->funcs->atomic_create_state) > + plane->funcs->atomic_create_state(plane); ^^^^ [Severity: High] Does this code leak the newly allocated plane state? The old reset callback assigned the state internally to plane->state, but the new atomic_create_state callback returns the allocated state and relies on the caller to handle it. Because the return value is ignored here, the state pointer appears to be lost and plane->state remains NULL. Later, when the driver tries to duplicate the plane state via amdgpu_dm_plane_drm_plane_duplicate_state(), it might pass the NULL plane->state to to_dm_plane_state() and dereference it while copying driver-specific fields, which could cause a kernel panic. Should we capture the return value and assign it to plane->state, or pass it to the appropriate DRM helper function to properly initialize the state? > return 0; > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=39
