[PATCHv2 0/3] [4.0] arm/arm64: Do not group hardware events from different PMUs

[PATCHv2 0/3] [4.0] arm/arm64: Do not group hardware events from different PMUs

[Suzuki K. Poulose]

From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>

This is a collection of fixes which denies grouping hardware events
from different PMUs. They also fix crashes triggered by perf_fuzzer
on Linux-4.0-rc2.

This series hasn't changed since V1, except for better commit messages.

Similar fixes are required in ARM-CCN PMU driver and is not straight forward
as these changes.

Suzuki K. Poulose (3):
  arm/pmu: Reject groups spanning multiple hardware PMUs
  arm64/pmu: Reject groups spanning multiple HW PMUs
  arm-cci: Reject groups spanning multiple HW PMUs

 arch/arm/kernel/perf_event.c   |   21 +++++++++++++++------
 arch/arm64/kernel/perf_event.c |   21 +++++++++++++++------
 drivers/bus/arm-cci.c          |   19 ++++++++++++++-----
 3 files changed, 44 insertions(+), 17 deletions(-)

-- 
1.7.9.5

[PATCH 1/3] arm/pmu: Reject groups spanning multiple hardware PMUs

[Suzuki K. Poulose]

From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>

The perf core implicitly rejects events spanning multiple HW PMUs, as in
these cases the event->ctx will differ. However this validation is
performed after pmu::event_init() is called in perf_init_event(), and
thus pmu::event_init() may be called with a group leader from a
different HW PMU.

The ARM PMU driver does not take this fact into account, and when
validating groups assumes that it can call to_arm_pmu(event->pmu) for
any HW event. When the event in question is from another HW PMU this is
wrong, and results in dereferencing garbage.

This patch updates the ARM PMU driver to first test for and reject
events from other PMUs, moving the to_arm_pmu and related logic after
this test. Fixes a crash triggered by perf_fuzzer on Linux-4.0-rc2, with
a CCI PMU present:

 ---
CPU: 0 PID: 1527 Comm: perf_fuzzer Not tainted 4.0.0-rc2 #57
Hardware name: ARM-Versatile Express
task: bd8484c0 ti: be676000 task.ti: be676000
PC is at 0xbf1bbc90
LR is at validate_event+0x34/0x5c
pc : [<bf1bbc90>]    lr : [<80016060>]    psr: 00000013
...
[<80016060>] (validate_event) from [<80016198>] (validate_group+0x28/0x90)
[<80016198>] (validate_group) from [<80016398>] (armpmu_event_init+0x150/0x218)
[<80016398>] (armpmu_event_init) from [<800882e4>] (perf_try_init_event+0x30/0x48)
[<800882e4>] (perf_try_init_event) from [<8008f544>] (perf_init_event+0x5c/0xf4)
[<8008f544>] (perf_init_event) from [<8008f8a8>] (perf_event_alloc+0x2cc/0x35c)
[<8008f8a8>] (perf_event_alloc) from [<8009015c>] (SyS_perf_event_open+0x498/0xa70)
[<8009015c>] (SyS_perf_event_open) from [<8000e420>] (ret_fast_syscall+0x0/0x34)
Code: bf1be000 bf1bb380 802a2664 00000000 (00000002)
---[ end trace 01aff0ff00926a0a ]---

Also cleans up the code to use the arm_pmu only when we know that
we are dealing with an arm pmu event.

Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>
---
 arch/arm/kernel/perf_event.c |   21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/arch/arm/kernel/perf_event.c b/arch/arm/kernel/perf_event.c
index 557e128..4a86a01 100644
--- a/arch/arm/kernel/perf_event.c
+++ b/arch/arm/kernel/perf_event.c
@@ -259,20 +259,29 @@ out:
 }
 
 static int
-validate_event(struct pmu_hw_events *hw_events,
-	       struct perf_event *event)
+validate_event(struct pmu *pmu, struct pmu_hw_events *hw_events,
+			       struct perf_event *event)
 {
-	struct arm_pmu *armpmu = to_arm_pmu(event->pmu);
+	struct arm_pmu *armpmu;
 
 	if (is_software_event(event))
 		return 1;
 
+	/*
+	 * Reject groups spanning multiple HW PMUs (e.g. CPU + CCI). The
+	 * core perf code won't check that the pmu->ctx == leader->ctx
+	 * until after pmu->event_init(event).
+	 */
+	if (event->pmu != pmu)
+		return 0;
+
 	if (event->state < PERF_EVENT_STATE_OFF)
 		return 1;
 
 	if (event->state == PERF_EVENT_STATE_OFF && !event->attr.enable_on_exec)
 		return 1;
 
+	armpmu = to_arm_pmu(event->pmu);
 	return armpmu->get_event_idx(hw_events, event) >= 0;
 }
 
@@ -288,15 +297,15 @@ validate_group(struct perf_event *event)
 	 */
 	memset(&fake_pmu.used_mask, 0, sizeof(fake_pmu.used_mask));
 
-	if (!validate_event(&fake_pmu, leader))
+	if (!validate_event(event->pmu, &fake_pmu, leader))
 		return -EINVAL;
 
 	list_for_each_entry(sibling, &leader->sibling_list, group_entry) {
-		if (!validate_event(&fake_pmu, sibling))
+		if (!validate_event(event->pmu, &fake_pmu, sibling))
 			return -EINVAL;
 	}
 
-	if (!validate_event(&fake_pmu, event))
+	if (!validate_event(event->pmu, &fake_pmu, event))
 		return -EINVAL;
 
 	return 0;
-- 
1.7.9.5

[PATCH 2/3] arm64/pmu: Reject groups spanning multiple HW PMUs

[Suzuki K. Poulose]

From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>

The perf core implicitly rejects events spanning multiple HW PMUs, as in
these cases the event->ctx will differ. However this validation is
performed after pmu::event_init() is called in perf_init_event(), and
thus pmu::event_init() may be called with a group leader from a
different HW PMU.

The ARM64 PMU driver does not take this fact into account, and when
validating groups assumes that it can call to_arm_pmu(event->pmu) for
any HW event. When the event in question is from another HW PMU this is
wrong, and results in dereferencing garbage.

This patch updates the ARM64 PMU driver to first test for and reject
events from other PMUs, moving the to_arm_pmu and related logic after
this test. Fixes a crash triggered by perf_fuzzer on Linux-4.0-rc2, with
a CCI PMU present:

Bad mode in Synchronous Abort handler detected, code 0x86000006 -- IABT (current EL)
CPU: 0 PID: 1371 Comm: perf_fuzzer Not tainted 3.19.0+ #249
Hardware name: V2F-1XV7 Cortex-A53x2 SMM (DT)
task: ffffffc07c73a280 ti: ffffffc07b0a0000 task.ti: ffffffc07b0a0000
PC is at 0x0
LR is at validate_event+0x90/0xa8
pc : [<0000000000000000>] lr : [<ffffffc000090228>] pstate: 00000145
sp : ffffffc07b0a3ba0

[<          (null)>]           (null)
[<ffffffc0000907d8>] armpmu_event_init+0x174/0x3cc
[<ffffffc00015d870>] perf_try_init_event+0x34/0x70
[<ffffffc000164094>] perf_init_event+0xe0/0x10c
[<ffffffc000164348>] perf_event_alloc+0x288/0x358
[<ffffffc000164c5c>] SyS_perf_event_open+0x464/0x98c
Code: bad PC value

Also cleans up the code to use the arm_pmu only when we know
that we are dealing with an arm pmu event.

Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Peter Ziljstra <peterz@infradead.org>
Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>
---
 arch/arm64/kernel/perf_event.c |   21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index 25a5308..68a7415 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -322,22 +322,31 @@ out:
 }
 
 static int
-validate_event(struct pmu_hw_events *hw_events,
-	       struct perf_event *event)
+validate_event(struct pmu *pmu, struct pmu_hw_events *hw_events,
+				struct perf_event *event)
 {
-	struct arm_pmu *armpmu = to_arm_pmu(event->pmu);
+	struct arm_pmu *armpmu;
 	struct hw_perf_event fake_event = event->hw;
 	struct pmu *leader_pmu = event->group_leader->pmu;
 
 	if (is_software_event(event))
 		return 1;
 
+	/*
+	 * Reject groups spanning multiple HW PMUs (e.g. CPU + CCI). The
+	 * core perf code won't check that the pmu->ctx == leader->ctx
+	 * until after pmu->event_init(event).
+	 */
+	if (event->pmu != pmu)
+		return 0;
+
 	if (event->pmu != leader_pmu || event->state < PERF_EVENT_STATE_OFF)
 		return 1;
 
 	if (event->state == PERF_EVENT_STATE_OFF && !event->attr.enable_on_exec)
 		return 1;
 
+	armpmu = to_arm_pmu(event->pmu);
 	return armpmu->get_event_idx(hw_events, &fake_event) >= 0;
 }
 
@@ -355,15 +364,15 @@ validate_group(struct perf_event *event)
 	memset(fake_used_mask, 0, sizeof(fake_used_mask));
 	fake_pmu.used_mask = fake_used_mask;
 
-	if (!validate_event(&fake_pmu, leader))
+	if (!validate_event(event->pmu, &fake_pmu, leader))
 		return -EINVAL;
 
 	list_for_each_entry(sibling, &leader->sibling_list, group_entry) {
-		if (!validate_event(&fake_pmu, sibling))
+		if (!validate_event(event->pmu, &fake_pmu, sibling))
 			return -EINVAL;
 	}
 
-	if (!validate_event(&fake_pmu, event))
+	if (!validate_event(event->pmu, &fake_pmu, event))
 		return -EINVAL;
 
 	return 0;
-- 
1.7.9.5

[PATCH 3/3] arm-cci: Reject groups spanning multiple HW PMUs

[Suzuki K. Poulose]

From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>

The perf core implicitly rejects events spanning multiple HW PMUs, as in
these cases the event->ctx will differ. However this validation is
performed after pmu::event_init() is called in perf_init_event(), and
thus pmu::event_init() may be called with a group leader from a
different HW PMU.

The CCI PMU driver does not take this fact into account, and assumes
that the any other hardware event belongs to the CCI. There are two
issues with it :

1) It is wrong and we should reject such groups.
2) Validation allocates an temporary idx for this non-cci event, which leads
to wrong calculation of the counter availability, and eventually lesser
number of events in the group.

This patch updates the CCI PMU driver to first test for and reject
events from other PMUs, which is the right thing to do.

Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>
---
 drivers/bus/arm-cci.c |   19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
index 84fd660..68ef6f2 100644
--- a/drivers/bus/arm-cci.c
+++ b/drivers/bus/arm-cci.c
@@ -660,12 +660,21 @@ static void cci_pmu_del(struct perf_event *event, int flags)
 }
 
 static int
-validate_event(struct cci_pmu_hw_events *hw_events,
-	       struct perf_event *event)
+validate_event(struct pmu *cci_pmu,
+               struct cci_pmu_hw_events *hw_events,
+               struct perf_event *event)
 {
 	if (is_software_event(event))
 		return 1;
 
+	/*
+	 * Reject groups spanning multiple HW PMUs (e.g. CPU + CCI). The
+	 * core perf code won't check that the pmu->ctx == leader->ctx
+	 * until after pmu->event_init(event).
+	 */
+	if (event->pmu != cci_pmu)
+		return 0;
+
 	if (event->state < PERF_EVENT_STATE_OFF)
 		return 1;
 
@@ -687,15 +696,15 @@ validate_group(struct perf_event *event)
 		.used_mask = CPU_BITS_NONE,
 	};
 
-	if (!validate_event(&fake_pmu, leader))
+	if (!validate_event(event->pmu, &fake_pmu, leader))
 		return -EINVAL;
 
 	list_for_each_entry(sibling, &leader->sibling_list, group_entry) {
-		if (!validate_event(&fake_pmu, sibling))
+		if (!validate_event(event->pmu, &fake_pmu, sibling))
 			return -EINVAL;
 	}
 
-	if (!validate_event(&fake_pmu, event))
+	if (!validate_event(event->pmu, &fake_pmu, event))
 		return -EINVAL;
 
 	return 0;
-- 
1.7.9.5

[PATCHv2 0/3] [4.0] arm/arm64: Do not group hardware events from different PMUs

[Peter Zijlstra]

On Tue, Mar 17, 2015 at 06:14:57PM +0000, Suzuki K. Poulose wrote:
> From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>
> 
> This is a collection of fixes which denies grouping hardware events
> from different PMUs. They also fix crashes triggered by perf_fuzzer
> on Linux-4.0-rc2.
> 
> This series hasn't changed since V1, except for better commit messages.
> 
> Similar fixes are required in ARM-CCN PMU driver and is not straight forward
> as these changes.
> 
> Suzuki K. Poulose (3):
>   arm/pmu: Reject groups spanning multiple hardware PMUs
>   arm64/pmu: Reject groups spanning multiple HW PMUs
>   arm-cci: Reject groups spanning multiple HW PMUs
> 
>  arch/arm/kernel/perf_event.c   |   21 +++++++++++++++------
>  arch/arm64/kernel/perf_event.c |   21 +++++++++++++++------
>  drivers/bus/arm-cci.c          |   19 ++++++++++++++-----
>  3 files changed, 44 insertions(+), 17 deletions(-)

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

[PATCHv2 0/3] [4.0] arm/arm64: Do not group hardware events from different PMUs

[Mark Rutland]

On Tue, Mar 17, 2015 at 06:14:57PM +0000, Suzuki K. Poulose wrote:
> From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>
> 
> This is a collection of fixes which denies grouping hardware events
> from different PMUs. They also fix crashes triggered by perf_fuzzer
> on Linux-4.0-rc2.
> 
> This series hasn't changed since V1, except for better commit messages.
> 
> Similar fixes are required in ARM-CCN PMU driver and is not straight forward
> as these changes.
> 
> Suzuki K. Poulose (3):
>   arm/pmu: Reject groups spanning multiple hardware PMUs
>   arm64/pmu: Reject groups spanning multiple HW PMUs
>   arm-cci: Reject groups spanning multiple HW PMUs

For the series:

Acked-by: Mark Rutland <mark.rutland@arm.com>

Mark.

>  arch/arm/kernel/perf_event.c   |   21 +++++++++++++++------
>  arch/arm64/kernel/perf_event.c |   21 +++++++++++++++------
>  drivers/bus/arm-cci.c          |   19 ++++++++++++++-----
>  3 files changed, 44 insertions(+), 17 deletions(-)
> 
> -- 
> 1.7.9.5
> 

[PATCHv2 0/3] [4.0] arm/arm64: Do not group hardware events from different PMUs

[Pawel Moll]

On Tue, 2015-03-17 at 18:14 +0000, Suzuki K. Poulose wrote:
> Similar fixes are required in ARM-CCN PMU driver and is not straight forward
> as these changes.

I'll have a look at this.

Pawel