summaryrefslogtreecommitdiff
path: root/include/drm
diff options
context:
space:
mode:
authorDaniel Vetter <daniel.vetter@ffwll.ch>2013-08-15 00:02:45 +0200
committerDave Airlie <airlied@redhat.com>2013-08-21 12:58:17 +1000
commit20228c447846da9399ead53fdbbc8ab69b47788a (patch)
tree6fe0e7e3b7f822206dde345848138234c7a19465 /include/drm
parentcd4f013f3a4b6a55d484cc2e206dc08e055e5291 (diff)
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/drm')
-rw-r--r--include/drm/drmP.h3
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);