Re: [PATCH v4 1/3] drm/amd/display: Fix dangling pointer in plane reset function

[sashiko-bot@kernel.org]

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 <evg28bur@yandex.ru>

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 <evg28bur@yandex.ru>

> 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/20260629090435.9729-2-evg28bur@yandex.ru?part=1