Re: [PATCH v5 12/14] drm/atomic-uapi: Add DRM_MODE_ATOMIC_RESET flag

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm: DRM_MODE_ATOMIC_RESET bypasses DRM lease isolation
- [High] drm/amdgpu: UAF via active plane state destruction during DRM_MODE_ATOMIC_RESET
--

commit db28ec4a7c576dd0db5528bab59644c439286807
Author: Sebastian Wick <sebastian.wick@redhat.com>

drm/atomic-uapi: Add DRM_MODE_ATOMIC_RESET flag

Userspace currently has no atomic way to reset all KMS object states
to their defaults. To bring a display pipeline to a known state, a
compositor must explicitly set every property on every object, which
requires tracking which properties exist and what their defaults are.

> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index 1eebcf6f5e05b..237c7ff823523 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
[ ... ]
> @@ -1688,6 +1695,17 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
>  	fence_state = NULL;
>  	num_fences = 0;
>  
> +	if (arg->flags & DRM_MODE_ATOMIC_RESET) {
> +		if (!drm_atomic_implements_create_state(dev)) {
> +			ret = -EOPNOTSUPP;
> +			goto out;
> +		}
> +
> +		ret = drm_atomic_commit_fill_with_defaults(state);

[Severity: High]
Does this new reset path allow a client with a DRM lease to bypass lease
isolation?

When a leased DRM client calls this IOCTL with DRM_MODE_ATOMIC_RESET,
drm_atomic_commit_fill_with_defaults() blindly adds every device object
to the state with its default values without validating ownership via
drm_lease_held().

This could allow an unprivileged containerized app or VR compositor to
globally disable all displays and objects on the device, including those
owned by the host compositor.

[Severity: High]
Does this expose a Use-After-Free in the AMDGPU driver's plane state
creation?

This function iterates over all planes and calls the driver's
->atomic_create_state() hook to allocate default states.

In AMDGPU, it unconditionally destroys the currently bound state if it
exists:

amdgpu_dm_plane.c:amdgpu_dm_plane_drm_plane_create_state() {
	...
	if (plane->state)
		plane->funcs->atomic_destroy_state(plane, plane->state);
	...
}

Because the state is still actively used by the DRM framework and display
hardware until the atomic commit is completely swapped (or if the commit is
rejected entirely), destroying it here leaves a dangling pointer.

Since this reset path is now user-reachable via the new DRM_MODE_ATOMIC_RESET
ioctl flag, could userspace trigger a kernel panic or data corruption?

> +		if (ret)
> +			goto out;
> +	}
> +
>  	for (i = 0; i < arg->count_objs; i++) {
>  		uint32_t obj_id, count_props;
>  		struct drm_mode_object *obj;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016-drm-reset-state-flag-v5-0-dad2d64b693b@kernel.org?part=12