* [PATCH] drm/i915: Hold the table lock whilst walking the file's idr and counting the objects in debugfs
@ 2014-06-17 8:56 Chris Wilson
2014-06-17 9:35 ` Daniel Vetter
0 siblings, 1 reply; 3+ messages in thread
From: Chris Wilson @ 2014-06-17 8:56 UTC (permalink / raw)
To: intel-gfx; +Cc: Chris Wilson, Sam Jansen, stable
Fixes an issue whereby we may race with the table updates (before the
core takes the struct_mutex) and so risk dereferencing a stale pointer in
the iterator for /debugfs/.../i915_gem_objects. For example,
[ 1524.757545] BUG: unable to handle kernel paging request at f53af748
[ 1524.757572] IP: [<c1406982>] per_file_stats+0x12/0x100
[ 1524.757599] *pdpt = 0000000001b13001 *pde = 00000000379fb067 *pte = 80000000353af060
[ 1524.757621] Oops: 0000 [#1] SMP DEBUG_PAGEALLOC
[ 1524.757637] Modules linked in: ctr ccm arc4 ath9k ath9k_common ath9k_hw ath snd_hda_codec_conexant mac80211 snd_hda_codec_generic snd_hda_intel snd_hda_controller snd_hda_codec bnep snd_hwdep rfcomm snd_pcm gpio_ich dell_wmi sparse_keymap snd_seq_midi hid_multitouch uvcvideo snd_seq_midi_event dell_laptop snd_rawmidi dcdbas snd_seq videobuf2_vmalloc videobuf2_memops videobuf2_core usbhid videodev snd_seq_device coretemp snd_timer hid joydev kvm_intel cfg80211 ath3k kvm btusb bluetooth serio_raw snd microcode soundcore lpc_ich wmi mac_hid parport_pc ppdev lp parport psmouse ahci libahci
[ 1524.757825] CPU: 3 PID: 1911 Comm: intel-gpu-overl Tainted: G W OE 3.15.0-rc3+ #96
[ 1524.757840] Hardware name: Dell Inc. Inspiron 1090/Inspiron 1090, BIOS A06 08/23/2011
[ 1524.757855] task: f52f36c0 ti: f4cbc000 task.ti: f4cbc000
[ 1524.757869] EIP: 0060:[<c1406982>] EFLAGS: 00210202 CPU: 3
[ 1524.757884] EIP is at per_file_stats+0x12/0x100
[ 1524.757896] EAX: 0000002d EBX: 00000000 ECX: f4cbdefc EDX: f53af700
[ 1524.757909] ESI: c1406970 EDI: f53af700 EBP: f4cbde6c ESP: f4cbde5c
[ 1524.757922] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
[ 1524.757934] CR0: 80050033 CR2: f53af748 CR3: 356af000 CR4: 000007f0
[ 1524.757945] Stack:
[ 1524.757957] f4cbdefc 00000000 c1406970 f53af700 f4cbdea8 c12e5f15 f4cbdefc c1406970
[ 1524.757993] 0000ffff f4cbde90 0000002d f5dc5cd0 e4e80438 c1181d59 f4cbded8 f4d89900
[ 1524.758027] f5631b40 e5131074 c1903f37 f4cbdf28 c14068e6 f52648a0 c1927748 c1903f37
[ 1524.758062] Call Trace:
[ 1524.758084] [<c1406970>] ? i915_gem_object_info+0x510/0x510
[ 1524.758106] [<c12e5f15>] idr_for_each+0xa5/0x100
[ 1524.758126] [<c1406970>] ? i915_gem_object_info+0x510/0x510
[ 1524.758148] [<c1181d59>] ? seq_vprintf+0x29/0x50
[ 1524.758168] [<c14068e6>] i915_gem_object_info+0x486/0x510
[ 1524.758189] [<c11823a6>] seq_read+0xd6/0x380
[ 1524.758208] [<c116d11d>] ? final_putname+0x1d/0x40
[ 1524.758227] [<c11822d0>] ? seq_hlist_next_percpu+0x90/0x90
[ 1524.758246] [<c1163e52>] vfs_read+0x82/0x150
[ 1524.758265] [<c11645d6>] SyS_read+0x46/0x90
[ 1524.758285] [<c16b8d8c>] sysenter_do_call+0x12/0x22
[ 1524.758298] Code: f5 8f 2a 00 83 c4 6c 31 c0 5b 5e 5f 5d c3 8d 74 26 00 8d bc 27 00 00 00 00 55 89 e5 57 56 53 83 ec 04 3e 8d 74 26 00 83 41 04 01 <8b> 42 48 01 41 08 8b 42 4c 89 d7 85 c0 75 07 8b 42 60 85 c0 74
[ 1524.758461] EIP: [<c1406982>] per_file_stats+0x12/0x100 SS:ESP 0068:f4cbde5c
[ 1524.758485] CR2: 00000000f53af748
Reported-by: Sam Jansen <sam.jansen@starleaf.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Sam Jansen <sam.jansen@starleaf.com>
Cc: stable@vger.kernel.org
---
drivers/gpu/drm/i915/i915_debugfs.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index a8b81407338a..d2610a5f4efe 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -446,7 +446,9 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
memset(&stats, 0, sizeof(stats));
stats.file_priv = file->driver_priv;
+ spin_lock(&file->table_lock);
idr_for_each(&file->object_idr, per_file_stats, &stats);
+ spin_unlock(&file->table_lock);
/*
* Although we have a valid reference on file->pid, that does
* not guarantee that the task_struct who called get_pid() is
--
2.0.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] drm/i915: Hold the table lock whilst walking the file's idr and counting the objects in debugfs
2014-06-17 8:56 [PATCH] drm/i915: Hold the table lock whilst walking the file's idr and counting the objects in debugfs Chris Wilson
@ 2014-06-17 9:35 ` Daniel Vetter
2014-06-24 13:18 ` [Intel-gfx] " Jani Nikula
0 siblings, 1 reply; 3+ messages in thread
From: Daniel Vetter @ 2014-06-17 9:35 UTC (permalink / raw)
To: Chris Wilson; +Cc: Sam Jansen, intel-gfx, stable
On Tue, Jun 17, 2014 at 09:56:24AM +0100, Chris Wilson wrote:
> Fixes an issue whereby we may race with the table updates (before the
> core takes the struct_mutex) and so risk dereferencing a stale pointer in
> the iterator for /debugfs/.../i915_gem_objects. For example,
>
> [ 1524.757545] BUG: unable to handle kernel paging request at f53af748
> [ 1524.757572] IP: [<c1406982>] per_file_stats+0x12/0x100
> [ 1524.757599] *pdpt = 0000000001b13001 *pde = 00000000379fb067 *pte = 80000000353af060
> [ 1524.757621] Oops: 0000 [#1] SMP DEBUG_PAGEALLOC
> [ 1524.757637] Modules linked in: ctr ccm arc4 ath9k ath9k_common ath9k_hw ath snd_hda_codec_conexant mac80211 snd_hda_codec_generic snd_hda_intel snd_hda_controller snd_hda_codec bnep snd_hwdep rfcomm snd_pcm gpio_ich dell_wmi sparse_keymap snd_seq_midi hid_multitouch uvcvideo snd_seq_midi_event dell_laptop snd_rawmidi dcdbas snd_seq videobuf2_vmalloc videobuf2_memops videobuf2_core usbhid videodev snd_seq_device coretemp snd_timer hid joydev kvm_intel cfg80211 ath3k kvm btusb bluetooth serio_raw snd microcode soundcore lpc_ich wmi mac_hid parport_pc ppdev lp parport psmouse ahci libahci
> [ 1524.757825] CPU: 3 PID: 1911 Comm: intel-gpu-overl Tainted: G W OE 3.15.0-rc3+ #96
> [ 1524.757840] Hardware name: Dell Inc. Inspiron 1090/Inspiron 1090, BIOS A06 08/23/2011
> [ 1524.757855] task: f52f36c0 ti: f4cbc000 task.ti: f4cbc000
> [ 1524.757869] EIP: 0060:[<c1406982>] EFLAGS: 00210202 CPU: 3
> [ 1524.757884] EIP is at per_file_stats+0x12/0x100
> [ 1524.757896] EAX: 0000002d EBX: 00000000 ECX: f4cbdefc EDX: f53af700
> [ 1524.757909] ESI: c1406970 EDI: f53af700 EBP: f4cbde6c ESP: f4cbde5c
> [ 1524.757922] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
> [ 1524.757934] CR0: 80050033 CR2: f53af748 CR3: 356af000 CR4: 000007f0
> [ 1524.757945] Stack:
> [ 1524.757957] f4cbdefc 00000000 c1406970 f53af700 f4cbdea8 c12e5f15 f4cbdefc c1406970
> [ 1524.757993] 0000ffff f4cbde90 0000002d f5dc5cd0 e4e80438 c1181d59 f4cbded8 f4d89900
> [ 1524.758027] f5631b40 e5131074 c1903f37 f4cbdf28 c14068e6 f52648a0 c1927748 c1903f37
> [ 1524.758062] Call Trace:
> [ 1524.758084] [<c1406970>] ? i915_gem_object_info+0x510/0x510
> [ 1524.758106] [<c12e5f15>] idr_for_each+0xa5/0x100
> [ 1524.758126] [<c1406970>] ? i915_gem_object_info+0x510/0x510
> [ 1524.758148] [<c1181d59>] ? seq_vprintf+0x29/0x50
> [ 1524.758168] [<c14068e6>] i915_gem_object_info+0x486/0x510
> [ 1524.758189] [<c11823a6>] seq_read+0xd6/0x380
> [ 1524.758208] [<c116d11d>] ? final_putname+0x1d/0x40
> [ 1524.758227] [<c11822d0>] ? seq_hlist_next_percpu+0x90/0x90
> [ 1524.758246] [<c1163e52>] vfs_read+0x82/0x150
> [ 1524.758265] [<c11645d6>] SyS_read+0x46/0x90
> [ 1524.758285] [<c16b8d8c>] sysenter_do_call+0x12/0x22
> [ 1524.758298] Code: f5 8f 2a 00 83 c4 6c 31 c0 5b 5e 5f 5d c3 8d 74 26 00 8d bc 27 00 00 00 00 55 89 e5 57 56 53 83 ec 04 3e 8d 74 26 00 83 41 04 01 <8b> 42 48 01 41 08 8b 42 4c 89 d7 85 c0 75 07 8b 42 60 85 c0 74
> [ 1524.758461] EIP: [<c1406982>] per_file_stats+0x12/0x100 SS:ESP 0068:f4cbde5c
> [ 1524.758485] CR2: 00000000f53af748
>
> Reported-by: Sam Jansen <sam.jansen@starleaf.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Sam Jansen <sam.jansen@starleaf.com>
> Cc: stable@vger.kernel.org
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index a8b81407338a..d2610a5f4efe 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -446,7 +446,9 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
>
> memset(&stats, 0, sizeof(stats));
> stats.file_priv = file->driver_priv;
> + spin_lock(&file->table_lock);
> idr_for_each(&file->object_idr, per_file_stats, &stats);
> + spin_unlock(&file->table_lock);
> /*
> * Although we have a valid reference on file->pid, that does
> * not guarantee that the task_struct who called get_pid() is
> --
> 2.0.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Hold the table lock whilst walking the file's idr and counting the objects in debugfs
2014-06-17 9:35 ` Daniel Vetter
@ 2014-06-24 13:18 ` Jani Nikula
0 siblings, 0 replies; 3+ messages in thread
From: Jani Nikula @ 2014-06-24 13:18 UTC (permalink / raw)
To: Daniel Vetter, Chris Wilson; +Cc: Sam Jansen, intel-gfx, stable
On Tue, 17 Jun 2014, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Tue, Jun 17, 2014 at 09:56:24AM +0100, Chris Wilson wrote:
>> Fixes an issue whereby we may race with the table updates (before the
>> core takes the struct_mutex) and so risk dereferencing a stale pointer in
>> the iterator for /debugfs/.../i915_gem_objects. For example,
>>
>> [ 1524.757545] BUG: unable to handle kernel paging request at f53af748
>> [ 1524.757572] IP: [<c1406982>] per_file_stats+0x12/0x100
>> [ 1524.757599] *pdpt = 0000000001b13001 *pde = 00000000379fb067 *pte = 80000000353af060
>> [ 1524.757621] Oops: 0000 [#1] SMP DEBUG_PAGEALLOC
>> [ 1524.757637] Modules linked in: ctr ccm arc4 ath9k ath9k_common ath9k_hw ath snd_hda_codec_conexant mac80211 snd_hda_codec_generic snd_hda_intel snd_hda_controller snd_hda_codec bnep snd_hwdep rfcomm snd_pcm gpio_ich dell_wmi sparse_keymap snd_seq_midi hid_multitouch uvcvideo snd_seq_midi_event dell_laptop snd_rawmidi dcdbas snd_seq videobuf2_vmalloc videobuf2_memops videobuf2_core usbhid videodev snd_seq_device coretemp snd_timer hid joydev kvm_intel cfg80211 ath3k kvm btusb bluetooth serio_raw snd microcode soundcore lpc_ich wmi mac_hid parport_pc ppdev lp parport psmouse ahci libahci
>> [ 1524.757825] CPU: 3 PID: 1911 Comm: intel-gpu-overl Tainted: G W OE 3.15.0-rc3+ #96
>> [ 1524.757840] Hardware name: Dell Inc. Inspiron 1090/Inspiron 1090, BIOS A06 08/23/2011
>> [ 1524.757855] task: f52f36c0 ti: f4cbc000 task.ti: f4cbc000
>> [ 1524.757869] EIP: 0060:[<c1406982>] EFLAGS: 00210202 CPU: 3
>> [ 1524.757884] EIP is at per_file_stats+0x12/0x100
>> [ 1524.757896] EAX: 0000002d EBX: 00000000 ECX: f4cbdefc EDX: f53af700
>> [ 1524.757909] ESI: c1406970 EDI: f53af700 EBP: f4cbde6c ESP: f4cbde5c
>> [ 1524.757922] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
>> [ 1524.757934] CR0: 80050033 CR2: f53af748 CR3: 356af000 CR4: 000007f0
>> [ 1524.757945] Stack:
>> [ 1524.757957] f4cbdefc 00000000 c1406970 f53af700 f4cbdea8 c12e5f15 f4cbdefc c1406970
>> [ 1524.757993] 0000ffff f4cbde90 0000002d f5dc5cd0 e4e80438 c1181d59 f4cbded8 f4d89900
>> [ 1524.758027] f5631b40 e5131074 c1903f37 f4cbdf28 c14068e6 f52648a0 c1927748 c1903f37
>> [ 1524.758062] Call Trace:
>> [ 1524.758084] [<c1406970>] ? i915_gem_object_info+0x510/0x510
>> [ 1524.758106] [<c12e5f15>] idr_for_each+0xa5/0x100
>> [ 1524.758126] [<c1406970>] ? i915_gem_object_info+0x510/0x510
>> [ 1524.758148] [<c1181d59>] ? seq_vprintf+0x29/0x50
>> [ 1524.758168] [<c14068e6>] i915_gem_object_info+0x486/0x510
>> [ 1524.758189] [<c11823a6>] seq_read+0xd6/0x380
>> [ 1524.758208] [<c116d11d>] ? final_putname+0x1d/0x40
>> [ 1524.758227] [<c11822d0>] ? seq_hlist_next_percpu+0x90/0x90
>> [ 1524.758246] [<c1163e52>] vfs_read+0x82/0x150
>> [ 1524.758265] [<c11645d6>] SyS_read+0x46/0x90
>> [ 1524.758285] [<c16b8d8c>] sysenter_do_call+0x12/0x22
>> [ 1524.758298] Code: f5 8f 2a 00 83 c4 6c 31 c0 5b 5e 5f 5d c3 8d 74 26 00 8d bc 27 00 00 00 00 55 89 e5 57 56 53 83 ec 04 3e 8d 74 26 00 83 41 04 01 <8b> 42 48 01 41 08 8b 42 4c 89 d7 85 c0 75 07 8b 42 60 85 c0 74
>> [ 1524.758461] EIP: [<c1406982>] per_file_stats+0x12/0x100 SS:ESP 0068:f4cbde5c
>> [ 1524.758485] CR2: 00000000f53af748
>>
>> Reported-by: Sam Jansen <sam.jansen@starleaf.com>
>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Sam Jansen <sam.jansen@starleaf.com>
>> Cc: stable@vger.kernel.org
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Pushed to -fixes, thanks for the patch and review.
BR,
Jani.
>> ---
>> drivers/gpu/drm/i915/i915_debugfs.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>> index a8b81407338a..d2610a5f4efe 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -446,7 +446,9 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
>>
>> memset(&stats, 0, sizeof(stats));
>> stats.file_priv = file->driver_priv;
>> + spin_lock(&file->table_lock);
>> idr_for_each(&file->object_idr, per_file_stats, &stats);
>> + spin_unlock(&file->table_lock);
>> /*
>> * Although we have a valid reference on file->pid, that does
>> * not guarantee that the task_struct who called get_pid() is
>> --
>> 2.0.0
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-06-24 13:18 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-06-17 8:56 [PATCH] drm/i915: Hold the table lock whilst walking the file's idr and counting the objects in debugfs Chris Wilson
2014-06-17 9:35 ` Daniel Vetter
2014-06-24 13:18 ` [Intel-gfx] " Jani Nikula
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox