* perf NULL pointer dereference on -rc5
@ 2011-12-13 15:26 Will Deacon
2011-12-13 19:48 ` Peter Zijlstra
2011-12-14 10:21 ` [tip:perf/urgent] perf events: Fix ring_buffer_wakeup() brown paperbag bug tip-bot for Will Deacon
0 siblings, 2 replies; 5+ messages in thread
From: Will Deacon @ 2011-12-13 15:26 UTC (permalink / raw)
To: a.p.zijlstra; +Cc: eranian, mingo, linux-kernel
Hi Peter,
Commit 10c6db11 ("perf: Fix loss of notification with multi-event") seems to
dereference a NULL event->rb in the wakeup handler during Vince Weaver's perf
tests (specifically corner_cases/overflow_requires_mmap).
This diff seems to fix the problem, but I'm not sure if it just hides something else:
diff --git a/kernel/events/core.c b/kernel/events/core.c
index d3b9df5..b466e7fc 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -3558,9 +3558,13 @@ static void ring_buffer_wakeup(struct perf_event *event)
rcu_read_lock();
rb = rcu_dereference(event->rb);
+ if (!rb)
+ goto unlock;
+
list_for_each_entry_rcu(event, &rb->event_list, rb_entry) {
wake_up_all(&event->waitq);
}
+unlock:
rcu_read_unlock();
}
Log follows...
Cheers,
Will
[ 77.705045] Unable to handle kernel NULL pointer dereference at virtual address 0000004c
[ 77.732457] pgd = ef254000
[ 77.740547] [0000004c] *pgd=9f81f831
[ 77.751258] Internal error: Oops: 17 [#1] PREEMPT SMP
[ 77.766382] Modules linked in:
[ 77.775527] CPU: 0 Tainted: G W (3.2.0-rc5 #5)
[ 77.792491] PC is at perf_event_wakeup+0x18/0x88
[ 77.806315] LR is at perf_event_wakeup+0x10/0x88
[ 77.820143] pc : [<c007fbec>] lr : [<c007fbe4>] psr: 20000193
[ 77.820153] sp : ef271e80 ip : 00000007 fp : 00000118
[ 77.854552] r10: ef270000 r9 : 00000000 r8 : 00000001
[ 77.870199] r7 : ef357800 r6 : ef271e88 r5 : 00000000 r4 : ef3579e0
[ 77.889753] r3 : ef8cd6a0 r2 : 00000001 r1 : ef270000 r0 : ef357800
[ 77.909310] Flags: nzCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment user
[ 77.930949] Control: 10c5387d Table: 8f25404a DAC: 00000015
[ 77.948159] Process overflow_requir (pid: 3092, stack limit = 0xef2702f8)
[ 77.968495] Stack: (0xef271e80 to 0xef272000)
[ 77.981545] 1e80: 00000001 00000000 000f4071 ef3579e0 00000000 00000000 00000002 c007b30c
[ 78.006056] 1ea0: c0401da8 00000007 00000000 c0418d08 c0d3d160 c00168a8 00000002 ef3578c8
[ 78.030568] 1ec0: 00000100 ef271fb0 00000001 00000000 00008a5a 00000000 ef357000 ef82f700
[ 78.055077] 1ee0: ef821000 00000000 00000000 00000000 c03fe2b4 c01ec0a0 ef357000 c03fe080
[ 78.079588] 1f00: ef402c60 40000193 000f4240 00000000 ef1b9dc0 00000000 00000000 ef1c1640
[ 78.104098] 1f20: ef80a654 00000000 00000000 0000005c c0424198 ef80a600 00000001 c006ea94
[ 78.128608] 1f40: 00000001 00000c14 ef1b9140 ef80a600 ef80a654 ef1c1640 0000005c 00000000
[ 78.153118] 1f60: 00000004 000f4240 00000000 c006ebfc ef80a600 ef80a654 00000001 c0071500
[ 78.177628] 1f80: 0000005c c03f9c4c ef270000 c006e3e0 c03feb30 c000e9e4 00008a5a 20000030
[ 78.202138] 1fa0: f8e00100 00000003 00000001 c000deb4 00000000 00002400 00938462 00000000
[ 78.226648] 1fc0: becc46a0 00000000 000121f8 00000003 00000001 00000004 000f4240 00000000
[ 78.251159] 1fe0: 0001203c becc45c0 00008a4d 00008a5a 20000030 ffffffff 8f538c6b 2493cd43
[ 78.275697] [<c007fbec>] (perf_event_wakeup+0x18/0x88) from [<c007b30c>] (irq_work_run+0x90/0xc4)
[ 78.302314] [<c007b30c>] (irq_work_run+0x90/0xc4) from [<c00168a8>] (armv7pmu_handle_irq+0x104/0x17c)
[ 78.329970] [<c00168a8>] (armv7pmu_handle_irq+0x104/0x17c) from [<c006ea94>] (handle_irq_event_percpu+0x54/0x180)
[ 78.360741] [<c006ea94>] (handle_irq_event_percpu+0x54/0x180) from [<c006ebfc>] (handle_irq_event+0x3c/0x5c)
[ 78.390213] [<c006ebfc>] (handle_irq_event+0x3c/0x5c) from [<c0071500>] (handle_fasteoi_irq+0x9c/0x140)
[ 78.418379] [<c0071500>] (handle_fasteoi_irq+0x9c/0x140) from [<c006e3e0>] (generic_handle_irq+0x20/0x30)
[ 78.447078] [<c006e3e0>] (generic_handle_irq+0x20/0x30) from [<c000e9e4>] (handle_IRQ+0x58/0xac)
[ 78.473420] [<c000e9e4>] (handle_IRQ+0x58/0xac) from [<c000deb4>] (__irq_usr+0x34/0xa0)
[ 78.497411] Code: e24dd00c ebffcdef e59751b4 e28d6008 (e5b5304c)
[ 78.515665] ---[ end trace 1b75b31a2719ed1e ]---
[ 78.529490] Kernel panic - not syncing: Fatal exception in interrupt
[ 78.548543] [<c0013e7c>] (unwind_backtrace+0x0/0xf8) from [<c02e6cc0>] (panic+0x7c/0x1bc)
[ 78.573069] [<c02e6cc0>] (panic+0x7c/0x1bc) from [<c00118a8>] (die+0x1f4/0x1f8)
[ 78.594987] [<c00118a8>] (die+0x1f4/0x1f8) from [<c0017f60>] (__do_kernel_fault+0x74/0x84)
[ 78.619766] [<c0017f60>] (__do_kernel_fault+0x74/0x84) from [<c001810c>] (do_page_fault+0x19c/0x2f0)
[ 78.647149] [<c001810c>] (do_page_fault+0x19c/0x2f0) from [<c00083f0>] (do_DataAbort+0x34/0x9c)
[ 78.673228] [<c00083f0>] (do_DataAbort+0x34/0x9c) from [<c000dc58>] (__dabt_svc+0x38/0x60)
[ 78.697992] Exception stack(0xef271e38 to 0xef271e80)
[ 78.713119] 1e20: ef357800 ef270000
[ 78.737630] 1e40: 00000001 ef8cd6a0 ef3579e0 00000000 ef271e88 ef357800 00000001 00000000
[ 78.762141] 1e60: ef270000 00000118 00000007 ef271e80 c007fbe4 c007fbec 20000193 ffffffff
[ 78.786658] [<c000dc58>] (__dabt_svc+0x38/0x60) from [<c007fbec>] (perf_event_wakeup+0x18/0x88)
[ 78.812739] [<c007fbec>] (perf_event_wakeup+0x18/0x88) from [<c007b30c>] (irq_work_run+0x90/0xc4)
[ 78.839341] [<c007b30c>] (irq_work_run+0x90/0xc4) from [<c00168a8>] (armv7pmu_handle_irq+0x104/0x17c)
[ 78.866986] [<c00168a8>] (armv7pmu_handle_irq+0x104/0x17c) from [<c006ea94>] (handle_irq_event_percpu+0x54/0x180)
[ 78.897754] [<c006ea94>] (handle_irq_event_percpu+0x54/0x180) from [<c006ebfc>] (handle_irq_event+0x3c/0x5c)
[ 78.927221] [<c006ebfc>] (handle_irq_event+0x3c/0x5c) from [<c0071500>] (handle_fasteoi_irq+0x9c/0x140)
[ 78.955385] [<c0071500>] (handle_fasteoi_irq+0x9c/0x140) from [<c006e3e0>] (generic_handle_irq+0x20/0x30)
[ 78.984071] [<c006e3e0>] (generic_handle_irq+0x20/0x30) from [<c000e9e4>] (handle_IRQ+0x58/0xac)
[ 79.010410] [<c000e9e4>] (handle_IRQ+0x58/0xac) from [<c000deb4>] (__irq_usr+0x34/0xa0)
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: perf NULL pointer dereference on -rc5
2011-12-13 15:26 perf NULL pointer dereference on -rc5 Will Deacon
@ 2011-12-13 19:48 ` Peter Zijlstra
2011-12-13 21:08 ` Will Deacon
2011-12-14 10:21 ` [tip:perf/urgent] perf events: Fix ring_buffer_wakeup() brown paperbag bug tip-bot for Will Deacon
1 sibling, 1 reply; 5+ messages in thread
From: Peter Zijlstra @ 2011-12-13 19:48 UTC (permalink / raw)
To: Will Deacon; +Cc: eranian, mingo, linux-kernel
On Tue, 2011-12-13 at 15:26 +0000, Will Deacon wrote:
>
> Commit 10c6db11 ("perf: Fix loss of notification with multi-event") seems to
> dereference a NULL event->rb in the wakeup handler during Vince Weaver's perf
> tests (specifically corner_cases/overflow_requires_mmap).
>
> This diff seems to fix the problem, but I'm not sure if it just hides something else:
No that is about right.. not so very good of us to have missed that.
Can I add your SoB to this?
---
Subject: perf: Fix ring_buffer_wakeup()
From: Will Deacon <will.deacon@arm.com>
Date: Tue Dec 13 20:40:45 CET 2011
Commit 10c6db11 ("perf: Fix loss of notification with multi-event")
seems to unconditionally dereference event->rb in the wakeup handler,
this is wrong, there might not be a buffer attached.
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/20111213152651.GP20297@mudshark.cambridge.arm.com
---
kernel/events/core.c | 3 +++
1 file changed, 3 insertions(+)
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -3360,9 +3360,12 @@ static void ring_buffer_wakeup(struct pe
rcu_read_lock();
rb = rcu_dereference(event->rb);
+ if (!rb)
+ goto unlock;
list_for_each_entry_rcu(event, &rb->event_list, rb_entry) {
wake_up_all(&event->waitq);
}
+unlock:
rcu_read_unlock();
}
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: perf NULL pointer dereference on -rc5
2011-12-13 19:48 ` Peter Zijlstra
@ 2011-12-13 21:08 ` Will Deacon
2011-12-13 21:23 ` Stephane Eranian
0 siblings, 1 reply; 5+ messages in thread
From: Will Deacon @ 2011-12-13 21:08 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: eranian, mingo, linux-kernel
On Tue, Dec 13, 2011 at 07:48:55PM +0000, Peter Zijlstra wrote:
> On Tue, 2011-12-13 at 15:26 +0000, Will Deacon wrote:
> >
> > Commit 10c6db11 ("perf: Fix loss of notification with multi-event") seems to
> > dereference a NULL event->rb in the wakeup handler during Vince Weaver's perf
> > tests (specifically corner_cases/overflow_requires_mmap).
> >
> > This diff seems to fix the problem, but I'm not sure if it just hides something else:
>
> No that is about right.. not so very good of us to have missed that.
Well, at least we caught it in the end.
> Can I add your SoB to this?
Sure:
Signed-off-by: Will Deacon <will.deacon@arm.com>
Will
> ---
> Subject: perf: Fix ring_buffer_wakeup()
> From: Will Deacon <will.deacon@arm.com>
> Date: Tue Dec 13 20:40:45 CET 2011
>
> Commit 10c6db11 ("perf: Fix loss of notification with multi-event")
> seems to unconditionally dereference event->rb in the wakeup handler,
> this is wrong, there might not be a buffer attached.
>
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Link: http://lkml.kernel.org/r/20111213152651.GP20297@mudshark.cambridge.arm.com
> ---
> kernel/events/core.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -3360,9 +3360,12 @@ static void ring_buffer_wakeup(struct pe
>
> rcu_read_lock();
> rb = rcu_dereference(event->rb);
> + if (!rb)
> + goto unlock;
> list_for_each_entry_rcu(event, &rb->event_list, rb_entry) {
> wake_up_all(&event->waitq);
> }
> +unlock:
> rcu_read_unlock();
> }
>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: perf NULL pointer dereference on -rc5
2011-12-13 21:08 ` Will Deacon
@ 2011-12-13 21:23 ` Stephane Eranian
0 siblings, 0 replies; 5+ messages in thread
From: Stephane Eranian @ 2011-12-13 21:23 UTC (permalink / raw)
To: Will Deacon; +Cc: Peter Zijlstra, mingo, linux-kernel
On Tue, Dec 13, 2011 at 1:08 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Tue, Dec 13, 2011 at 07:48:55PM +0000, Peter Zijlstra wrote:
>> On Tue, 2011-12-13 at 15:26 +0000, Will Deacon wrote:
>> >
>> > Commit 10c6db11 ("perf: Fix loss of notification with multi-event") seems to
>> > dereference a NULL event->rb in the wakeup handler during Vince Weaver's perf
>> > tests (specifically corner_cases/overflow_requires_mmap).
>> >
>> > This diff seems to fix the problem, but I'm not sure if it just hides something else:
>>
>> No that is about right.. not so very good of us to have missed that.
>
> Well, at least we caught it in the end.
>
>> Can I add your SoB to this?
>
> Sure:
>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
>
Thanks for fixing this. Somehow in my testing I never ran into this problem.
> Will
>
>> ---
>> Subject: perf: Fix ring_buffer_wakeup()
>> From: Will Deacon <will.deacon@arm.com>
>> Date: Tue Dec 13 20:40:45 CET 2011
>>
>> Commit 10c6db11 ("perf: Fix loss of notification with multi-event")
>> seems to unconditionally dereference event->rb in the wakeup handler,
>> this is wrong, there might not be a buffer attached.
>>
>> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
>> Link: http://lkml.kernel.org/r/20111213152651.GP20297@mudshark.cambridge.arm.com
>> ---
>> kernel/events/core.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> --- a/kernel/events/core.c
>> +++ b/kernel/events/core.c
>> @@ -3360,9 +3360,12 @@ static void ring_buffer_wakeup(struct pe
>>
>> rcu_read_lock();
>> rb = rcu_dereference(event->rb);
>> + if (!rb)
>> + goto unlock;
>> list_for_each_entry_rcu(event, &rb->event_list, rb_entry) {
>> wake_up_all(&event->waitq);
>> }
>> +unlock:
>> rcu_read_unlock();
>> }
>>
>>
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [tip:perf/urgent] perf events: Fix ring_buffer_wakeup() brown paperbag bug
2011-12-13 15:26 perf NULL pointer dereference on -rc5 Will Deacon
2011-12-13 19:48 ` Peter Zijlstra
@ 2011-12-14 10:21 ` tip-bot for Will Deacon
1 sibling, 0 replies; 5+ messages in thread
From: tip-bot for Will Deacon @ 2011-12-14 10:21 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, hpa, mingo, a.p.zijlstra, will.deacon, tglx, mingo
Commit-ID: 44b7f4b98d8877e2a4427f2a2f2e42ae8227a58f
Gitweb: http://git.kernel.org/tip/44b7f4b98d8877e2a4427f2a2f2e42ae8227a58f
Author: Will Deacon <will.deacon@arm.com>
AuthorDate: Tue, 13 Dec 2011 20:40:45 +0100
Committer: Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 14 Dec 2011 08:44:53 +0100
perf events: Fix ring_buffer_wakeup() brown paperbag bug
Commit 10c6db11 ("perf: Fix loss of notification with multi-event")
seems to unconditionally dereference event->rb in the wakeup handler,
this is wrong, there might not be a buffer attached.
Signed-off-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/20111213152651.GP20297@mudshark.cambridge.arm.com
[ minor edits ]
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
kernel/events/core.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index d3b9df5..58690af 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -3558,9 +3558,13 @@ static void ring_buffer_wakeup(struct perf_event *event)
rcu_read_lock();
rb = rcu_dereference(event->rb);
- list_for_each_entry_rcu(event, &rb->event_list, rb_entry) {
+ if (!rb)
+ goto unlock;
+
+ list_for_each_entry_rcu(event, &rb->event_list, rb_entry)
wake_up_all(&event->waitq);
- }
+
+unlock:
rcu_read_unlock();
}
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-12-14 10:22 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-12-13 15:26 perf NULL pointer dereference on -rc5 Will Deacon
2011-12-13 19:48 ` Peter Zijlstra
2011-12-13 21:08 ` Will Deacon
2011-12-13 21:23 ` Stephane Eranian
2011-12-14 10:21 ` [tip:perf/urgent] perf events: Fix ring_buffer_wakeup() brown paperbag bug tip-bot for Will Deacon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome