diff options
author | Daniel Vetter <daniel.vetter@ffwll.ch> | 2013-08-15 00:02:45 +0200 |
---|---|---|
committer | Dave Airlie <airlied@redhat.com> | 2013-08-21 12:58:17 +1000 |
commit | 20228c447846da9399ead53fdbbc8ab69b47788a (patch) | |
tree | 6fe0e7e3b7f822206dde345848138234c7a19465 /include | |
parent | cd4f013f3a4b6a55d484cc2e206dc08e055e5291 (diff) | |
download | linux-20228c447846da9399ead53fdbbc8ab69b47788a.tar.bz2 |
drm/gem: completely close gem_open vs. gem_close races
The gem flink name holds a reference onto the object itself, and this
self-reference would prevent an flink'ed object from every being
freed. To break that loop we remove the flink name when the last
userspace handle disappears, i.e. when obj->handle_count reaches 0.
Now in gem_open we drop the dev->object_name_lock between the flink
name lookup and actually adding the handle. This means a concurrent
gem_close of the last handle could result in the flink name getting
reaped right inbetween, i.e.
Thread 1 Thread 2
gem_open gem_close
flink -> obj lookup
handle_count drops to 0
remove flink name
create_handle
handle_count++
If someone now flinks this object again, we'll get a new flink name.
We can close this race by removing the lock dropping and making the
entire lookup+handle_create sequence atomic. Unfortunately to still be
able to share the handle_create logic this requires a
handle_create_tail function which drops the lock - we can't hold the
object_name_lock while calling into a driver's ->gem_open callback.
Note that for flink fixing this race isn't really important, since
racing gem_open against gem_close is clearly a userspace bug. And no
matter how the race ends, we won't leak any references.
But with dma-buf where the userspace dma-buf fd itself is refcounted
this is a valid sequence and hence we should fix it. Therefore this
patch here is just a warm-up exercise (and for consistency between
flink buffer sharing and dma-buf buffer sharing with self-imports).
Also note that this extension of the critical section in gem_open
protected by dev->object_name_lock only works because it's now a
mutex: A spinlock would conflict with the potential memory allocation
in idr_preload().
This is exercises by igt/gem_flink_race/flink_name.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Diffstat (limited to 'include')
-rw-r--r-- | include/drm/drmP.h | 3 |
1 files changed, 3 insertions, 0 deletions
diff --git a/include/drm/drmP.h b/include/drm/drmP.h index bf058470a0fd..063eac31b97b 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -1575,6 +1575,9 @@ drm_gem_object_unreference_unlocked(struct drm_gem_object *obj) } } +int drm_gem_handle_create_tail(struct drm_file *file_priv, + struct drm_gem_object *obj, + u32 *handlep); int drm_gem_handle_create(struct drm_file *file_priv, struct drm_gem_object *obj, u32 *handlep); |