Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1347976

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

From Peter Zijlstra <peterz@infradead.org>
Newsgroups linux.kernel
Subject Re: [PATCH] hw_breakpoint: Fix Oops at destroying hw_breakpoint event on powerpc
Date 2016-03-02 12:50 +0100
Message-ID <r8drX-71N-11@gated-at.bofh.it> (permalink)
References <r8bJw-5SA-11@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


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 <ravi.bangoria@linux.vnet.ibm.com>
> ---
>  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 <linux/kernel.h>
>  #include <linux/sched.h>
>  #include <linux/smp.h>
> +#include <linux/perf_event.h>
>  
>  #include <asm/hw_breakpoint.h>
>  #include <asm/processor.h>
> @@ -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?

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH] hw_breakpoint: Fix Oops at destroying hw_breakpoint event on powerpc Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> - 2016-03-02 11:00 +0100
  Re: [PATCH] hw_breakpoint: Fix Oops at destroying hw_breakpoint  event on powerpc Peter Zijlstra <peterz@infradead.org> - 2016-03-02 12:50 +0100
  Re: hw_breakpoint: Fix Oops at destroying hw_breakpoint event on powerpc Michael Ellerman <mpe@ellerman.id.au> - 2016-03-02 13:00 +0100
    Re: hw_breakpoint: Fix Oops at destroying hw_breakpoint event on  powerpc Peter Zijlstra <peterz@infradead.org> - 2016-03-02 13:00 +0100
      Re: hw_breakpoint: Fix Oops at destroying hw_breakpoint event on  powerpc Michael Ellerman <mpe@ellerman.id.au> - 2016-03-03 10:30 +0100
        Re: hw_breakpoint: Fix Oops at destroying hw_breakpoint event on  powerpc Peter Zijlstra <peterz@infradead.org> - 2016-03-03 11:30 +0100
          Re: hw_breakpoint: Fix Oops at destroying hw_breakpoint event on  powerpc Michael Ellerman <mpe@ellerman.id.au> - 2016-03-04 09:50 +0100

csiph-web