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