github-mirror/linux
1
0
Fork 0
mirror of https://github.com/torvalds/linux.git synced 2026-05-12 16:18:45 +02:00
Code Issues Actions Packages Projects Releases Wiki Activity

drm: Set old handle to NULL before prime swap in change_handle

Browse Source 
There was a potential race condition in change_handle. The ioctl
briefly had a single object with two idr entries; a concurrent
gem_close could delete the object and remove one of the handles
while leaving the other one dangling, which could subsequently
be dereferenced for a use-after-free.

To fix this, do the same dance that gem_close itself does.
(f6cd7daecf drm: Release driver references to handle before making it available again)
First idr_replace the old handle to NULL. Later, if the prime
operations are successful, actually close it.

create_tail required a similar dance to avoid a similar problem.
(bd46cece51 drm/gem: Fix race in drm_gem_handle_create_tail())
It idr_allocs the new handle with NULL, then swaps in the correct
object later to avoid races. We don't need to do that here, since
the only operations that could race are drm_prime, and
change_handle holds the prime lock for the entire duration.

v2: cleanups of error paths

Signed-off-by: David Francis <David.Francis@amd.com>
Co-authored-by: Dave Airlie <airlied@gmail.com>
Reported-by: Puttimet Thammasaeng <pwn8official@gmail.com>
Tested-by: Vitaly Prosyak <Vitaly.Prosyak@amd.com>
Cc: Simona Vetter <simona@ffwll.ch>
Cc: stable@vger.kernel.org
Cc: Christian Koenig <Christian.Koenig@amd.com>
Fixes: 53096728b8 ("drm: Add DRM prime interface to reassign GEM handle")
Signed-off-by: Dave Airlie <airlied@redhat.com>
This commit is contained in:
Francis, David 2026-04-28 19:25:50 +00:00 committed by Dave Airlie
parent d8a70292c3
commit 5e28b7b944
1 changed files with 24 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
drivers/gpu/drm/drm_gem.c
View File

@ -1019,7 +1019,7 @@ int drm_gem_change_handle_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv) struct drm_file *file_priv)
{ {
struct drm_gem_change_handle *args = data; struct drm_gem_change_handle *args = data;
struct drm_gem_object *obj; struct drm_gem_object *obj, *idrobj;
int handle, ret; int handle, ret;
if (!drm_core_check_feature(dev, DRIVER_GEM)) if (!drm_core_check_feature(dev, DRIVER_GEM))
@ -1042,8 +1042,29 @@ int drm_gem_change_handle_ioctl(struct drm_device *dev, void *data,
mutex_lock(&file_priv->prime.lock); mutex_lock(&file_priv->prime.lock);
spin_lock(&file_priv->table_lock); spin_lock(&file_priv->table_lock);
/* When create_tail allocs an obj idr, it needs to first alloc as NULL,
* then later replace with the correct object. This is not necessary
* here, because the only operations that could race are drm_prime
* bookkeeping, and we hold the prime lock.
*/
ret = idr_alloc(&file_priv->object_idr, obj, handle, handle + 1, ret = idr_alloc(&file_priv->object_idr, obj, handle, handle + 1,
GFP_NOWAIT); GFP_NOWAIT);
if (ret < 0) {
spin_unlock(&file_priv->table_lock);
goto out_unlock;
}
idrobj = idr_replace(&file_priv->object_idr, NULL, handle);
if (idrobj != obj) {
idr_replace(&file_priv->object_idr, idrobj, handle);
idr_remove(&file_priv->object_idr, args->new_handle);
spin_unlock(&file_priv->table_lock);
ret = -ENOENT;
goto out_unlock;
}
spin_unlock(&file_priv->table_lock); spin_unlock(&file_priv->table_lock);
if (ret < 0) if (ret < 0)
@ -1055,6 +1076,8 @@ int drm_gem_change_handle_ioctl(struct drm_device *dev, void *data,
if (ret < 0) { if (ret < 0) {
spin_lock(&file_priv->table_lock); spin_lock(&file_priv->table_lock);
idr_remove(&file_priv->object_idr, handle); idr_remove(&file_priv->object_idr, handle);
idrobj = idr_replace(&file_priv->object_idr, obj, handle);
WARN_ON(idrobj != NULL);
spin_unlock(&file_priv->table_lock); spin_unlock(&file_priv->table_lock);
goto out_unlock; goto out_unlock;
} }