[lvc-project] [PATCH v2] drm/amd/display: Fix dangling pointers in state reset functions on allocation failure

[Mario Limonciello]

On 6/26/26 08:38, Evgenii Burenchev wrote:
> Multiple reset functions in amdgpu_dm free 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. If allocation fails, warn and
> return without touching the old state, as it remains valid and the caller
> will handle the error appropriately.
> 
> This affects three functions:
> - amdgpu_dm_plane_drm_plane_reset()
> - amdgpu_dm_crtc_reset_state()
> - amdgpu_dm_connector_funcs_reset()
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Fixes: 5d945cbcd4b1 ("drm/amd/display: Create a file dedicated to planes")
> Fixes: 473683a03495 ("drm/amd/display: Create a file dedicated for CRTC")
> Fixes: e7b07ceef2a6 ("drm/amd/display: Merge amdgpu_dm_types and amdgpu_dm")
> Signed-off-by: Evgenii Burenchev <evg28bur at yandex.ru>
> ---
> Changes in v2:
> - Also fix amdgpu_dm_crtc_reset_state() and amdgpu_dm_connector_funcs_reset()
> ---
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 48 ++++++++++---------
>   .../amd/display/amdgpu_dm/amdgpu_dm_crtc.c    |  8 ++--
>   .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   | 14 +++---
>   3 files changed, 37 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 97ab1e83b318..6ef1b07ec251 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -8151,33 +8151,35 @@ static void amdgpu_dm_connector_destroy(struct drm_connector *connector)
>   
>   void amdgpu_dm_connector_funcs_reset(struct drm_connector *connector)
>   {
> -	struct dm_connector_state *state =
> -		to_dm_connector_state(connector->state);
> -
> -	if (connector->state)
> -		__drm_atomic_helper_connector_destroy_state(connector->state);
> -
> -	kfree(state);

You seem to have lost this kfree() in your refactor.

> +	struct dm_connector_state *state;
>   
> +	/* Allocate new state first */
>   	state = kzalloc_obj(*state);
> +	if (WARN_ON(!state))
> +		return;
>   
> -	if (state) {
> -		state->scaling = RMX_OFF;
> -		state->underscan_enable = false;
> -		state->underscan_hborder = 0;
> -		state->underscan_vborder = 0;
> -		state->base.max_requested_bpc = 8;
> -		state->vcpi_slots = 0;
> -		state->pbn = 0;
> -
> -		if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
> -			if (amdgpu_dm_abm_level <= 0)
> -				state->abm_level = ABM_LEVEL_IMMEDIATE_DISABLE;
> -			else
> -				state->abm_level = amdgpu_dm_abm_level;
> -		}
> +	/* Destroy old state only after successful allocation */
> +	if (connector->state)
> +		__drm_atomic_helper_connector_destroy_state(connector->state);
>   
> -		__drm_atomic_helper_connector_reset(connector, &state->base);
> +	/* Let DRM core install the new state */
> +	__drm_atomic_helper_connector_reset(connector, &state->base);
> +
> +	/* Initialize driver-specific fields */
> +	state->scaling = RMX_OFF;
> +	state->underscan_enable = false;
> +	state->underscan_hborder = 0;
> +	state->underscan_vborder = 0;
> +	state->base.max_requested_bpc = 8;
> +	state->vcpi_slots = 0;
> +	state->pbn = 0;
> +
> +	/* eDP-specific initialization */
> +	if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
> +		if (amdgpu_dm_abm_level <= 0)
> +			state->abm_level = ABM_LEVEL_IMMEDIATE_DISABLE;
> +		else
> +			state->abm_level = amdgpu_dm_abm_level;
>   	}
>   }
>   
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> index 3dcedaa67ed8..6146fbc528c3 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> @@ -437,13 +437,15 @@ static void amdgpu_dm_crtc_reset_state(struct drm_crtc *crtc)
>   {
>   	struct dm_crtc_state *state;
>   
> -	if (crtc->state)
> -		amdgpu_dm_crtc_destroy_state(crtc, crtc->state);
> -
> +	/* Allocate new state first */
>   	state = kzalloc_obj(*state);
>   	if (WARN_ON(!state))
>   		return;
>   
> +	/* Destroy old state only after successful allocation */
> +	if (crtc->state)
> +		amdgpu_dm_crtc_destroy_state(crtc, crtc->state);
> +
>   	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>   }
>   
> 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 e957657b06c7..eb1c0a26f20d 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,17 @@ 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;
>   
> +	/* Allocate new state first */
>   	amdgpu_state = kzalloc_obj(*amdgpu_state);
> -	WARN_ON(amdgpu_state == NULL);
> -
> -	if (!amdgpu_state)
> +	if (WARN_ON(!amdgpu_state))
>   		return;
>   
> +	/* Destroy old state only after successful allocation */
> +	if (plane->state)
> +		plane->funcs->atomic_destroy_state(plane, plane->state);
> +
>   	__drm_atomic_helper_plane_reset(plane, &amdgpu_state->base);
>   	amdgpu_state->degamma_tf = AMDGPU_TRANSFER_FUNCTION_DEFAULT;
>   	amdgpu_state->hdr_mult = AMDGPU_HDR_MULT_DEFAULT;