2016-04-27 01:21:10

by Adam Borowski

[permalink] [raw]
Subject: [PATCH] perf/x86/amd: Explicitly define PERF_COUNT_HW_REF_CPU_CYCLES as undefined.

filter_events() relies on the value of 0 to remove events that are not
applicable, like this one.

UBSAN: Undefined behaviour in arch/x86/events/amd/core.c:132:30
index 9 is out of range for type 'u64 [9]'
UBSAN: Undefined behaviour in arch/x86/events/amd/core.c:132:9
load of address ffffffff81c021c8 with insufficient space
for an object of type 'const u64'

Signed-off-by: Adam Borowski <[email protected]>
---
arch/x86/events/amd/core.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
index 86a9bec..5fa1b8e 100644
--- a/arch/x86/events/amd/core.c
+++ b/arch/x86/events/amd/core.c
@@ -125,6 +125,7 @@ static const u64 amd_perfmon_event_map[] =
[PERF_COUNT_HW_BRANCH_MISSES] = 0x00c3,
[PERF_COUNT_HW_STALLED_CYCLES_FRONTEND] = 0x00d0, /* "Decoder empty" event */
[PERF_COUNT_HW_STALLED_CYCLES_BACKEND] = 0x00d1, /* "Dispatch stalls" event */
+ [PERF_COUNT_HW_REF_CPU_CYCLES] = 0,
};

static u64 amd_pmu_event_map(int hw_event)
--
2.8.1


2016-04-27 08:03:58

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] perf/x86/amd: Explicitly define PERF_COUNT_HW_REF_CPU_CYCLES as undefined.


* Adam Borowski <[email protected]> wrote:

> filter_events() relies on the value of 0 to remove events that are not
> applicable, like this one.
>
> UBSAN: Undefined behaviour in arch/x86/events/amd/core.c:132:30
> index 9 is out of range for type 'u64 [9]'
> UBSAN: Undefined behaviour in arch/x86/events/amd/core.c:132:9
> load of address ffffffff81c021c8 with insufficient space
> for an object of type 'const u64'
>
> Signed-off-by: Adam Borowski <[email protected]>
> ---
> arch/x86/events/amd/core.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
> index 86a9bec..5fa1b8e 100644
> --- a/arch/x86/events/amd/core.c
> +++ b/arch/x86/events/amd/core.c
> @@ -125,6 +125,7 @@ static const u64 amd_perfmon_event_map[] =
> [PERF_COUNT_HW_BRANCH_MISSES] = 0x00c3,
> [PERF_COUNT_HW_STALLED_CYCLES_FRONTEND] = 0x00d0, /* "Decoder empty" event */
> [PERF_COUNT_HW_STALLED_CYCLES_BACKEND] = 0x00d1, /* "Dispatch stalls" event */
> + [PERF_COUNT_HW_REF_CPU_CYCLES] = 0,
> };

Hm, I think it would be cleaner and more robust to change this (and all other
similar, if any) arrays to [PERF_COUNT_HW_MAX] instead.

Thanks,

Ingo

2016-04-27 09:33:04

by Adam Borowski

[permalink] [raw]
Subject: Re: [PATCH] perf/x86/amd: Explicitly define PERF_COUNT_HW_REF_CPU_CYCLES as undefined.

On Wed, Apr 27, 2016 at 10:03:45AM +0200, Ingo Molnar wrote:
> * Adam Borowski <[email protected]> wrote:
> > diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
> > index 86a9bec..5fa1b8e 100644
> > --- a/arch/x86/events/amd/core.c
> > +++ b/arch/x86/events/amd/core.c
> > @@ -125,6 +125,7 @@ static const u64 amd_perfmon_event_map[] =
> > [PERF_COUNT_HW_BRANCH_MISSES] = 0x00c3,
> > [PERF_COUNT_HW_STALLED_CYCLES_FRONTEND] = 0x00d0, /* "Decoder empty" event */
> > [PERF_COUNT_HW_STALLED_CYCLES_BACKEND] = 0x00d1, /* "Dispatch stalls" event */
> > + [PERF_COUNT_HW_REF_CPU_CYCLES] = 0,
> > };
>
> Hm, I think it would be cleaner and more robust to change this (and all other
> similar, if any) arrays to [PERF_COUNT_HW_MAX] instead.

Good idea! Both of Intel's copies (one for p4, one for core+) already set
the size this way.

--
A tit a day keeps the vet away.

2016-04-27 09:37:33

by Adam Borowski

[permalink] [raw]
Subject: [PATCH] perf/x86/amd: Set the size of event map array to PERF_COUNT_HW_MAX.

The entry for PERF_COUNT_HW_REF_CPU_CYCLES is not used on AMD, but is
referenced by filter_events() which expects undefined events to have a
value of 0.

UBSAN: Undefined behaviour in arch/x86/events/amd/core.c:132:30
index 9 is out of range for type 'u64 [9]'
UBSAN: Undefined behaviour in arch/x86/events/amd/core.c:132:9
load of address ffffffff81c021c8 with insufficient space
for an object of type 'const u64'

Signed-off-by: Adam Borowski <[email protected]>
---
arch/x86/events/amd/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
index 86a9bec..bd3e842 100644
--- a/arch/x86/events/amd/core.c
+++ b/arch/x86/events/amd/core.c
@@ -115,7 +115,7 @@ static __initconst const u64 amd_hw_cache_event_ids
/*
* AMD Performance Monitor K7 and later.
*/
-static const u64 amd_perfmon_event_map[] =
+static const u64 amd_perfmon_event_map[PERF_COUNT_HW_MAX] =
{
[PERF_COUNT_HW_CPU_CYCLES] = 0x0076,
[PERF_COUNT_HW_INSTRUCTIONS] = 0x00c0,
--
2.8.1

Subject: [tip:perf/urgent] perf/x86/amd: Set the size of event map array to PERF_COUNT_HW_MAX

Commit-ID: 0a25556f84d5f79e68e9502bb1f32a43377ab2bf
Gitweb: http://git.kernel.org/tip/0a25556f84d5f79e68e9502bb1f32a43377ab2bf
Author: Adam Borowski <[email protected]>
AuthorDate: Wed, 27 Apr 2016 11:35:31 +0200
Committer: Ingo Molnar <[email protected]>
CommitDate: Thu, 28 Apr 2016 10:20:25 +0200

perf/x86/amd: Set the size of event map array to PERF_COUNT_HW_MAX

The entry for PERF_COUNT_HW_REF_CPU_CYCLES is not used on AMD, but is
referenced by filter_events() which expects undefined events to have a
value of 0.

Found via KASAN:

UBSAN: Undefined behaviour in arch/x86/events/amd/core.c:132:30
index 9 is out of range for type 'u64 [9]'
UBSAN: Undefined behaviour in arch/x86/events/amd/core.c:132:9
load of address ffffffff81c021c8 with insufficient space for an object of type 'const u64'

Signed-off-by: Adam Borowski <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Vince Weaver <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/events/amd/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
index 86a9bec..bd3e842 100644
--- a/arch/x86/events/amd/core.c
+++ b/arch/x86/events/amd/core.c
@@ -115,7 +115,7 @@ static __initconst const u64 amd_hw_cache_event_ids
/*
* AMD Performance Monitor K7 and later.
*/
-static const u64 amd_perfmon_event_map[] =
+static const u64 amd_perfmon_event_map[PERF_COUNT_HW_MAX] =
{
[PERF_COUNT_HW_CPU_CYCLES] = 0x0076,
[PERF_COUNT_HW_INSTRUCTIONS] = 0x00c0,