On Thu, Apr 23, 2015 at 03:07:56PM +0100, Peter Antoine wrote:
If an application that has a driver lock created, wants the lock the kernel context, it is not allowed to. If the call to drm_lock has a context of 0, it is rejected. If you set the context to _DRM_LOCK_CONT then call drm lock, it will pass the context == DRM_KERNEL_CONTEXT checks. But as the DRM_LOCK_CONT bits are not part of the context id this allows operations on the DRM_KERNEL_CONTEXT.
Issue: VIZ-5485 Signed-off-by: Peter Antoine peter.antoine@intel.com
drivers/gpu/drm/drm_context.c | 6 +++--- drivers/gpu/drm/drm_lock.c | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_context.c b/drivers/gpu/drm/drm_context.c index 96350d1..1febcd3 100644 --- a/drivers/gpu/drm/drm_context.c +++ b/drivers/gpu/drm/drm_context.c @@ -123,7 +123,7 @@ void drm_legacy_ctxbitmap_flush(struct drm_device *dev, struct drm_file *file)
list_for_each_entry_safe(pos, tmp, &dev->ctxlist, head) { if (pos->tag == file &&
pos->handle != DRM_KERNEL_CONTEXT) {
_DRM_LOCKING_CONTEXT(pos->handle) != DRM_KERNEL_CONTEXT) { if (dev->driver->context_dtor) dev->driver->context_dtor(dev, pos->handle);@@ -342,7 +342,7 @@ int drm_legacy_addctx(struct drm_device *dev, void *data, struct drm_ctx *ctx = data;
ctx->handle = drm_legacy_ctxbitmap_next(dev);
- if (ctx->handle == DRM_KERNEL_CONTEXT) {
- if (_DRM_LOCKING_CONTEXT(ctx->handle) == DRM_KERNEL_CONTEXT) { /* Skip kernel's context and get a new one. */ ctx->handle = drm_legacy_ctxbitmap_next(dev); }
@@ -449,7 +449,7 @@ int drm_legacy_rmctx(struct drm_device *dev, void *data, struct drm_ctx *ctx = data;
DRM_DEBUG("%d\n", ctx->handle);
- if (ctx->handle != DRM_KERNEL_CONTEXT) {
- if (_DRM_LOCKING_CONTEXT(ctx->handle) != DRM_KERNEL_CONTEXT) { if (dev->driver->context_dtor) dev->driver->context_dtor(dev, ctx->handle); drm_legacy_ctxbitmap_free(dev, ctx->handle);
How about just fixing the end parameter passed to idr_alloc()? AFAICS that would take care of the context code.
Well, there are a few more issues with the code: - not properly checking for negative return value from idr_alloc() - leaking the ctx id on kmalloc() error - pointless check for idr_alloc() returning 0 even though the min is 1
diff --git a/drivers/gpu/drm/drm_lock.c b/drivers/gpu/drm/drm_lock.c index 070dd5d..94500930 100644 --- a/drivers/gpu/drm/drm_lock.c +++ b/drivers/gpu/drm/drm_lock.c @@ -63,7 +63,7 @@ int drm_legacy_lock(struct drm_device *dev, void *data,
++file_priv->lock_count;
While you're poking around this dungeopn, maybe you can kill lock_count? We never seem to decrement it, and it's only checked in drm_legacy_i_have_hw_lock().
- if (lock->context == DRM_KERNEL_CONTEXT) {
- if (_DRM_LOCKING_CONTEXT(lock->context) == DRM_KERNEL_CONTEXT) { DRM_ERROR("Process %d using kernel context %d\n", task_pid_nr(current), lock->context); return -EINVAL;
@@ -153,7 +153,7 @@ int drm_legacy_unlock(struct drm_device *dev, void *data, struct drm_file *file_ struct drm_lock *lock = data; struct drm_master *master = file_priv->master;
- if (lock->context == DRM_KERNEL_CONTEXT) {
- if (_DRM_LOCKING_CONTEXT(lock->context) == DRM_KERNEL_CONTEXT) { DRM_ERROR("Process %d using kernel context %d\n", task_pid_nr(current), lock->context); return -EINVAL;
These two changes look OK to me.
-- 1.9.1
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx