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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The old connector state memory is leaked during a successful connector reset in `amdgpu_dm_connector_funcs_reset()`.
--

commit 48ece48fead57764b257711d256ed0e86eefc3c4
Author: Evgenii Burenchev <evg28bur@yandex.ru>
Subject: drm/amd/display: Fix dangling pointers in state reset functions on allocation failure

This patch addresses dangling pointers in AMD display state reset functions by
allocating new state structures before destroying the old ones.

> 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 eb5696b5daeb7..c7fe28758e962 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);
> +	struct dm_connector_state *state;
>  
> +	/* Allocate new state first */
>  	state = kzalloc_obj(*state);
> +	if (WARN_ON(!state))
> +		return;
>  
> -	if (state) {
> -		state->scaling = RMX_OFF;

[ ... ]

> +	/* 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);

[Severity: Medium]
Does this code leak the old connector state memory?

The original code explicitly called kfree(state) after calling
__drm_atomic_helper_connector_destroy_state(connector->state). The new
implementation removes this kfree() call.

Looking at __drm_atomic_helper_connector_destroy_state(), it only cleans up
references to nested DRM objects (like CRTCs or properties) but does not free
the connector_state structure itself. When __drm_atomic_helper_connector_reset()
then overwrites connector->state with the new pointer, it appears to
permanently orphan the old memory.

Should the old state memory be explicitly freed here as it was previously?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260626133859.21492-1-evg28bur@yandex.ru?part=1