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 <[email protected]> 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/[email protected]?part=8
