diff mbox

[2/2] drm: don't oops in ioctls that require the lock if no lock

Message ID 1361317212-23356-2-git-send-email-airlied@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dave Airlie Feb. 19, 2013, 11:40 p.m. UTC 
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(-)

Comments

Tommi Rantala Feb. 20, 2013, 4:11 p.m. UTC | #1 
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
diff mbox

Patch

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)) {