Re: [PATCH RFC 8/8] 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:
- [Critical] The newly added DRM_MODE_ATOMIC_RESET flag allows userspace to crash the kernel by unconditionally dereferencing an unimplemented `atomic_create_state` hook on CRTCs, planes, and connectors.
- [High] The DRM_MODE_ATOMIC_RESET flag bypasses DRM lease boundaries, allowing a restricted client to reset all displays on the device.
--

--- Patch [8]: [PATCH RFC 8/8] drm/atomic-uapi: Add DRM_MODE_ATOMIC_RESET flag ---
commit f7802a930a44f2b2c525a72c03344afc1a5cba57
Author: Sebastian Wick <sebastian.wick@redhat.com>
Subject: drm/atomic-uapi: Add DRM_MODE_ATOMIC_RESET flag

Introduce DRM_MODE_ATOMIC_RESET (0x0800) which, when passed to the
atomic ioctl, fills the commit with default states for all KMS objects
before applying the properties supplied in the request. The flag cannot
be combined with DRM_MODE_PAGE_FLIP_ASYNC.

> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index 1050dddadb17e..cdcc6efb9a00f 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
[ ... ]
> @@ -1675,6 +1682,12 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
>  	fence_state = NULL;
>  	num_fences = 0;
>  
> +	if (arg->flags & DRM_MODE_ATOMIC_RESET) {
> +		ret = drm_atomic_commit_fill_with_defaults(state);

[Severity: Critical]
Can this cause a NULL pointer dereference for drivers that don't implement the
atomic_create_state hook?

When userspace passes DRM_MODE_ATOMIC_RESET, this calls
drm_atomic_commit_fill_with_defaults(), which unconditionally calls the
atomic_create_state hook for CRTCs, planes, and connectors:

drivers/gpu/drm/drm_atomic.c:drm_atomic_commit_fill_with_defaults() {
	...
	drm_for_each_crtc(crtc, dev) {
		...
		crtc_state = crtc->funcs->atomic_create_state(crtc);
		...
	}
	...
}

If a driver doesn't implement this optional hook, will this dereference a NULL
pointer?


[Severity: High]
Does this bypass DRM lease boundaries?

When called here, drm_atomic_commit_fill_with_defaults() iterates globally over
all CRTCs, planes, and connectors on the device without checking file_priv or
enforcing lease boundaries.

Could a restricted client use this flag to break lease isolation and forcibly
reset or turn off all displays on the system?

> +		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/20260708-drm-reset-state-flag-v1-0-c37dc985485d@kernel.org?part=8