On Thu, Feb 15, 2018 at 03:19:42PM +0100, Christian KÃnig wrote:
amdgpu needs to verify if userspace sends us valid addresses and the simplestJust comparing the context should be good enough. If you ever pass a
way of doing this is to check if the buffer object is locked with the ticket
of the current submission.
Clean up the access to the ww_mutex internals by providing a function
for this and extend the check to the thread owning the underlying mutex.
Signed-off-by: Christian KÃnig <christian.koenig@xxxxxxx>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 3 ++-
include/linux/ww_mutex.h | 17 +++++++++++++++++
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index eaa3cb0c3ad1..4c04b560e358 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -1594,7 +1594,8 @@ int amdgpu_cs_find_mapping(struct amdgpu_cs_parser *parser,
*map = mapping;
/* Double check that the BO is reserved by this CS */
- if (READ_ONCE((*bo)->tbo.resv->lock.ctx) != &parser->ticket)
+ if (!ww_mutex_is_owned_by(&(*bo)->tbo.resv->lock, current,
+ &parser->ticket))
return -EINVAL;
if (!((*bo)->flags & AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS)) {
diff --git a/include/linux/ww_mutex.h b/include/linux/ww_mutex.h
index 39fda195bf78..dd580db289e8 100644
--- a/include/linux/ww_mutex.h
+++ b/include/linux/ww_mutex.h
@@ -358,4 +358,21 @@ static inline bool ww_mutex_is_locked(struct ww_mutex *lock)
return mutex_is_locked(&lock->base);
}
+/**
+ * ww_mutex_is_owned_by - is the w/w mutex locked by this task in that context
+ * @lock: the mutex to be queried
+ * @task: the task structure to check
+ * @ctx: the w/w acquire context to test
+ *
+ * Returns true if the mutex is locked in the context by the given task, false
+ * otherwise.
+ */
+static inline bool ww_mutex_is_owned_by(struct ww_mutex *lock,
+ struct task_struct *task,
+ struct ww_acquire_ctx *ctx)
+{
+ return likely(__mutex_owner(&lock->base) == task) &&
+ READ_ONCE(lock->ctx) == ctx;
ww_acquire_ctx which does not belong to your own thread your seriously
wreaking things much worse already (and if we do catch that, should
probably lock the ctx to a given task when ww-mutex debugging is enabled).
That also simplifies the function signature.
Of course that means if you don't have a ctx, you can't test ownership of
a ww_mute, but I think that's not a really valid use-case.
And not needed
for cmd submission, where you need the ctx anyway.
Besides this interface nit looks all good. With the task check¶meter
removed:
Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
-Daniel
+}
+
#endif
--
2.14.1
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel