stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Fix use-after-free when destroying GEM context
@ 2020-01-14 18:39 Tyler Hicks
  2020-01-15 15:08 ` Greg KH
  0 siblings, 1 reply; 2+ messages in thread
From: Tyler Hicks @ 2020-01-14 18:39 UTC (permalink / raw)
  To: stable; +Cc: Chris Wilson, Jon Bloomfield

This patch is a simplified fix to address a use-after-free in 4.14.x and
4.19.x stable kernels. The flaw is already fixed upstream, starting in
5.2, by commit 7dc40713618c ("drm/i915: Introduce a mutex for
file_priv->context_idr") as part of a more complex patch series that
isn't appropriate for backporting to stable kernels.

Expand mutex coverage, while destroying the GEM context, to include the
GEM context lookup step. This fixes a use-after-free detected by KASAN:

 ==================================================================
 BUG: KASAN: use-after-free in i915_ppgtt_close+0x2ca/0x2f0
 Write of size 1 at addr ffff8881368a8368 by task i915-poc/3124

 CPU: 0 PID: 3124 Comm: i915-poc Not tainted 4.14.164 #1
 Hardware name: HP HP Elite x2 1012 G1 /80FC, BIOS N85 Ver. 01.20 04/05/2017
 Call Trace:
  dump_stack+0xcd/0x12e
  ? _atomic_dec_and_lock+0x1b2/0x1b2
  ? i915_ppgtt_close+0x2ca/0x2f0
  ? printk+0x8f/0xab
  ? show_regs_print_info+0x53/0x53
  ? i915_ppgtt_close+0x2ca/0x2f0
  print_address_description+0x65/0x270
  ? i915_ppgtt_close+0x2ca/0x2f0
  kasan_report+0x251/0x340
  i915_ppgtt_close+0x2ca/0x2f0
  ? __radix_tree_insert+0x3f0/0x3f0
  ? i915_ppgtt_init_hw+0x7c0/0x7c0
  context_close+0x42e/0x680
  ? i915_gem_context_release+0x230/0x230
  ? kasan_kmalloc+0xa0/0xd0
  ? radix_tree_delete_item+0x1d4/0x250
  ? radix_tree_lookup+0x10/0x10
  ? inet_recvmsg+0x4b0/0x4b0
  ? kasan_slab_free+0x88/0xc0
  i915_gem_context_destroy_ioctl+0x236/0x300
  ? i915_gem_context_create_ioctl+0x360/0x360
  ? drm_dev_printk+0x1d0/0x1d0
  ? memcpy+0x34/0x50
  ? i915_gem_context_create_ioctl+0x360/0x360
  drm_ioctl_kernel+0x1b0/0x2b0
  ? drm_ioctl_permit+0x2a0/0x2a0
  ? avc_ss_reset+0xd0/0xd0
  drm_ioctl+0x6fe/0xa20
  ? i915_gem_context_create_ioctl+0x360/0x360
  ? drm_getstats+0x20/0x20
  ? put_unused_fd+0x260/0x260
  do_vfs_ioctl+0x189/0x12d0
  ? ioctl_preallocate+0x280/0x280
  ? selinux_file_ioctl+0x3a7/0x680
  ? selinux_bprm_set_creds+0xe30/0xe30
  ? security_file_ioctl+0x69/0xa0
  ? selinux_bprm_set_creds+0xe30/0xe30
  SyS_ioctl+0x6f/0x80
  ? __sys_sendmmsg+0x4a0/0x4a0
  ? do_vfs_ioctl+0x12d0/0x12d0
  do_syscall_64+0x214/0x5f0
  ? __switch_to_asm+0x31/0x60
  ? __switch_to_asm+0x25/0x60
  ? __switch_to_asm+0x31/0x60
  ? syscall_return_slowpath+0x2c0/0x2c0
  ? copy_overflow+0x20/0x20
  ? __switch_to_asm+0x25/0x60
  ? syscall_return_via_sysret+0x2a/0x7a
  ? prepare_exit_to_usermode+0x200/0x200
  ? __switch_to_asm+0x31/0x60
  ? __switch_to_asm+0x31/0x60
  ? __switch_to_asm+0x25/0x60
  ? __switch_to_asm+0x25/0x60
  ? __switch_to_asm+0x31/0x60
  ? __switch_to_asm+0x25/0x60
  ? __switch_to_asm+0x31/0x60
  ? __switch_to_asm+0x31/0x60
  ? __switch_to_asm+0x25/0x60
  entry_SYSCALL_64_after_hwframe+0x3d/0xa2
 RIP: 0033:0x7f7fda5115d7
 RSP: 002b:00007f7eec317ec8 EFLAGS: 00000286 ORIG_RAX: 0000000000000010
 RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f7fda5115d7
 RDX: 000055b306db9188 RSI: 000000004008646e RDI: 0000000000000003
 RBP: 00007f7eec317ef0 R08: 00007f7eec318700 R09: 0000000000000000
 R10: 0000000000000000 R11: 0000000000000286 R12: 00007f7eec317fc0
 R13: 0000000000000000 R14: 0000000000000000 R15: 00007ffd8007ade0

 Allocated by task 2898:
  save_stack+0x32/0xb0
  kasan_kmalloc+0xa0/0xd0
  kmem_cache_alloc_trace+0x5e/0x180
  i915_ppgtt_create+0xab/0x2510
  i915_gem_create_context+0x981/0xf90
  i915_gem_context_create_ioctl+0x1d7/0x360
  drm_ioctl_kernel+0x1b0/0x2b0
  drm_ioctl+0x6fe/0xa20
  do_vfs_ioctl+0x189/0x12d0
  SyS_ioctl+0x6f/0x80
  do_syscall_64+0x214/0x5f0
  entry_SYSCALL_64_after_hwframe+0x3d/0xa2

 Freed by task 104:
  save_stack+0x32/0xb0
  kasan_slab_free+0x72/0xc0
  kfree+0x88/0x190
  i915_ppgtt_release+0x24e/0x460
  i915_gem_context_free+0x90/0x480
  contexts_free_worker+0x54/0x80
  process_one_work+0x876/0x14e0
  worker_thread+0x1b8/0xfd0
  kthread+0x2f8/0x3c0
  ret_from_fork+0x35/0x40

 The buggy address belongs to the object at ffff8881368a8000
  which belongs to the cache kmalloc-8192 of size 8192
 The buggy address is located 872 bytes inside of
  8192-byte region [ffff8881368a8000, ffff8881368aa000)
 The buggy address belongs to the page:
 page:ffffea0004da2a00 count:1 mapcount:0 mapping:          (null) index:0x0 compound_mapcount: 0
 flags: 0x200000000008100(slab|head)
 raw: 0200000000008100 0000000000000000 0000000000000000 0000000100030003
 raw: dead000000000100 dead000000000200 ffff88822a002280 0000000000000000
 page dumped because: kasan: bad access detected

 Memory state around the buggy address:
  ffff8881368a8200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
  ffff8881368a8280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
 >ffff8881368a8300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
                                                           ^
  ffff8881368a8380: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
  ffff8881368a8400: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
 ==================================================================

Fixes: 1acfc104cdf8 ("drm/i915: Enable rcu-only context lookups")
Reported-by: 罗权 <luoquan@qianxin.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jon Bloomfield <jon.bloomfield@intel.com>
Cc: stable@vger.kernel.org # 4.14.x
Cc: stable@vger.kernel.org # 4.19.x
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
---
 drivers/gpu/drm/i915/i915_gem_context.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 7a0e6dbbad2e..ef383fd42988 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -770,18 +770,19 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
 	if (args->ctx_id == DEFAULT_CONTEXT_HANDLE)
 		return -ENOENT;
 
+	ret = i915_mutex_lock_interruptible(dev);
+	if (ret)
+		return ret;
+
 	ctx = i915_gem_context_lookup(file_priv, args->ctx_id);
-	if (!ctx)
+	if (!ctx) {
+		mutex_unlock(&dev->struct_mutex);
 		return -ENOENT;
-
-	ret = mutex_lock_interruptible(&dev->struct_mutex);
-	if (ret)
-		goto out;
+	}
 
 	__destroy_hw_context(ctx, file_priv);
 	mutex_unlock(&dev->struct_mutex);
 
-out:
 	i915_gem_context_put(ctx);
 	return 0;
 }
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] drm/i915: Fix use-after-free when destroying GEM context
  2020-01-14 18:39 [PATCH] drm/i915: Fix use-after-free when destroying GEM context Tyler Hicks
@ 2020-01-15 15:08 ` Greg KH
  0 siblings, 0 replies; 2+ messages in thread
From: Greg KH @ 2020-01-15 15:08 UTC (permalink / raw)
  To: Tyler Hicks; +Cc: stable, Chris Wilson, Jon Bloomfield

On Tue, Jan 14, 2020 at 06:39:37PM +0000, Tyler Hicks wrote:
> This patch is a simplified fix to address a use-after-free in 4.14.x and
> 4.19.x stable kernels. The flaw is already fixed upstream, starting in
> 5.2, by commit 7dc40713618c ("drm/i915: Introduce a mutex for
> file_priv->context_idr") as part of a more complex patch series that
> isn't appropriate for backporting to stable kernels.
> 
> Expand mutex coverage, while destroying the GEM context, to include the
> GEM context lookup step. This fixes a use-after-free detected by KASAN:
> 
>  ==================================================================
>  BUG: KASAN: use-after-free in i915_ppgtt_close+0x2ca/0x2f0
>  Write of size 1 at addr ffff8881368a8368 by task i915-poc/3124
> 
>  CPU: 0 PID: 3124 Comm: i915-poc Not tainted 4.14.164 #1
>  Hardware name: HP HP Elite x2 1012 G1 /80FC, BIOS N85 Ver. 01.20 04/05/2017
>  Call Trace:
>   dump_stack+0xcd/0x12e
>   ? _atomic_dec_and_lock+0x1b2/0x1b2
>   ? i915_ppgtt_close+0x2ca/0x2f0
>   ? printk+0x8f/0xab
>   ? show_regs_print_info+0x53/0x53
>   ? i915_ppgtt_close+0x2ca/0x2f0
>   print_address_description+0x65/0x270
>   ? i915_ppgtt_close+0x2ca/0x2f0
>   kasan_report+0x251/0x340
>   i915_ppgtt_close+0x2ca/0x2f0
>   ? __radix_tree_insert+0x3f0/0x3f0
>   ? i915_ppgtt_init_hw+0x7c0/0x7c0
>   context_close+0x42e/0x680
>   ? i915_gem_context_release+0x230/0x230
>   ? kasan_kmalloc+0xa0/0xd0
>   ? radix_tree_delete_item+0x1d4/0x250
>   ? radix_tree_lookup+0x10/0x10
>   ? inet_recvmsg+0x4b0/0x4b0
>   ? kasan_slab_free+0x88/0xc0
>   i915_gem_context_destroy_ioctl+0x236/0x300
>   ? i915_gem_context_create_ioctl+0x360/0x360
>   ? drm_dev_printk+0x1d0/0x1d0
>   ? memcpy+0x34/0x50
>   ? i915_gem_context_create_ioctl+0x360/0x360
>   drm_ioctl_kernel+0x1b0/0x2b0
>   ? drm_ioctl_permit+0x2a0/0x2a0
>   ? avc_ss_reset+0xd0/0xd0
>   drm_ioctl+0x6fe/0xa20
>   ? i915_gem_context_create_ioctl+0x360/0x360
>   ? drm_getstats+0x20/0x20
>   ? put_unused_fd+0x260/0x260
>   do_vfs_ioctl+0x189/0x12d0
>   ? ioctl_preallocate+0x280/0x280
>   ? selinux_file_ioctl+0x3a7/0x680
>   ? selinux_bprm_set_creds+0xe30/0xe30
>   ? security_file_ioctl+0x69/0xa0
>   ? selinux_bprm_set_creds+0xe30/0xe30
>   SyS_ioctl+0x6f/0x80
>   ? __sys_sendmmsg+0x4a0/0x4a0
>   ? do_vfs_ioctl+0x12d0/0x12d0
>   do_syscall_64+0x214/0x5f0
>   ? __switch_to_asm+0x31/0x60
>   ? __switch_to_asm+0x25/0x60
>   ? __switch_to_asm+0x31/0x60
>   ? syscall_return_slowpath+0x2c0/0x2c0
>   ? copy_overflow+0x20/0x20
>   ? __switch_to_asm+0x25/0x60
>   ? syscall_return_via_sysret+0x2a/0x7a
>   ? prepare_exit_to_usermode+0x200/0x200
>   ? __switch_to_asm+0x31/0x60
>   ? __switch_to_asm+0x31/0x60
>   ? __switch_to_asm+0x25/0x60
>   ? __switch_to_asm+0x25/0x60
>   ? __switch_to_asm+0x31/0x60
>   ? __switch_to_asm+0x25/0x60
>   ? __switch_to_asm+0x31/0x60
>   ? __switch_to_asm+0x31/0x60
>   ? __switch_to_asm+0x25/0x60
>   entry_SYSCALL_64_after_hwframe+0x3d/0xa2
>  RIP: 0033:0x7f7fda5115d7
>  RSP: 002b:00007f7eec317ec8 EFLAGS: 00000286 ORIG_RAX: 0000000000000010
>  RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f7fda5115d7
>  RDX: 000055b306db9188 RSI: 000000004008646e RDI: 0000000000000003
>  RBP: 00007f7eec317ef0 R08: 00007f7eec318700 R09: 0000000000000000
>  R10: 0000000000000000 R11: 0000000000000286 R12: 00007f7eec317fc0
>  R13: 0000000000000000 R14: 0000000000000000 R15: 00007ffd8007ade0
> 
>  Allocated by task 2898:
>   save_stack+0x32/0xb0
>   kasan_kmalloc+0xa0/0xd0
>   kmem_cache_alloc_trace+0x5e/0x180
>   i915_ppgtt_create+0xab/0x2510
>   i915_gem_create_context+0x981/0xf90
>   i915_gem_context_create_ioctl+0x1d7/0x360
>   drm_ioctl_kernel+0x1b0/0x2b0
>   drm_ioctl+0x6fe/0xa20
>   do_vfs_ioctl+0x189/0x12d0
>   SyS_ioctl+0x6f/0x80
>   do_syscall_64+0x214/0x5f0
>   entry_SYSCALL_64_after_hwframe+0x3d/0xa2
> 
>  Freed by task 104:
>   save_stack+0x32/0xb0
>   kasan_slab_free+0x72/0xc0
>   kfree+0x88/0x190
>   i915_ppgtt_release+0x24e/0x460
>   i915_gem_context_free+0x90/0x480
>   contexts_free_worker+0x54/0x80
>   process_one_work+0x876/0x14e0
>   worker_thread+0x1b8/0xfd0
>   kthread+0x2f8/0x3c0
>   ret_from_fork+0x35/0x40
> 
>  The buggy address belongs to the object at ffff8881368a8000
>   which belongs to the cache kmalloc-8192 of size 8192
>  The buggy address is located 872 bytes inside of
>   8192-byte region [ffff8881368a8000, ffff8881368aa000)
>  The buggy address belongs to the page:
>  page:ffffea0004da2a00 count:1 mapcount:0 mapping:          (null) index:0x0 compound_mapcount: 0
>  flags: 0x200000000008100(slab|head)
>  raw: 0200000000008100 0000000000000000 0000000000000000 0000000100030003
>  raw: dead000000000100 dead000000000200 ffff88822a002280 0000000000000000
>  page dumped because: kasan: bad access detected
> 
>  Memory state around the buggy address:
>   ffff8881368a8200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>   ffff8881368a8280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>  >ffff8881368a8300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>                                                            ^
>   ffff8881368a8380: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>   ffff8881368a8400: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>  ==================================================================
> 
> Fixes: 1acfc104cdf8 ("drm/i915: Enable rcu-only context lookups")
> Reported-by: 罗权 <luoquan@qianxin.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Jon Bloomfield <jon.bloomfield@intel.com>
> Cc: stable@vger.kernel.org # 4.14.x
> Cc: stable@vger.kernel.org # 4.19.x
> Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_context.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 7a0e6dbbad2e..ef383fd42988 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -770,18 +770,19 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
>  	if (args->ctx_id == DEFAULT_CONTEXT_HANDLE)
>  		return -ENOENT;
>  
> +	ret = i915_mutex_lock_interruptible(dev);
> +	if (ret)
> +		return ret;
> +
>  	ctx = i915_gem_context_lookup(file_priv, args->ctx_id);
> -	if (!ctx)
> +	if (!ctx) {
> +		mutex_unlock(&dev->struct_mutex);
>  		return -ENOENT;
> -
> -	ret = mutex_lock_interruptible(&dev->struct_mutex);
> -	if (ret)
> -		goto out;
> +	}
>  
>  	__destroy_hw_context(ctx, file_priv);
>  	mutex_unlock(&dev->struct_mutex);
>  
> -out:
>  	i915_gem_context_put(ctx);
>  	return 0;
>  }
> -- 
> 2.17.1
> 

Thanks for this, looks sane at first glance so I've queued it up.

greg k-h

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2020-01-15 15:08 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-14 18:39 [PATCH] drm/i915: Fix use-after-free when destroying GEM context Tyler Hicks
2020-01-15 15:08 ` Greg KH

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).