On Tue, 3 Oct 2017 15:58:13 +0100 Mark Rutland <mark.rutl...@arm.com> 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 <pet...@infradead.org> > > Fixes: 1fd7e4169954 ("perf/core: Remove perf_cpu_context::unique_pmu") > > Reported-by: Kim Phillips <kim.phill...@arm.com> > > Signed-off-by: Will Deacon <will.dea...@arm.com> > > Acked-by: Mark Rutland <mark.rutl...@arm.com> Tested-by: Kim Phillips <kim.phill...@arm.com> Thanks, Kim