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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 4 Oct 2017 10:31:46 -0500
From:   Kim Phillips <kim.phillips@....com>
To:     Mark Rutland <mark.rutland@....com>
Cc:     Will Deacon <will.deacon@....com>, <mingo@...hat.com>,
        <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH] perf/core: Avoid freeing static PMU contexts when PMU
 is unregistered

On Tue, 3 Oct 2017 15:58:13 +0100
Mark Rutland <mark.rutland@....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 <peterz@...radead.org>
> > Fixes: 1fd7e4169954 ("perf/core: Remove perf_cpu_context::unique_pmu")
> > Reported-by: Kim Phillips <kim.phillips@....com>
> > Signed-off-by: Will Deacon <will.deacon@....com>
> 
> Acked-by: Mark Rutland <mark.rutland@....com>

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

Thanks,

Kim