Make the following changes to CHANGE_HANDLE - reject new_handle = old_handle with -EINVAL - idr_preload before idr_alloc - reject new_handle = 0 with -EINVAL and re-enable it.
These changes are prerequisities for the enabling of this ioctl, which was disabled over security concerns and the lack of IGT tests. For a summary of the saga so far, see https://lists.freedesktop.org/archives/dri-devel/2026-June/571890.html I hope these changes leave the ioctl in a spot where it is safe to re-enable. There are now igt tests covering all corner case behaviour and race conditions between change_handle and itself, create, free, import, and export. The new_handle = 0 and new_handle = old_handle corner cases are now -EINVAL as recommended; these are covered in the igt tests. idr_preload is now correctly called before idr_alloc. With these changes the TODO can be removed and the ioctl can be re-enabled. Accompanying IGT patch on igt-dev is tests/intel: Add edge case and race condition tests for GEM_CHANGE_HANDLE v2: new_handle = old_handle is EINVAL v3: More detailed commit message and some cleanup cc: Tvrtko Ursulin <[email protected]> cc: Christian Koenig <[email protected]> cc: Simona Vetter <[email protected]> Signed-off-by: David Francis <[email protected]> --- drivers/gpu/drm/drm_gem.c | 30 ++++++++++-------------------- drivers/gpu/drm/drm_ioctl.c | 3 +-- 2 files changed, 11 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 6a86bd2a0343e..bb55715e8d170 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -1018,19 +1018,6 @@ drm_gem_open_ioctl(struct drm_device *dev, void *data, return ret; } -/* - * This ioctl is disabled for security reasons but also it failed - * to follow process in terms of adding testing in igt and verifying - * all the corner cases which made fixing security bugs in it even - * harder than necessary. - * - * To re-enable this ioctl - * 1. land working IGT tests in igt-gpu-tools that cover - * all corner cases and race conditions. - * 2. handle idr_preload - * 3. handle == 0 - * 4. handle == new_handle semantics definition. - */ int drm_gem_change_handle_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv) { @@ -1041,21 +1028,22 @@ int drm_gem_change_handle_ioctl(struct drm_device *dev, void *data, if (!drm_core_check_feature(dev, DRIVER_GEM)) return -EOPNOTSUPP; - /* idr_alloc() limitation. */ - if (args->new_handle > INT_MAX) - return -EINVAL; new_handle = args->new_handle; - if (args->handle == new_handle) - return 0; + /* idr_alloc() limitations. */ + if (new_handle == 0 || new_handle > INT_MAX || + args->handle == new_handle) + return -EINVAL; mutex_lock(&file_priv->prime.lock); + idr_preload(GFP_KERNEL); spin_lock(&file_priv->table_lock); - ret = idr_alloc(&file_priv->object_idr, NULL, new_handle, new_handle + 1, - GFP_NOWAIT); + ret = idr_alloc(&file_priv->object_idr, NULL, new_handle, + new_handle + 1, GFP_NOWAIT); if (ret < 0) { spin_unlock(&file_priv->table_lock); + idr_preload_end(); goto out_unlock; } @@ -1063,10 +1051,12 @@ int drm_gem_change_handle_ioctl(struct drm_device *dev, void *data, if (IS_ERR_OR_NULL(obj)) { idr_remove(&file_priv->object_idr, new_handle); spin_unlock(&file_priv->table_lock); + idr_preload_end(); ret = -ENOENT; goto out_unlock; } spin_unlock(&file_priv->table_lock); + idr_preload_end(); if (obj->dma_buf) { ret = drm_prime_add_buf_handle(&file_priv->prime, obj->dma_buf, diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index e2df4becce629..ff193155129e7 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -660,8 +660,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_IOCTL_DEF(DRM_IOCTL_GEM_CLOSE, drm_gem_close_ioctl, DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_GEM_FLINK, drm_gem_flink_ioctl, DRM_AUTH), DRM_IOCTL_DEF(DRM_IOCTL_GEM_OPEN, drm_gem_open_ioctl, DRM_AUTH), - /* see drm_gem.c:drm_gem_change_handle_ioctl for why this is invalid */ - DRM_IOCTL_DEF(DRM_IOCTL_GEM_CHANGE_HANDLE, drm_invalid_op, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF(DRM_IOCTL_GEM_CHANGE_HANDLE, drm_gem_change_handle_ioctl, DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETRESOURCES, drm_mode_getresources, 0), -- 2.34.1
