From: Dave Airlie airlied@redhat.com
Lookup the context first to see if its valid before trying to remove it.
Saw this WARNING a few times while fuzzing the kernel with Trinity in a qemu virtual machine:
[ 22.883257] idr_remove called for id=4096 which is not allocated. [ 22.884487] Pid: 2303, comm: trinity-child1 Not tainted 3.8.0+ #87 [ 22.885601] Call Trace: [ 22.886080] [<ffffffff8134b111>] idr_remove+0x131/0x1f0 [ 22.887107] [<ffffffff8143c678>] drm_ctxbitmap_free+0x38/0x50 [ 22.888158] [<ffffffff8143cbd3>] drm_rmctx+0x63/0x100 [ 22.889091] [<ffffffff8143d710>] drm_ioctl+0x3d0/0x4d0 [ 22.890034] [<ffffffff8143cb70>] ? drm_newctx+0xb0/0xb0 [ 22.890970] [<ffffffff812fb640>] ? avc_has_perm_flags+0x1d0/0x2a0 [ 22.892127] [<ffffffff812fb498>] ? avc_has_perm_flags+0x28/0x2a0 [ 22.893218] [<ffffffff810f5b18>] ? trace_hardirqs_off_caller+0x28/0xd0 [ 22.894401] [<ffffffff810f5bcd>] ? trace_hardirqs_off+0xd/0x10 [ 22.895461] [<ffffffff811b5ff2>] do_vfs_ioctl+0x532/0x580 [ 22.896447] [<ffffffff812fc7d3>] ? file_has_perm+0x83/0xa0 [ 22.897453] [<ffffffff811b609d>] sys_ioctl+0x5d/0xa0 [ 22.898429] [<ffffffff813571de>] ? trace_hardirqs_on_thunk+0x3a/0x3f [ 22.899629] [<ffffffff81ca07e9>] system_call_fastpath+0x16/0x1b
Reported-by: Tommi Rantala tt.rantala@gmail.com Signed-off-by: Dave Airlie airlied@redhat.com --- drivers/gpu/drm/drm_context.c | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/drm_context.c b/drivers/gpu/drm/drm_context.c index 45adf97..a186563 100644 --- a/drivers/gpu/drm/drm_context.c +++ b/drivers/gpu/drm/drm_context.c @@ -438,6 +438,13 @@ int drm_rmctx(struct drm_device *dev, void *data,
DRM_DEBUG("%d\n", ctx->handle); if (ctx->handle != DRM_KERNEL_CONTEXT) { + struct drm_local_map *map; + mutex_lock(&dev->struct_mutex); + map = idr_find(&dev->ctx_idr, ctx->handle); + mutex_unlock(&dev->struct_mutex); + + if (!map) + return -EINVAL; if (dev->driver->context_dtor) dev->driver->context_dtor(dev, ctx->handle); drm_ctxbitmap_free(dev, ctx->handle);
From: Dave Airlie airlied@redhat.com
if we don't have a lock, detect it early and avoid oopsing, this should fix some of the NULL pointer derefs under fuzzing.
Reported-by: Tommi Rantala tt.rantala@gmail.com Signed-off-by: Dave Airlie airlied@redhat.com --- drivers/gpu/drm/drm_context.c | 5 +++++ drivers/gpu/drm/drm_lock.c | 3 +++ 2 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/drm_context.c b/drivers/gpu/drm/drm_context.c index a186563..7db0fb0 100644 --- a/drivers/gpu/drm/drm_context.c +++ b/drivers/gpu/drm/drm_context.c @@ -394,6 +394,9 @@ int drm_switchctx(struct drm_device *dev, void *data, { struct drm_ctx *ctx = data;
+ if (!file_priv->master->lock.hw_lock) + return -EINVAL; + DRM_DEBUG("%d\n", ctx->handle); return drm_context_switch(dev, dev->last_context, ctx->handle); } @@ -414,6 +417,8 @@ int drm_newctx(struct drm_device *dev, void *data, { struct drm_ctx *ctx = data;
+ if (!file_priv->master->lock.hw_lock) + return -EINVAL; DRM_DEBUG("%d\n", ctx->handle); drm_context_switch_complete(dev, file_priv, ctx->handle);
diff --git a/drivers/gpu/drm/drm_lock.c b/drivers/gpu/drm/drm_lock.c index d752c96..e177abe 100644 --- a/drivers/gpu/drm/drm_lock.c +++ b/drivers/gpu/drm/drm_lock.c @@ -157,6 +157,9 @@ int drm_unlock(struct drm_device *dev, void *data, struct drm_file *file_priv) return -EINVAL; }
+ if (!master->lock.hw_lock) + return -EINVAL; + atomic_inc(&dev->counts[_DRM_STAT_UNLOCKS]);
if (drm_lock_free(&master->lock, lock->context)) {
2013/2/20 Dave Airlie airlied@gmail.com:
From: Dave Airlie airlied@redhat.com
if we don't have a lock, detect it early and avoid oopsing, this should fix some of the NULL pointer derefs under fuzzing.
With these two patches applied, I can still see:
[ 66.166645] BUG: unable to handle kernel NULL pointer dereference at (null) [ 66.167069] IP: [<ffffffff8144226f>] drm_lock+0xaf/0x330 [ 66.167069] PGD 395ff067 PUD 39600067 PMD 0 [ 66.167069] Oops: 0000 [#1] SMP [ 66.167069] CPU 0 [ 66.167069] Pid: 2290, comm: trinity-child3 Not tainted 3.8.0+ #88 Bochs Bochs [ 66.167069] RIP: 0010:[<ffffffff8144226f>] [<ffffffff8144226f>] drm_lock+0xaf/0x330 [ 66.167069] RSP: 0018:ffff88003960dce8 EFLAGS: 00010202 [ 66.167069] RAX: 0000000000000000 RBX: ffff880039e12290 RCX: ffffffff820e8148 [ 66.167069] RDX: 0000000000003800 RSI: ffffffff8214f674 RDI: 0000000000000001 [ 66.167069] RBP: ffff88003960dd78 R08: 0000000000000080 R09: 00000000000008f2 [ 66.167069] R10: 0000000000000000 R11: 0000000000000001 R12: ffff88003a960a00 [ 66.167069] R13: ffff88003960ddd8 R14: ffff88003a960a58 R15: ffff88003b3ac400 [ 66.167069] FS: 00007f0868664700(0000) GS:ffff88003fc00000(0000) knlGS:0000000000000000 [ 66.167069] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 66.167069] CR2: 0000000000000000 CR3: 00000000395fd000 CR4: 00000000000006f0 [ 66.167069] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 66.167069] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 [ 66.167069] Process trinity-child3 (pid: 2290, threadinfo ffff88003960c000, task ffff880039e12290) [ 66.167069] Stack: [ 66.167069] ffffffff8143d7ad ffffffff8117a1f0 0000000000000246 0000000000000000 [ 66.167069] ffff88003ca08000 ffffffff8143d7ad ffffffff82274918 0000000000000000 [ 66.167069] ffff880039e12290 ffffffff810d2b60 0000000000000000 0000000000000000 [ 66.167069] Call Trace: [ 66.167069] [<ffffffff8143d7ad>] ? drm_ioctl+0x3bd/0x4d0 [ 66.167069] [<ffffffff8117a1f0>] ? might_fault+0x40/0x90 [ 66.167069] [<ffffffff8143d7ad>] ? drm_ioctl+0x3bd/0x4d0 [ 66.167069] [<ffffffff810d2b60>] ? try_to_wake_up+0x340/0x340 [ 66.167069] [<ffffffff8143d7c0>] drm_ioctl+0x3d0/0x4d0 [ 66.167069] [<ffffffff814421c0>] ? drm_lock_take+0x100/0x100 [ 66.167069] [<ffffffff812fb640>] ? avc_has_perm_flags+0x1d0/0x2a0 [ 66.167069] [<ffffffff812fb498>] ? avc_has_perm_flags+0x28/0x2a0 [ 66.167069] [<ffffffff810f5b18>] ? trace_hardirqs_off_caller+0x28/0xd0 [ 66.167069] [<ffffffff810f5bcd>] ? trace_hardirqs_off+0xd/0x10 [ 66.167069] [<ffffffff811b5ff2>] do_vfs_ioctl+0x532/0x580 [ 66.167069] [<ffffffff812fc7d3>] ? file_has_perm+0x83/0xa0 [ 66.167069] [<ffffffff811b609d>] sys_ioctl+0x5d/0xa0 [ 66.167069] [<ffffffff813571de>] ? trace_hardirqs_on_thunk+0x3a/0x3f [ 66.167069] [<ffffffff81ca08a9>] system_call_fastpath+0x16/0x1b [ 66.167069] Code: 1f 44 00 00 49 8b 44 24 58 8b 56 04 48 c7 c1 48 81 0e 82 44 8b 8b 94 02 00 00 48 c7 c6 74 f6 14 82 bf 01 00 00 00 4d 8d 74 24 58 <8b> 00 89 54 24 08 48 c7 c2 f9 2c e7 81 89 04 24 31 c0 e8 ea 0c [ 66.167069] RIP [<ffffffff8144226f>] drm_lock+0xaf/0x330 [ 66.167069] RSP <ffff88003960dce8> [ 66.167069] CR2: 0000000000000000 [ 66.216777] ---[ end trace 148abe85b0cacc0f ]---
Reported-by: Tommi Rantala tt.rantala@gmail.com Signed-off-by: Dave Airlie airlied@redhat.com
drivers/gpu/drm/drm_context.c | 5 +++++ drivers/gpu/drm/drm_lock.c | 3 +++ 2 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/drm_context.c b/drivers/gpu/drm/drm_context.c index a186563..7db0fb0 100644 --- a/drivers/gpu/drm/drm_context.c +++ b/drivers/gpu/drm/drm_context.c @@ -394,6 +394,9 @@ int drm_switchctx(struct drm_device *dev, void *data, { struct drm_ctx *ctx = data;
if (!file_priv->master->lock.hw_lock)return -EINVAL;DRM_DEBUG("%d\n", ctx->handle); return drm_context_switch(dev, dev->last_context, ctx->handle);} @@ -414,6 +417,8 @@ int drm_newctx(struct drm_device *dev, void *data, { struct drm_ctx *ctx = data;
if (!file_priv->master->lock.hw_lock)return -EINVAL; DRM_DEBUG("%d\n", ctx->handle); drm_context_switch_complete(dev, file_priv, ctx->handle);diff --git a/drivers/gpu/drm/drm_lock.c b/drivers/gpu/drm/drm_lock.c index d752c96..e177abe 100644 --- a/drivers/gpu/drm/drm_lock.c +++ b/drivers/gpu/drm/drm_lock.c @@ -157,6 +157,9 @@ int drm_unlock(struct drm_device *dev, void *data, struct drm_file *file_priv) return -EINVAL; }
if (!master->lock.hw_lock)return -EINVAL;atomic_inc(&dev->counts[_DRM_STAT_UNLOCKS]); if (drm_lock_free(&master->lock, lock->context)) {-- 1.7.1
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel@lists.freedesktop.org
-
Dave Airlie -
Tommi Rantala