public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/i915: Avoid dereference past end of page array in gen6_ppgtt_insert_entries()
@ 2013-12-31 15:50 Chris Wilson
  2013-12-31 15:50 ` [PATCH 2/2] drm/i915: Avoid dereference past end of page array in gen8_ppgtt_insert_entries() Chris Wilson
  0 siblings, 1 reply; 3+ messages in thread
From: Chris Wilson @ 2013-12-31 15:50 UTC (permalink / raw)
  To: intel-gfx; +Cc: Chris Wilson, Daniel Vetter, Ben Widawsky, stable

[   89.237347] BUG: unable to handle kernel paging request at ffff880096326000
[   89.237369] IP: [<ffffffff81347227>] gen6_ppgtt_insert_entries+0x117/0x170
[   89.237382] PGD 2272067 PUD 25df0e067 PMD 25de5c067 PTE 8000000096326060
[   89.237394] Oops: 0000 [#1] SMP DEBUG_PAGEALLOC
[   89.237404] CPU: 1 PID: 1981 Comm: gem_concurrent_ Not tainted 3.13.0-rc4+ #639
[   89.237411] Hardware name: Intel Corporation 2012 Client Platform/Emerald Lake 2, BIOS ACRVMBY1.86C.0078.P00.1201161002 01/16/2012
[   89.237420] task: ffff88024c038030 ti: ffff88024b130000 task.ti: ffff88024b130000
[   89.237425] RIP: 0010:[<ffffffff81347227>]  [<ffffffff81347227>] gen6_ppgtt_insert_entries+0x117/0x170
[   89.237435] RSP: 0018:ffff88024b131ae0  EFLAGS: 00010286
[   89.237440] RAX: ffff880096325000 RBX: 0000000000000400 RCX: 0000000000001000
[   89.237445] RDX: 0000000000000200 RSI: 0000000000000001 RDI: 0000000000000010
[   89.237451] RBP: ffff88024b131b30 R08: ffff88024cc3aef0 R09: 0000000000000000
[   89.237456] R10: 0000000000000000 R11: 0000000000000000 R12: ffff88024cc3ae00
[   89.237462] R13: ffff88024a578000 R14: 0000000000000001 R15: ffff88024a578ffc
[   89.237469] FS:  00007ff5475d8900(0000) GS:ffff88025d020000(0000) knlGS:0000000000000000
[   89.237475] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   89.237480] CR2: ffff880096326000 CR3: 000000024d531000 CR4: 00000000001407e0
[   89.237485] Stack:
[   89.237488]  ffff880000000000 0000020000000000 ffff88024b23f2c0 0000000100000000
[   89.237499]  0000000000000001 000000000007ffff ffff8801e7bf5ac0 ffff8801e7bf5ac0
[   89.237510]  ffff88024cc3ae00 ffff880248a2ee40 ffff88024b131b58 ffffffff813455ed
[   89.237521] Call Trace:
[   89.237528]  [<ffffffff813455ed>] ppgtt_bind_vma+0x3d/0x60
[   89.237534]  [<ffffffff8133d8dc>] i915_gem_object_pin+0x55c/0x6a0
[   89.237541]  [<ffffffff8134275b>] i915_gem_execbuffer_reserve_vma.isra.14+0x5b/0x110
[   89.237548]  [<ffffffff81342a88>] i915_gem_execbuffer_reserve+0x278/0x2c0
[   89.237555]  [<ffffffff81343d29>] i915_gem_do_execbuffer.isra.22+0x699/0x1250
[   89.237562]  [<ffffffff81344d91>] ? i915_gem_execbuffer2+0x51/0x290
[   89.237569]  [<ffffffff81344de6>] i915_gem_execbuffer2+0xa6/0x290
[   89.237575]  [<ffffffff813014f2>] drm_ioctl+0x4d2/0x610
[   89.237582]  [<ffffffff81080bf1>] ? cpuacct_account_field+0xa1/0xc0
[   89.237588]  [<ffffffff81080b55>] ? cpuacct_account_field+0x5/0xc0
[   89.237597]  [<ffffffff811371c0>] do_vfs_ioctl+0x300/0x520
[   89.237603]  [<ffffffff810757a1>] ? vtime_account_user+0x91/0xa0
[   89.237610]  [<ffffffff810e40eb>] ?  context_tracking_user_exit+0x9b/0xe0
[   89.237617]  [<ffffffff81083d7d>] ? trace_hardirqs_on+0xd/0x10
[   89.237623]  [<ffffffff81137425>] SyS_ioctl+0x45/0x80
[   89.237630]  [<ffffffff815afffa>] tracesys+0xd4/0xd9
[   89.237634] Code: 5b 41 5c 41 5d 41 5e 41 5f 5d c3 66 0f 1f 84 00 00 00 00 00 83 45 bc 01 49 8b 84 24 78 01 00 00 65 ff 0c 25 e0 b8 00 00 8b 55 bc <4c> 8b 2c d0 65 ff 04 25 e0 b8 00 00 49 8b 45 00 48 c1 e8 2d 48
[   89.237741] RIP  [<ffffffff81347227>] gen6_ppgtt_insert_entries+0x117/0x170
[   89.237749]  RSP <ffff88024b131ae0>
[   89.237753] CR2: ffff880096326000
[   89.237758] ---[ end trace 27416ba8b18d496c ]---

This bug dates back to the original introduction of the
gen6_ppgtt_insert_entries()

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Ben Widawsky <benjamin.widawsky@intel.com>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index d113eb5e2f5b..366ede6352db 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -801,21 +801,23 @@ static void gen6_ppgtt_insert_entries(struct i915_address_space *vm,
 	unsigned act_pte = first_entry % I915_PPGTT_PT_ENTRIES;
 	struct sg_page_iter sg_iter;
 
-	pt_vaddr = kmap_atomic(ppgtt->pt_pages[act_pt]);
+	pt_vaddr = NULL;
 	for_each_sg_page(pages->sgl, &sg_iter, pages->nents, 0) {
-		dma_addr_t page_addr;
+		if (pt_vaddr == NULL)
+			pt_vaddr = kmap_atomic(ppgtt->pt_pages[act_pt]);
 
-		page_addr = sg_page_iter_dma_address(&sg_iter);
-		pt_vaddr[act_pte] = vm->pte_encode(page_addr, cache_level, true);
+		pt_vaddr[act_pte] =
+			vm->pte_encode(sg_page_iter_dma_address(&sg_iter),
+				       cache_level, true);
 		if (++act_pte == I915_PPGTT_PT_ENTRIES) {
 			kunmap_atomic(pt_vaddr);
+			pt_vaddr = NULL;
 			act_pt++;
-			pt_vaddr = kmap_atomic(ppgtt->pt_pages[act_pt]);
 			act_pte = 0;
-
 		}
 	}
-	kunmap_atomic(pt_vaddr);
+	if (pt_vaddr)
+		kunmap_atomic(pt_vaddr);
 }
 
 static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
-- 
1.8.5.2

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

* [PATCH 2/2] drm/i915: Avoid dereference past end of page array in gen8_ppgtt_insert_entries()
  2013-12-31 15:50 [PATCH 1/2] drm/i915: Avoid dereference past end of page array in gen6_ppgtt_insert_entries() Chris Wilson
@ 2013-12-31 15:50 ` Chris Wilson
  2014-01-01 20:15   ` Ben Widawsky
  0 siblings, 1 reply; 3+ messages in thread
From: Chris Wilson @ 2013-12-31 15:50 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter, Ben Widawsky

The bug from gen6_ppgtt_insert_entries() was replicated into
gen8_ppgtt_insert_entries(). This applies the fix for the OOPS from the
previous patch to the gen8 routine.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Ben Widawsky <benjamin.widawsky@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 366ede6352db..2a26d739a962 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -299,23 +299,23 @@ static void gen8_ppgtt_insert_entries(struct i915_address_space *vm,
 	unsigned act_pte = first_entry % GEN8_PTES_PER_PAGE;
 	struct sg_page_iter sg_iter;
 
-	pt_vaddr = kmap_atomic(&ppgtt->gen8_pt_pages[act_pt]);
+	pt_vaddr = NULL;
 	for_each_sg_page(pages->sgl, &sg_iter, pages->nents, 0) {
-		dma_addr_t page_addr;
+		if (pt_vaddr == NULL)
+			pt_vaddr = kmap_atomic(&ppgtt->gen8_pt_pages[act_pt]);
 
-		page_addr = sg_dma_address(sg_iter.sg) +
-				(sg_iter.sg_pgoffset << PAGE_SHIFT);
-		pt_vaddr[act_pte] = gen8_pte_encode(page_addr, cache_level,
-						    true);
+		pt_vaddr[act_pte] =
+			gen8_pte_encode(sg_page_iter_dma_address(&sg_iter),
+					cache_level, true);
 		if (++act_pte == GEN8_PTES_PER_PAGE) {
 			kunmap_atomic(pt_vaddr);
+			pt_vaddr = NULL;
 			act_pt++;
-			pt_vaddr = kmap_atomic(&ppgtt->gen8_pt_pages[act_pt]);
 			act_pte = 0;
-
 		}
 	}
-	kunmap_atomic(pt_vaddr);
+	if (pt_vaddr)
+		kunmap_atomic(pt_vaddr);
 }
 
 static void gen8_ppgtt_cleanup(struct i915_address_space *vm)
-- 
1.8.5.2

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

* Re: [PATCH 2/2] drm/i915: Avoid dereference past end of page array in gen8_ppgtt_insert_entries()
  2013-12-31 15:50 ` [PATCH 2/2] drm/i915: Avoid dereference past end of page array in gen8_ppgtt_insert_entries() Chris Wilson
@ 2014-01-01 20:15   ` Ben Widawsky
  0 siblings, 0 replies; 3+ messages in thread
From: Ben Widawsky @ 2014-01-01 20:15 UTC (permalink / raw)
  To: Chris Wilson; +Cc: Daniel Vetter, intel-gfx

On Tue, Dec 31, 2013 at 03:50:31PM +0000, Chris Wilson wrote:
> The bug from gen6_ppgtt_insert_entries() was replicated into
> gen8_ppgtt_insert_entries(). This applies the fix for the OOPS from the
> previous patch to the gen8 routine.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Ben Widawsky <benjamin.widawsky@intel.com>

Note to Daniel: This is still broken (for gen8) in the 4GB GGTT series.
There, pdpe is the bad guy.

Both are:
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 366ede6352db..2a26d739a962 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -299,23 +299,23 @@ static void gen8_ppgtt_insert_entries(struct i915_address_space *vm,
>  	unsigned act_pte = first_entry % GEN8_PTES_PER_PAGE;
>  	struct sg_page_iter sg_iter;
>  
> -	pt_vaddr = kmap_atomic(&ppgtt->gen8_pt_pages[act_pt]);
> +	pt_vaddr = NULL;
>  	for_each_sg_page(pages->sgl, &sg_iter, pages->nents, 0) {
> -		dma_addr_t page_addr;
> +		if (pt_vaddr == NULL)
> +			pt_vaddr = kmap_atomic(&ppgtt->gen8_pt_pages[act_pt]);
>  
> -		page_addr = sg_dma_address(sg_iter.sg) +
> -				(sg_iter.sg_pgoffset << PAGE_SHIFT);
> -		pt_vaddr[act_pte] = gen8_pte_encode(page_addr, cache_level,
> -						    true);
> +		pt_vaddr[act_pte] =
> +			gen8_pte_encode(sg_page_iter_dma_address(&sg_iter),
> +					cache_level, true);
>  		if (++act_pte == GEN8_PTES_PER_PAGE) {
>  			kunmap_atomic(pt_vaddr);
> +			pt_vaddr = NULL;
>  			act_pt++;
> -			pt_vaddr = kmap_atomic(&ppgtt->gen8_pt_pages[act_pt]);
>  			act_pte = 0;
> -
>  		}
>  	}
> -	kunmap_atomic(pt_vaddr);
> +	if (pt_vaddr)
> +		kunmap_atomic(pt_vaddr);
>  }
>  
>  static void gen8_ppgtt_cleanup(struct i915_address_space *vm)
> -- 
> 1.8.5.2
> 

-- 
Ben Widawsky, Intel Open Source Technology Center

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

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

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-31 15:50 [PATCH 1/2] drm/i915: Avoid dereference past end of page array in gen6_ppgtt_insert_entries() Chris Wilson
2013-12-31 15:50 ` [PATCH 2/2] drm/i915: Avoid dereference past end of page array in gen8_ppgtt_insert_entries() Chris Wilson
2014-01-01 20:15   ` Ben Widawsky

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox