All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sultan Alsawaf <sultan@kerneltoast.com>
To: unlisted-recipients:; (no To-header on input)
Cc: Sultan Alsawaf <sultan@kerneltoast.com>,
	stable@vger.kernel.org, Jani Nikula <jani.nikula@linux.intel.com>,
	Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
	Rodrigo Vivi <rodrigo.vivi@intel.com>,
	David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
	Chris Wilson <chris@chris-wilson.co.uk>,
	Tvrtko Ursulin <tvrtko.ursulin@intel.com>,
	Mika Kuoppala <mika.kuoppala@linux.intel.com>,
	Matthew Auld <matthew.auld@intel.com>,
	Lionel Landwerlin <lionel.g.landwerlin@intel.com>,
	intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH v2] drm/i915: Fix use-after-free due to intel_context_pin/unpin race
Date: Thu,  2 Apr 2020 21:29:44 -0700	[thread overview]
Message-ID: <20200403042948.2533-1-sultan@kerneltoast.com> (raw)
In-Reply-To: <20200403011318.2280-1-sultan@kerneltoast.com>

From: Sultan Alsawaf <sultan@kerneltoast.com>

The retire and active callbacks can run simultaneously, allowing
intel_context_pin() and intel_context_unpin() to run at the same time,
trashing the ring and page tables. In 5.4, this was more noticeable
because intel_ring_unpin() would set ring->vaddr to NULL and cause a
clean NULL-pointer-dereference panic, but in newer kernels the
use-after-free goes unnoticed.

The NULL-pointer-dereference looks like this:
BUG: unable to handle page fault for address: 0000000000003448
RIP: 0010:gen8_emit_flush_render+0x163/0x190
Call Trace:
 execlists_request_alloc+0x25/0x40
 __i915_request_create+0x1f4/0x2c0
 i915_request_create+0x71/0xc0
 i915_gem_do_execbuffer+0xb98/0x1a80
 ? preempt_count_add+0x68/0xa0
 ? _raw_spin_lock+0x13/0x30
 ? _raw_spin_unlock+0x16/0x30
 i915_gem_execbuffer2_ioctl+0x1de/0x3c0
 ? i915_gem_busy_ioctl+0x7f/0x1d0
 ? i915_gem_execbuffer_ioctl+0x2d0/0x2d0
 drm_ioctl_kernel+0xb2/0x100
 drm_ioctl+0x209/0x360
 ? i915_gem_execbuffer_ioctl+0x2d0/0x2d0
 ksys_ioctl+0x87/0xc0
 __x64_sys_ioctl+0x16/0x20
 do_syscall_64+0x4e/0x150
 entry_SYSCALL_64_after_hwframe+0x44/0xa9

Protect __intel_context_retire() with active->mutex (i.e., ref->mutex)
to complement the active callback and fix the corruption.

Fixes: 12c255b5dad1 ("drm/i915: Provide an i915_active.acquire callback")
Cc: <stable@vger.kernel.org>
Signed-off-by: Sultan Alsawaf <sultan@kerneltoast.com>
---
v2: Reduce the scope of the mutex lock to only __intel_context_retire()
    and mark it as a function that may sleep so it doesn't run in
    atomic context

 drivers/gpu/drm/i915/gt/intel_context.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
index 57e8a051ddc2..9b9be8058881 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -221,6 +221,7 @@ static void __intel_context_retire(struct i915_active *active)
 
 	CE_TRACE(ce, "retire\n");
 
+	mutex_lock(&active->mutex);
 	set_bit(CONTEXT_VALID_BIT, &ce->flags);
 	if (ce->state)
 		__context_unpin_state(ce->state);
@@ -229,6 +230,7 @@ static void __intel_context_retire(struct i915_active *active)
 	__ring_retire(ce->ring);
 
 	intel_context_put(ce);
+	mutex_unlock(&active->mutex);
 }
 
 static int __intel_context_active(struct i915_active *active)
@@ -288,7 +290,8 @@ intel_context_init(struct intel_context *ce,
 	mutex_init(&ce->pin_mutex);
 
 	i915_active_init(&ce->active,
-			 __intel_context_active, __intel_context_retire);
+			 __intel_context_active,
+			 i915_active_may_sleep(__intel_context_retire));
 }
 
 void intel_context_fini(struct intel_context *ce)
-- 
2.26.0


  reply	other threads:[~2020-04-03  4:30 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-03  1:13 [PATCH] drm/i915: Fix use-after-free due to intel_context_pin/unpin race Sultan Alsawaf
2020-04-03  4:29 ` Sultan Alsawaf [this message]
2020-04-03 22:35   ` [PATCH v3] drm/i915: Synchronize active and retire callbacks Sultan Alsawaf
2020-04-04  2:41     ` Sultan Alsawaf
2020-04-04  2:41       ` Sultan Alsawaf
2020-04-04  2:41       ` [Intel-gfx] " Sultan Alsawaf
2020-04-07  6:40       ` [PATCH v4] " Sultan Alsawaf
2020-04-14  6:13         ` Sultan Alsawaf
2020-04-14  6:13           ` Sultan Alsawaf
2020-04-14  6:13           ` [Intel-gfx] " Sultan Alsawaf
2020-04-14  8:23           ` Chris Wilson
2020-04-14  8:23             ` Chris Wilson
2020-04-14  8:23             ` [Intel-gfx] " Chris Wilson
2020-04-14 14:43             ` Sultan Alsawaf
2020-04-14 14:43               ` Sultan Alsawaf
2020-04-14 14:43               ` [Intel-gfx] " Sultan Alsawaf
2020-04-20  5:24               ` Sultan Alsawaf
2020-04-20  5:24                 ` Sultan Alsawaf
2020-04-20  5:24                 ` [Intel-gfx] " Sultan Alsawaf
2020-04-20  8:21                 ` Joonas Lahtinen
2020-04-20  8:21                   ` Joonas Lahtinen
2020-04-20  8:21                   ` [Intel-gfx] " Joonas Lahtinen
2020-04-20 16:15                   ` Sultan Alsawaf
2020-04-20 16:15                     ` Sultan Alsawaf
2020-04-20 16:15                     ` [Intel-gfx] " Sultan Alsawaf
2020-04-21  6:51                     ` Joonas Lahtinen
2020-04-21  6:51                       ` Joonas Lahtinen
2020-04-21  6:51                       ` [Intel-gfx] " Joonas Lahtinen
2020-04-21 15:54                       ` Sultan Alsawaf
2020-04-21 15:54                         ` Sultan Alsawaf
2020-04-21 15:54                         ` [Intel-gfx] " Sultan Alsawaf
2020-04-15  0:30       ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2020-04-15  0:47       ` [Intel-gfx] ✗ Fi.CI.DOCS: " Patchwork
2020-04-15  0:50       ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2020-04-15  3:29 ` [drm/i915] 6dc0b234a6: BUG:sleeping_function_called_from_invalid_context_at_kernel/locking/mutex.c kernel test robot
2020-04-15  3:29   ` kernel test robot
2020-04-15  3:29   ` [Intel-gfx] " kernel test robot
2020-04-15  3:29   ` kernel test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200403042948.2533-1-sultan@kerneltoast.com \
    --to=sultan@kerneltoast.com \
    --cc=airlied@linux.ie \
    --cc=chris@chris-wilson.co.uk \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@linux.intel.com \
    --cc=joonas.lahtinen@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lionel.g.landwerlin@intel.com \
    --cc=matthew.auld@intel.com \
    --cc=mika.kuoppala@linux.intel.com \
    --cc=rodrigo.vivi@intel.com \
    --cc=stable@vger.kernel.org \
    --cc=tvrtko.ursulin@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.