diff mbox

drm/i915: fix out-of-bounds page_table access

Message ID 1466784286-29059-1-git-send-email-matthew.auld@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Matthew Auld June 24, 2016, 4:04 p.m. UTC
The gen6_for_all_pdes macro does the upper-bound evaluation after
accessing the page_table array, hence on the final iteration we end up
hitting an out-of-bounds error:

[ 1023.831657] UBSAN: Undefined behaviour in drivers/gpu/drm/i915/i915_gem_gtt.c:1993:2
[ 1023.831680] index 512 is out of range for type 'i915_page_table *[512]'
[ 1023.831696] CPU: 0 PID: 4833 Comm: rmmod Tainted: G     U          4.7.0-rc4-drm-intel-debug+ #5
[ 1023.831698] Hardware name: ASUS All Series/Z87-K, BIOS 1202 05/13/2014
[ 1023.831700]  0000000000000200 00000000adfe9733 ffff8801a3917988 ffffffff818cc0a4
[ 1023.831705]  0000000041b58ab3 ffffffff8275ca08 ffffffff818cbff2 ffff8801a39179b0
[ 1023.831708]  ffff8801a3917960 0000000000000200 1ffffffff4365b17 0000000000000001
[ 1023.831711] Call Trace:
[ 1023.831717]  [<ffffffff818cc0a4>] dump_stack+0xb2/0x10e
[ 1023.831721]  [<ffffffff818cbff2>] ? _atomic_dec_and_lock+0x152/0x152
[ 1023.831726]  [<ffffffff81952b0b>] ubsan_epilogue+0xd/0x4e
[ 1023.831730]  [<ffffffff8195373d>] __ubsan_handle_out_of_bounds+0x107/0x14d
[ 1023.831733]  [<ffffffff81953636>] ? __ubsan_handle_shift_out_of_bounds+0x24c/0x24c
[ 1023.831737]  [<ffffffff814bfde6>] ? kfree+0x246/0x3f0
[ 1023.831801]  [<ffffffffa183bff8>] gen6_ppgtt_cleanup+0x128/0x130 [i915]

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Chris Wilson June 24, 2016, 4:37 p.m. UTC | #1
On Fri, Jun 24, 2016 at 05:04:46PM +0100, Matthew Auld wrote:
> The gen6_for_all_pdes macro does the upper-bound evaluation after
> accessing the page_table array, hence on the final iteration we end up
> hitting an out-of-bounds error:
> 
> [ 1023.831657] UBSAN: Undefined behaviour in drivers/gpu/drm/i915/i915_gem_gtt.c:1993:2
> [ 1023.831680] index 512 is out of range for type 'i915_page_table *[512]'
> [ 1023.831696] CPU: 0 PID: 4833 Comm: rmmod Tainted: G     U          4.7.0-rc4-drm-intel-debug+ #5
> [ 1023.831698] Hardware name: ASUS All Series/Z87-K, BIOS 1202 05/13/2014
> [ 1023.831700]  0000000000000200 00000000adfe9733 ffff8801a3917988 ffffffff818cc0a4
> [ 1023.831705]  0000000041b58ab3 ffffffff8275ca08 ffffffff818cbff2 ffff8801a39179b0
> [ 1023.831708]  ffff8801a3917960 0000000000000200 1ffffffff4365b17 0000000000000001
> [ 1023.831711] Call Trace:
> [ 1023.831717]  [<ffffffff818cc0a4>] dump_stack+0xb2/0x10e
> [ 1023.831721]  [<ffffffff818cbff2>] ? _atomic_dec_and_lock+0x152/0x152
> [ 1023.831726]  [<ffffffff81952b0b>] ubsan_epilogue+0xd/0x4e
> [ 1023.831730]  [<ffffffff8195373d>] __ubsan_handle_out_of_bounds+0x107/0x14d
> [ 1023.831733]  [<ffffffff81953636>] ? __ubsan_handle_shift_out_of_bounds+0x24c/0x24c
> [ 1023.831737]  [<ffffffff814bfde6>] ? kfree+0x246/0x3f0
> [ 1023.831801]  [<ffffffffa183bff8>] gen6_ppgtt_cleanup+0x128/0x130 [i915]
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>

Ok. Tried to find something to complain about and couldn't.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
Dave Gordon June 24, 2016, 6:28 p.m. UTC | #2
On 24/06/16 17:37, Chris Wilson wrote:
> On Fri, Jun 24, 2016 at 05:04:46PM +0100, Matthew Auld wrote:
>> The gen6_for_all_pdes macro does the upper-bound evaluation after
>> accessing the page_table array, hence on the final iteration we end up
>> hitting an out-of-bounds error:
>>
>> [ 1023.831657] UBSAN: Undefined behaviour in drivers/gpu/drm/i915/i915_gem_gtt.c:1993:2
>> [ 1023.831680] index 512 is out of range for type 'i915_page_table *[512]'
>> [ 1023.831696] CPU: 0 PID: 4833 Comm: rmmod Tainted: G     U          4.7.0-rc4-drm-intel-debug+ #5
>> [ 1023.831698] Hardware name: ASUS All Series/Z87-K, BIOS 1202 05/13/2014
>> [ 1023.831700]  0000000000000200 00000000adfe9733 ffff8801a3917988 ffffffff818cc0a4
>> [ 1023.831705]  0000000041b58ab3 ffffffff8275ca08 ffffffff818cbff2 ffff8801a39179b0
>> [ 1023.831708]  ffff8801a3917960 0000000000000200 1ffffffff4365b17 0000000000000001
>> [ 1023.831711] Call Trace:
>> [ 1023.831717]  [<ffffffff818cc0a4>] dump_stack+0xb2/0x10e
>> [ 1023.831721]  [<ffffffff818cbff2>] ? _atomic_dec_and_lock+0x152/0x152
>> [ 1023.831726]  [<ffffffff81952b0b>] ubsan_epilogue+0xd/0x4e
>> [ 1023.831730]  [<ffffffff8195373d>] __ubsan_handle_out_of_bounds+0x107/0x14d
>> [ 1023.831733]  [<ffffffff81953636>] ? __ubsan_handle_shift_out_of_bounds+0x24c/0x24c
>> [ 1023.831737]  [<ffffffff814bfde6>] ? kfree+0x246/0x3f0
>> [ 1023.831801]  [<ffffffffa183bff8>] gen6_ppgtt_cleanup+0x128/0x130 [i915]
>>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
>
> Ok. Tried to find something to complain about and couldn't.
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris

Well ... not enough to reject it, but there's the lack of parentheses 
round macro parameters, and it uses ?: rather than the && style used in 
the Gen8 equivalents. I'll post an alternative based on the Gen8 version ...

.Dave.
Dave Gordon June 27, 2016, 11:59 a.m. UTC | #3
On 25/06/16 06:26, Patchwork wrote:
> == Series Details ==
>
> Series: drm/i915: fix out-of-bounds page_table access (rev2)
> URL   : https://patchwork.freedesktop.org/series/9148/
> State : warning
>
> == Summary ==
>
> Series 9148v2 drm/i915: fix out-of-bounds page_table access
> http://patchwork.freedesktop.org/api/1.0/series/9148/revisions/2/mbox
>
> Test drv_module_reload_basic:
>                  dmesg-warn -> PASS       (ro-byt-n2820)
> Test kms_pipe_crc_basic:
>          Subgroup hang-read-crc-pipe-c:
>                  pass       -> DMESG-WARN (ro-ivb2-i7-3770)

This looks like a variation of
https://bugs.freedesktop.org/show_bug.cgi?id=74102
[ILK/SNB/IVB]igt/testdisplay causes *ERROR* Pipe A FIFO underrun
so unrelated to page table macros.

.Dave.

>          Subgroup suspend-read-crc-pipe-c:
>                  dmesg-warn -> SKIP       (ro-bdw-i5-5250u)
>
> fi-hsw-i7-4770k  total:229  pass:194  dwarn:0   dfail:0   fail:2   skip:33
> fi-kbl-qkkr      total:229  pass:160  dwarn:29  dfail:0   fail:0   skip:40
> fi-skl-i5-6260u  total:229  pass:202  dwarn:0   dfail:0   fail:2   skip:25
> fi-snb-i7-2600   total:229  pass:174  dwarn:0   dfail:0   fail:2   skip:53
> ro-bdw-i5-5250u  total:227  pass:201  dwarn:3   dfail:1   fail:1   skip:21
> ro-bdw-i7-5600u  total:227  pass:189  dwarn:0   dfail:1   fail:0   skip:37
> ro-bsw-n3050     total:227  pass:176  dwarn:0   dfail:1   fail:2   skip:48
> ro-byt-n2820     total:227  pass:177  dwarn:0   dfail:1   fail:4   skip:45
> ro-hsw-i3-4010u  total:227  pass:194  dwarn:0   dfail:1   fail:1   skip:31
> ro-hsw-i7-4770r  total:227  pass:194  dwarn:0   dfail:1   fail:1   skip:31
> ro-ilk-i7-620lm  total:227  pass:154  dwarn:0   dfail:1   fail:2   skip:70
> ro-ilk1-i5-650   total:222  pass:154  dwarn:0   dfail:1   fail:2   skip:65
> ro-ivb-i7-3770   total:227  pass:185  dwarn:0   dfail:1   fail:1   skip:40
> ro-ivb2-i7-3770  total:227  pass:188  dwarn:1   dfail:1   fail:1   skip:36
> ro-skl3-i5-6260u total:227  pass:205  dwarn:1   dfail:1   fail:1   skip:19
> ro-snb-i7-2620M  total:227  pass:178  dwarn:0   dfail:1   fail:1   skip:47
> ro-bdw-i7-5557U failed to connect after reboot
>
> Results at /archive/results/CI_IGT_test/RO_Patchwork_1305/
>
> 5c244f4 drm-intel-nightly: 2016y-06m-24d-15h-17m-32s UTC integration manifest
> 6811927 drm/i915: tweak gen6_for_{each_pde, all_pdes} macros
>
Tvrtko Ursulin June 27, 2016, 12:14 p.m. UTC | #4
On 27/06/16 12:59, Dave Gordon wrote:
> On 25/06/16 06:26, Patchwork wrote:
>> == Series Details ==
>>
>> Series: drm/i915: fix out-of-bounds page_table access (rev2)
>> URL   : https://patchwork.freedesktop.org/series/9148/
>> State : warning
>>
>> == Summary ==
>>
>> Series 9148v2 drm/i915: fix out-of-bounds page_table access
>> http://patchwork.freedesktop.org/api/1.0/series/9148/revisions/2/mbox
>>
>> Test drv_module_reload_basic:
>>                  dmesg-warn -> PASS       (ro-byt-n2820)
>> Test kms_pipe_crc_basic:
>>          Subgroup hang-read-crc-pipe-c:
>>                  pass       -> DMESG-WARN (ro-ivb2-i7-3770)
>
> This looks like a variation of
> https://bugs.freedesktop.org/show_bug.cgi?id=74102
> [ILK/SNB/IVB]igt/testdisplay causes *ERROR* Pipe A FIFO underrun
> so unrelated to page table macros.
>
> .Dave.
>
>>          Subgroup suspend-read-crc-pipe-c:
>>                  dmesg-warn -> SKIP       (ro-bdw-i5-5250u)
>>
>> fi-hsw-i7-4770k  total:229  pass:194  dwarn:0   dfail:0   fail:2
>> skip:33
>> fi-kbl-qkkr      total:229  pass:160  dwarn:29  dfail:0   fail:0
>> skip:40
>> fi-skl-i5-6260u  total:229  pass:202  dwarn:0   dfail:0   fail:2
>> skip:25
>> fi-snb-i7-2600   total:229  pass:174  dwarn:0   dfail:0   fail:2
>> skip:53
>> ro-bdw-i5-5250u  total:227  pass:201  dwarn:3   dfail:1   fail:1
>> skip:21
>> ro-bdw-i7-5600u  total:227  pass:189  dwarn:0   dfail:1   fail:0
>> skip:37
>> ro-bsw-n3050     total:227  pass:176  dwarn:0   dfail:1   fail:2
>> skip:48
>> ro-byt-n2820     total:227  pass:177  dwarn:0   dfail:1   fail:4
>> skip:45
>> ro-hsw-i3-4010u  total:227  pass:194  dwarn:0   dfail:1   fail:1
>> skip:31
>> ro-hsw-i7-4770r  total:227  pass:194  dwarn:0   dfail:1   fail:1
>> skip:31
>> ro-ilk-i7-620lm  total:227  pass:154  dwarn:0   dfail:1   fail:2
>> skip:70
>> ro-ilk1-i5-650   total:222  pass:154  dwarn:0   dfail:1   fail:2
>> skip:65
>> ro-ivb-i7-3770   total:227  pass:185  dwarn:0   dfail:1   fail:1
>> skip:40
>> ro-ivb2-i7-3770  total:227  pass:188  dwarn:1   dfail:1   fail:1
>> skip:36
>> ro-skl3-i5-6260u total:227  pass:205  dwarn:1   dfail:1   fail:1
>> skip:19
>> ro-snb-i7-2620M  total:227  pass:178  dwarn:0   dfail:1   fail:1
>> skip:47
>> ro-bdw-i7-5557U failed to connect after reboot
>>
>> Results at /archive/results/CI_IGT_test/RO_Patchwork_1305/
>>
>> 5c244f4 drm-intel-nightly: 2016y-06m-24d-15h-17m-32s UTC integration
>> manifest
>> 6811927 drm/i915: tweak gen6_for_{each_pde, all_pdes} macros

Merged to dinq, thanks for the patch and review.

Regards,

Tvrtko
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 163b564..9e5228d 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -409,7 +409,7 @@  struct i915_hw_ppgtt {
 
 #define gen6_for_all_pdes(pt, ppgtt, iter)  \
 	for (iter = 0;		\
-	     pt = ppgtt->pd.page_table[iter], iter < I915_PDES;	\
+	     iter < I915_PDES ? (pt = ppgtt->pd.page_table[iter]), 1 : 0; \
 	     iter++)
 
 static inline uint32_t i915_pte_index(uint64_t address, uint32_t pde_shift)