From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1752538AbdJDPbt (ORCPT <rfc822;w@1wt.eu>);
        Wed, 4 Oct 2017 11:31:49 -0400
Received: from foss.arm.com ([217.140.101.70]:34666 "EHLO foss.arm.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1752458AbdJDPbs (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Wed, 4 Oct 2017 11:31:48 -0400
Date: Wed, 4 Oct 2017 10:31:46 -0500
From: Kim Phillips <kim.phillips@arm.com>
To: Mark Rutland <mark.rutland@arm.com>
Cc: Will Deacon <will.deacon@arm.com>, <mingo@redhat.com>,
        <linux-kernel@vger.kernel.org>, <linux-arm-kernel@lists.infradead.org>,
        Peter Zijlstra <peterz@infradead.org>
Subject: Re: [PATCH] perf/core: Avoid freeing static PMU contexts when PMU
 is unregistered
Message-Id: <20171004103146.43cd7185662d842ff11a5940@arm.com>
In-Reply-To: <20171003145813.GF4931@leverpostej>
References: <1507040450-7730-1-git-send-email-will.deacon@arm.com>
        <20171003145813.GF4931@leverpostej>
Organization: Arm
X-Mailer: Sylpheed 3.5.1 (GTK+ 2.24.31; x86_64-pc-linux-gnu)
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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

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

Thanks,

Kim