Re: [PATCH] perf/core: Avoid freeing static PMU contexts when PMU is unregistered

[Kim Phillips]

On Tue, 3 Oct 2017 15:58:13 +0100
Mark Rutland <mark.rutland@xxxxxxx> wrote:

> On Tue, Oct 03, 2017 at 03:20:50PM +0100, Will Deacon wrote:
> > Since commit 1fd7e4169954 ("perf/core: Remove perf_cpu_context::unique_pmu"),
> > when a PMU is unregistered then its associated ->pmu_cpu_context is
> > unconditionally freed. Whilst this is fine for dynamically allocated
> > context types (i.e. those registered using perf_invalid_context), this
> > causes a problem for sharing of static contexts such as
> > perf_{sw,hw}_context, which are used by multiple built-in PMUs and
> > effectively have a global lifetime.
> > 
> > Whilst testing the ARM SPE driver, which must use perf_sw_context to
> > support per-task AUX tracing, unregistering the driver as a result of a
> > module unload resulted in:
> > 
> >  Unable to handle kernel NULL pointer dereference at virtual address 00000038
> >  Internal error: Oops: 96000004 [#1] PREEMPT SMP
> >  Modules linked in: [last unloaded: arm_spe_pmu]
> >  PC is at ctx_resched+0x38/0xe8
> >  LR is at perf_event_exec+0x20c/0x278
> >  [...]
> >  ctx_resched+0x38/0xe8
> >  perf_event_exec+0x20c/0x278
> >  setup_new_exec+0x88/0x118
> >  load_elf_binary+0x26c/0x109c
> >  search_binary_handler+0x90/0x298
> >  do_execveat_common.isra.14+0x540/0x618
> >  SyS_execve+0x38/0x48
> > 
> > since the software context has been freed and the ctx.pmu->pmu_disable_count
> > field has been set to NULL.
> > 
> > This patch fixes the problem by avoiding the freeing of static PMU contexts
> > altogether. Whilst the sharing of dynamic contexts is questionable, this
> > actually requires the caller to share their context pointer explicitly
> > and so the burden is on them to manage the object lifetime.
> > 
> > Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> > Fixes: 1fd7e4169954 ("perf/core: Remove perf_cpu_context::unique_pmu")
> > Reported-by: Kim Phillips <kim.phillips@xxxxxxx>
> > Signed-off-by: Will Deacon <will.deacon@xxxxxxx>
> 
> Acked-by: Mark Rutland <mark.rutland@xxxxxxx>

Tested-by: Kim Phillips <kim.phillips@xxxxxxx>

Thanks,

Kim