Re: [PATCH] hw_breakpoint: Fix Oops at destroying hw_breakpoint event on powerpc

2016-03-02 Thread Peter Zijlstra 
On Wed, Mar 02, 2016 at 03:25:17PM +0530, Ravi Bangoria wrote:
> At a time of destroying hw_breakpoint event, kernel ends up with Oops.
> Here is the sample output from 4.5.0-rc6 kernel.

> Call chain:
> 
>   hw_breakpoint_event_init()
> bp->destroy = bp_perf_event_destroy;
> 
>   do_exit()
> perf_event_exit_task()
>   perf_event_exit_task_context()
> WRITE_ONCE(child_ctx->task, TASK_TOMBSTONE);
> perf_event_exit_event()
>   free_event()
> _free_event()
>   bp_perf_event_destroy()//event->destroy(event);
> release_bp_slot()
>   arch_unregister_hw_breakpoint()
> 
> perf_event_exit_task_context sets child_ctx->task as TASK_TOMBSTONE
> which is (void *)-1. arch_unregister_hw_breakpoint tries to fetch
> 'thread' attribute of 'task' resulting in Oops.
> 
> This patch adds one more condition before accessing data from 'task'.
> 
> Signed-off-by: Ravi Bangoria 
> ---
>  arch/powerpc/kernel/hw_breakpoint.c | 3 ++-
>  include/linux/perf_event.h  | 2 ++
>  kernel/events/core.c| 2 --
>  3 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/hw_breakpoint.c 
> b/arch/powerpc/kernel/hw_breakpoint.c
> index 05e804c..43d8496 100644
> --- a/arch/powerpc/kernel/hw_breakpoint.c
> +++ b/arch/powerpc/kernel/hw_breakpoint.c
> @@ -29,6 +29,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -110,7 +111,7 @@ void arch_unregister_hw_breakpoint(struct perf_event *bp)
>* and the single_step_dabr_instruction(), then cleanup the breakpoint
>* restoration variables to prevent dangling pointers.
>*/
> - if (bp->ctx && bp->ctx->task)
> + if (bp->ctx && bp->ctx->task && bp->ctx->task != TASK_TOMBSTONE)
>   bp->ctx->task->thread.last_hit_ubp = NULL;
>  }

Yuck.. you should not be touching ctx (esp not this late).

And I suppose you cannot use bp->hw.target either because we don't
actually keep that up-to-date.

Why do you keep per task state anyway? What do per-cpu breakpoints do?

Re: [PATCH] hw_breakpoint: Fix Oops at destroying hw_breakpoint event on powerpc

2016-03-02 Thread Peter Zijlstra 
On Wed, Mar 02, 2016 at 03:25:17PM +0530, Ravi Bangoria wrote:
> At a time of destroying hw_breakpoint event, kernel ends up with Oops.
> Here is the sample output from 4.5.0-rc6 kernel.

> Call chain:
> 
>   hw_breakpoint_event_init()
> bp->destroy = bp_perf_event_destroy;
> 
>   do_exit()
> perf_event_exit_task()
>   perf_event_exit_task_context()
> WRITE_ONCE(child_ctx->task, TASK_TOMBSTONE);
> perf_event_exit_event()
>   free_event()
> _free_event()
>   bp_perf_event_destroy()//event->destroy(event);
> release_bp_slot()
>   arch_unregister_hw_breakpoint()
> 
> perf_event_exit_task_context sets child_ctx->task as TASK_TOMBSTONE
> which is (void *)-1. arch_unregister_hw_breakpoint tries to fetch
> 'thread' attribute of 'task' resulting in Oops.
> 
> This patch adds one more condition before accessing data from 'task'.
> 
> Signed-off-by: Ravi Bangoria 
> ---
>  arch/powerpc/kernel/hw_breakpoint.c | 3 ++-
>  include/linux/perf_event.h  | 2 ++
>  kernel/events/core.c| 2 --
>  3 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/hw_breakpoint.c 
> b/arch/powerpc/kernel/hw_breakpoint.c
> index 05e804c..43d8496 100644
> --- a/arch/powerpc/kernel/hw_breakpoint.c
> +++ b/arch/powerpc/kernel/hw_breakpoint.c
> @@ -29,6 +29,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -110,7 +111,7 @@ void arch_unregister_hw_breakpoint(struct perf_event *bp)
>* and the single_step_dabr_instruction(), then cleanup the breakpoint
>* restoration variables to prevent dangling pointers.
>*/
> - if (bp->ctx && bp->ctx->task)
> + if (bp->ctx && bp->ctx->task && bp->ctx->task != TASK_TOMBSTONE)
>   bp->ctx->task->thread.last_hit_ubp = NULL;
>  }

Yuck.. you should not be touching ctx (esp not this late).

And I suppose you cannot use bp->hw.target either because we don't
actually keep that up-to-date.

Why do you keep per task state anyway? What do per-cpu breakpoints do?

[PATCH] hw_breakpoint: Fix Oops at destroying hw_breakpoint event on powerpc

2016-03-02 Thread Ravi Bangoria 
At a time of destroying hw_breakpoint event, kernel ends up with Oops.
Here is the sample output from 4.5.0-rc6 kernel.

  [  450.708568] Unable to handle kernel paging request for data at address 
0x0c07
  [  450.708684] Faulting instruction address: 0xc00291d0
  [  450.708750] Oops: Kernel access of bad area, sig: 11 [#1]
  [  450.708798] SMP NR_CPUS=1024 NUMA pSeries
  [  450.708856] Modules linked in: 
stap_4c2bdcf3e1aee79b646bb9a844e600f7__4962(O) xt_CHECKSUM ...
  [  450.709539] CPU: 5 PID: 5106 Comm: perf_fuzzer Tainted: G   O
4.5.0-rc5+ #1
  [  450.709620] task: c000f8795c80 ti: c000e334 task.ti: 
c000e334
  [  450.709691] NIP: c00291d0 LR: c020b6b4 CTR: 
c020b6f0
  [  450.709760] REGS: c000e3343760 TRAP: 0300   Tainted: G   O 
(4.5.0-rc5+)
  [  450.709831] MSR: 80009033   CR: 22008828  
XER: 2000
  [  450.710001] CFAR: c0010708 DAR: 0c07 DSISR: 4200 
SOFTE: 1
  GPR00: c020b6b4 c000e33439e0 c1350900 c0009efa7000
  GPR04: 0001 c0009efa7000  0001
  GPR08:    
  GPR12: c020b6f0 c7e02800 c0009efa5208 
  GPR16: 0001   c000f3ad7f10
  GPR20: c000f87964c8 0001 c000f8795c80 fffd
  GPR24:  c000f3ad7f08 c000f3ad7f68 c0009efa6800
  GPR28: c000f3ad7f00 c0009efa5000 c1259520 c0009efa7000
  [  450.710996] NIP [c00291d0] arch_unregister_hw_breakpoint+0x40/0x60
  [  450.711066] LR [c020b6b4] release_bp_slot+0x44/0x80
  [  450.77] Call Trace:
  [  450.711165] [c000e33439e0] [c09c1e38] mutex_lock+0x28/0x70 
(unreliable)
  [  450.711257] [c000e3343a10] [c020b6b4] release_bp_slot+0x44/0x80
  [  450.711332] [c000e3343a40] [c02036c8] _free_event+0xd8/0x350
  [  450.711404] [c000e3343a70] [c0208260] 
perf_event_exit_task+0x2b0/0x4c0
  [  450.711490] [c000e3343b20] [c00b8ac8] do_exit+0x388/0xc60
  [  450.711563] [c000e3343be0] [c00b9484] do_group_exit+0x64/0x100
  [  450.711641] [c000e3343c20] [c00c9100] get_signal+0x220/0x770
  [  450.711716] [c000e3343d10] [c0017884] do_signal+0x54/0x2b0
  [  450.711793] [c000e3343e00] [c0017cac] 
do_notify_resume+0xbc/0xd0
  [  450.711865] [c000e3343e30] [c0009838] 
ret_from_except_lite+0x64/0x68
  [  450.711948] Instruction dump:
  [  450.711986] f8010010 f821ffd1 7c7f1b78 6000 6000 e93f01e8 2fa9 
419e0018
  [  450.712107] e9290098 2fa9 419e000c 3940  38210030 
e8010010 ebe1fff8
  [  450.712230] ---[ end trace 3cf087de955e9358 ]---

Call chain:

  hw_breakpoint_event_init()
bp->destroy = bp_perf_event_destroy;

  do_exit()
perf_event_exit_task()
  perf_event_exit_task_context()
WRITE_ONCE(child_ctx->task, TASK_TOMBSTONE);
perf_event_exit_event()
  free_event()
_free_event()
  bp_perf_event_destroy()//event->destroy(event);
release_bp_slot()
  arch_unregister_hw_breakpoint()

perf_event_exit_task_context sets child_ctx->task as TASK_TOMBSTONE
which is (void *)-1. arch_unregister_hw_breakpoint tries to fetch
'thread' attribute of 'task' resulting in Oops.

This patch adds one more condition before accessing data from 'task'.

Signed-off-by: Ravi Bangoria 
---
 arch/powerpc/kernel/hw_breakpoint.c | 3 ++-
 include/linux/perf_event.h  | 2 ++
 kernel/events/core.c| 2 --
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/hw_breakpoint.c 
b/arch/powerpc/kernel/hw_breakpoint.c
index 05e804c..43d8496 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -29,6 +29,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -110,7 +111,7 @@ void arch_unregister_hw_breakpoint(struct perf_event *bp)
 * and the single_step_dabr_instruction(), then cleanup the breakpoint
 * restoration variables to prevent dangling pointers.
 */
-   if (bp->ctx && bp->ctx->task)
+   if (bp->ctx && bp->ctx->task && bp->ctx->task != TASK_TOMBSTONE)
bp->ctx->task->thread.last_hit_ubp = NULL;
 }
 
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index f5c5a3f..491c50e 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -1192,4 +1192,6 @@ _name##_show(struct device *dev,  
\
\
 static struct device_attribute format_attr_##_name = __ATTR_RO(_name)
 
+#define TASK_TOMBSTONE ((void *)-1L)
+
 #endif /*

[PATCH] hw_breakpoint: Fix Oops at destroying hw_breakpoint event on powerpc

2016-03-02 Thread Ravi Bangoria 
At a time of destroying hw_breakpoint event, kernel ends up with Oops.
Here is the sample output from 4.5.0-rc6 kernel.

  [  450.708568] Unable to handle kernel paging request for data at address 
0x0c07
  [  450.708684] Faulting instruction address: 0xc00291d0
  [  450.708750] Oops: Kernel access of bad area, sig: 11 [#1]
  [  450.708798] SMP NR_CPUS=1024 NUMA pSeries
  [  450.708856] Modules linked in: 
stap_4c2bdcf3e1aee79b646bb9a844e600f7__4962(O) xt_CHECKSUM ...
  [  450.709539] CPU: 5 PID: 5106 Comm: perf_fuzzer Tainted: G   O
4.5.0-rc5+ #1
  [  450.709620] task: c000f8795c80 ti: c000e334 task.ti: 
c000e334
  [  450.709691] NIP: c00291d0 LR: c020b6b4 CTR: 
c020b6f0
  [  450.709760] REGS: c000e3343760 TRAP: 0300   Tainted: G   O 
(4.5.0-rc5+)
  [  450.709831] MSR: 80009033   CR: 22008828  
XER: 2000
  [  450.710001] CFAR: c0010708 DAR: 0c07 DSISR: 4200 
SOFTE: 1
  GPR00: c020b6b4 c000e33439e0 c1350900 c0009efa7000
  GPR04: 0001 c0009efa7000  0001
  GPR08:    
  GPR12: c020b6f0 c7e02800 c0009efa5208 
  GPR16: 0001   c000f3ad7f10
  GPR20: c000f87964c8 0001 c000f8795c80 fffd
  GPR24:  c000f3ad7f08 c000f3ad7f68 c0009efa6800
  GPR28: c000f3ad7f00 c0009efa5000 c1259520 c0009efa7000
  [  450.710996] NIP [c00291d0] arch_unregister_hw_breakpoint+0x40/0x60
  [  450.711066] LR [c020b6b4] release_bp_slot+0x44/0x80
  [  450.77] Call Trace:
  [  450.711165] [c000e33439e0] [c09c1e38] mutex_lock+0x28/0x70 
(unreliable)
  [  450.711257] [c000e3343a10] [c020b6b4] release_bp_slot+0x44/0x80
  [  450.711332] [c000e3343a40] [c02036c8] _free_event+0xd8/0x350
  [  450.711404] [c000e3343a70] [c0208260] 
perf_event_exit_task+0x2b0/0x4c0
  [  450.711490] [c000e3343b20] [c00b8ac8] do_exit+0x388/0xc60
  [  450.711563] [c000e3343be0] [c00b9484] do_group_exit+0x64/0x100
  [  450.711641] [c000e3343c20] [c00c9100] get_signal+0x220/0x770
  [  450.711716] [c000e3343d10] [c0017884] do_signal+0x54/0x2b0
  [  450.711793] [c000e3343e00] [c0017cac] 
do_notify_resume+0xbc/0xd0
  [  450.711865] [c000e3343e30] [c0009838] 
ret_from_except_lite+0x64/0x68
  [  450.711948] Instruction dump:
  [  450.711986] f8010010 f821ffd1 7c7f1b78 6000 6000 e93f01e8 2fa9 
419e0018
  [  450.712107] e9290098 2fa9 419e000c 3940  38210030 
e8010010 ebe1fff8
  [  450.712230] ---[ end trace 3cf087de955e9358 ]---

Call chain:

  hw_breakpoint_event_init()
bp->destroy = bp_perf_event_destroy;

  do_exit()
perf_event_exit_task()
  perf_event_exit_task_context()
WRITE_ONCE(child_ctx->task, TASK_TOMBSTONE);
perf_event_exit_event()
  free_event()
_free_event()
  bp_perf_event_destroy()//event->destroy(event);
release_bp_slot()
  arch_unregister_hw_breakpoint()

perf_event_exit_task_context sets child_ctx->task as TASK_TOMBSTONE
which is (void *)-1. arch_unregister_hw_breakpoint tries to fetch
'thread' attribute of 'task' resulting in Oops.

This patch adds one more condition before accessing data from 'task'.

Signed-off-by: Ravi Bangoria 
---
 arch/powerpc/kernel/hw_breakpoint.c | 3 ++-
 include/linux/perf_event.h  | 2 ++
 kernel/events/core.c| 2 --
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/hw_breakpoint.c 
b/arch/powerpc/kernel/hw_breakpoint.c
index 05e804c..43d8496 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -29,6 +29,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -110,7 +111,7 @@ void arch_unregister_hw_breakpoint(struct perf_event *bp)
 * and the single_step_dabr_instruction(), then cleanup the breakpoint
 * restoration variables to prevent dangling pointers.
 */
-   if (bp->ctx && bp->ctx->task)
+   if (bp->ctx && bp->ctx->task && bp->ctx->task != TASK_TOMBSTONE)
bp->ctx->task->thread.last_hit_ubp = NULL;
 }
 
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index f5c5a3f..491c50e 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -1192,4 +1192,6 @@ _name##_show(struct device *dev,  
\
\
 static struct device_attribute format_attr_##_name = __ATTR_RO(_name)
 
+#define TASK_TOMBSTONE ((void *)-1L)
+
 #endif /* _LINUX_PERF_EVENT_H */
diff --git a/kernel/events/core.c