* [PATCH 1/2] perf ioctl: Add check for the sample_period value
@ 2019-05-11 2:42 ` Ravi Bangoria
0 siblings, 0 replies; 24+ messages in thread
From: Ravi Bangoria @ 2019-05-11 2:42 UTC (permalink / raw)
To: peterz, jolsa, mpe, maddy; +Cc: acme, linux-kernel, linuxppc-dev, Ravi Bangoria
Add a check for sample_period value sent from userspace. Negative
value does not make sense. And in powerpc arch code this could cause
a recursive PMI leading to a hang (reported when running perf-fuzzer).
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
kernel/events/core.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index abbd4b3b96c2..e44c90378940 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -5005,6 +5005,9 @@ static int perf_event_period(struct perf_event *event, u64 __user *arg)
if (perf_event_check_period(event, value))
return -EINVAL;
+ if (!event->attr.freq && (value & (1ULL << 63)))
+ return -EINVAL;
+
event_function_call(event, __perf_event_period, &value);
return 0;
--
2.20.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 1/2] perf ioctl: Add check for the sample_period value
@ 2019-05-11 2:42 ` Ravi Bangoria
0 siblings, 0 replies; 24+ messages in thread
From: Ravi Bangoria @ 2019-05-11 2:42 UTC (permalink / raw)
To: peterz, jolsa, mpe, maddy; +Cc: Ravi Bangoria, linuxppc-dev, linux-kernel, acme
Add a check for sample_period value sent from userspace. Negative
value does not make sense. And in powerpc arch code this could cause
a recursive PMI leading to a hang (reported when running perf-fuzzer).
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
kernel/events/core.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index abbd4b3b96c2..e44c90378940 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -5005,6 +5005,9 @@ static int perf_event_period(struct perf_event *event, u64 __user *arg)
if (perf_event_check_period(event, value))
return -EINVAL;
+ if (!event->attr.freq && (value & (1ULL << 63)))
+ return -EINVAL;
+
event_function_call(event, __perf_event_period, &value);
return 0;
--
2.20.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 2/2] powerpc/perf: Fix mmcra corruption by bhrb_filter
2019-05-11 2:42 ` Ravi Bangoria
@ 2019-05-11 2:42 ` Ravi Bangoria
-1 siblings, 0 replies; 24+ messages in thread
From: Ravi Bangoria @ 2019-05-11 2:42 UTC (permalink / raw)
To: peterz, jolsa, mpe, maddy; +Cc: acme, linux-kernel, linuxppc-dev, Ravi Bangoria
Consider a scenario where user creates two events:
1st event:
attr.sample_type |= PERF_SAMPLE_BRANCH_STACK;
attr.branch_sample_type = PERF_SAMPLE_BRANCH_ANY;
fd = perf_event_open(attr, 0, 1, -1, 0);
This sets cpuhw->bhrb_filter to 0 and returns valid fd.
2nd event:
attr.sample_type |= PERF_SAMPLE_BRANCH_STACK;
attr.branch_sample_type = PERF_SAMPLE_BRANCH_CALL;
fd = perf_event_open(attr, 0, 1, -1, 0);
It overrides cpuhw->bhrb_filter to -1 and returns with error.
Now if power_pmu_enable() gets called by any path other than
power_pmu_add(), ppmu->config_bhrb(-1) will set mmcra to -1.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/perf/core-book3s.c | 6 ++++--
arch/powerpc/perf/power8-pmu.c | 3 +++
arch/powerpc/perf/power9-pmu.c | 3 +++
3 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index b0723002a396..8eb5dc5df62b 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -1846,6 +1846,7 @@ static int power_pmu_event_init(struct perf_event *event)
int n;
int err;
struct cpu_hw_events *cpuhw;
+ u64 bhrb_filter;
if (!ppmu)
return -ENOENT;
@@ -1951,13 +1952,14 @@ static int power_pmu_event_init(struct perf_event *event)
err = power_check_constraints(cpuhw, events, cflags, n + 1);
if (has_branch_stack(event)) {
- cpuhw->bhrb_filter = ppmu->bhrb_filter_map(
+ bhrb_filter = ppmu->bhrb_filter_map(
event->attr.branch_sample_type);
- if (cpuhw->bhrb_filter == -1) {
+ if (bhrb_filter == -1) {
put_cpu_var(cpu_hw_events);
return -EOPNOTSUPP;
}
+ cpuhw->bhrb_filter = bhrb_filter;
}
put_cpu_var(cpu_hw_events);
diff --git a/arch/powerpc/perf/power8-pmu.c b/arch/powerpc/perf/power8-pmu.c
index d12a2db26353..d10feef93b6b 100644
--- a/arch/powerpc/perf/power8-pmu.c
+++ b/arch/powerpc/perf/power8-pmu.c
@@ -29,6 +29,7 @@ enum {
#define POWER8_MMCRA_IFM1 0x0000000040000000UL
#define POWER8_MMCRA_IFM2 0x0000000080000000UL
#define POWER8_MMCRA_IFM3 0x00000000C0000000UL
+#define POWER8_MMCRA_BHRB_MASK 0x00000000C0000000UL
/*
* Raw event encoding for PowerISA v2.07 (Power8):
@@ -243,6 +244,8 @@ static u64 power8_bhrb_filter_map(u64 branch_sample_type)
static void power8_config_bhrb(u64 pmu_bhrb_filter)
{
+ pmu_bhrb_filter &= POWER8_MMCRA_BHRB_MASK;
+
/* Enable BHRB filter in PMU */
mtspr(SPRN_MMCRA, (mfspr(SPRN_MMCRA) | pmu_bhrb_filter));
}
diff --git a/arch/powerpc/perf/power9-pmu.c b/arch/powerpc/perf/power9-pmu.c
index 030544e35959..f3987915cadc 100644
--- a/arch/powerpc/perf/power9-pmu.c
+++ b/arch/powerpc/perf/power9-pmu.c
@@ -92,6 +92,7 @@ enum {
#define POWER9_MMCRA_IFM1 0x0000000040000000UL
#define POWER9_MMCRA_IFM2 0x0000000080000000UL
#define POWER9_MMCRA_IFM3 0x00000000C0000000UL
+#define POWER9_MMCRA_BHRB_MASK 0x00000000C0000000UL
/* Nasty Power9 specific hack */
#define PVR_POWER9_CUMULUS 0x00002000
@@ -300,6 +301,8 @@ static u64 power9_bhrb_filter_map(u64 branch_sample_type)
static void power9_config_bhrb(u64 pmu_bhrb_filter)
{
+ pmu_bhrb_filter &= POWER9_MMCRA_BHRB_MASK;
+
/* Enable BHRB filter in PMU */
mtspr(SPRN_MMCRA, (mfspr(SPRN_MMCRA) | pmu_bhrb_filter));
}
--
2.20.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 2/2] powerpc/perf: Fix mmcra corruption by bhrb_filter
@ 2019-05-11 2:42 ` Ravi Bangoria
0 siblings, 0 replies; 24+ messages in thread
From: Ravi Bangoria @ 2019-05-11 2:42 UTC (permalink / raw)
To: peterz, jolsa, mpe, maddy; +Cc: Ravi Bangoria, linuxppc-dev, linux-kernel, acme
Consider a scenario where user creates two events:
1st event:
attr.sample_type |= PERF_SAMPLE_BRANCH_STACK;
attr.branch_sample_type = PERF_SAMPLE_BRANCH_ANY;
fd = perf_event_open(attr, 0, 1, -1, 0);
This sets cpuhw->bhrb_filter to 0 and returns valid fd.
2nd event:
attr.sample_type |= PERF_SAMPLE_BRANCH_STACK;
attr.branch_sample_type = PERF_SAMPLE_BRANCH_CALL;
fd = perf_event_open(attr, 0, 1, -1, 0);
It overrides cpuhw->bhrb_filter to -1 and returns with error.
Now if power_pmu_enable() gets called by any path other than
power_pmu_add(), ppmu->config_bhrb(-1) will set mmcra to -1.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/perf/core-book3s.c | 6 ++++--
arch/powerpc/perf/power8-pmu.c | 3 +++
arch/powerpc/perf/power9-pmu.c | 3 +++
3 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index b0723002a396..8eb5dc5df62b 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -1846,6 +1846,7 @@ static int power_pmu_event_init(struct perf_event *event)
int n;
int err;
struct cpu_hw_events *cpuhw;
+ u64 bhrb_filter;
if (!ppmu)
return -ENOENT;
@@ -1951,13 +1952,14 @@ static int power_pmu_event_init(struct perf_event *event)
err = power_check_constraints(cpuhw, events, cflags, n + 1);
if (has_branch_stack(event)) {
- cpuhw->bhrb_filter = ppmu->bhrb_filter_map(
+ bhrb_filter = ppmu->bhrb_filter_map(
event->attr.branch_sample_type);
- if (cpuhw->bhrb_filter == -1) {
+ if (bhrb_filter == -1) {
put_cpu_var(cpu_hw_events);
return -EOPNOTSUPP;
}
+ cpuhw->bhrb_filter = bhrb_filter;
}
put_cpu_var(cpu_hw_events);
diff --git a/arch/powerpc/perf/power8-pmu.c b/arch/powerpc/perf/power8-pmu.c
index d12a2db26353..d10feef93b6b 100644
--- a/arch/powerpc/perf/power8-pmu.c
+++ b/arch/powerpc/perf/power8-pmu.c
@@ -29,6 +29,7 @@ enum {
#define POWER8_MMCRA_IFM1 0x0000000040000000UL
#define POWER8_MMCRA_IFM2 0x0000000080000000UL
#define POWER8_MMCRA_IFM3 0x00000000C0000000UL
+#define POWER8_MMCRA_BHRB_MASK 0x00000000C0000000UL
/*
* Raw event encoding for PowerISA v2.07 (Power8):
@@ -243,6 +244,8 @@ static u64 power8_bhrb_filter_map(u64 branch_sample_type)
static void power8_config_bhrb(u64 pmu_bhrb_filter)
{
+ pmu_bhrb_filter &= POWER8_MMCRA_BHRB_MASK;
+
/* Enable BHRB filter in PMU */
mtspr(SPRN_MMCRA, (mfspr(SPRN_MMCRA) | pmu_bhrb_filter));
}
diff --git a/arch/powerpc/perf/power9-pmu.c b/arch/powerpc/perf/power9-pmu.c
index 030544e35959..f3987915cadc 100644
--- a/arch/powerpc/perf/power9-pmu.c
+++ b/arch/powerpc/perf/power9-pmu.c
@@ -92,6 +92,7 @@ enum {
#define POWER9_MMCRA_IFM1 0x0000000040000000UL
#define POWER9_MMCRA_IFM2 0x0000000080000000UL
#define POWER9_MMCRA_IFM3 0x00000000C0000000UL
+#define POWER9_MMCRA_BHRB_MASK 0x00000000C0000000UL
/* Nasty Power9 specific hack */
#define PVR_POWER9_CUMULUS 0x00002000
@@ -300,6 +301,8 @@ static u64 power9_bhrb_filter_map(u64 branch_sample_type)
static void power9_config_bhrb(u64 pmu_bhrb_filter)
{
+ pmu_bhrb_filter &= POWER9_MMCRA_BHRB_MASK;
+
/* Enable BHRB filter in PMU */
mtspr(SPRN_MMCRA, (mfspr(SPRN_MMCRA) | pmu_bhrb_filter));
}
--
2.20.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 2/2] powerpc/perf: Fix mmcra corruption by bhrb_filter
2019-05-11 2:42 ` Ravi Bangoria
@ 2019-05-11 2:47 ` Ravi Bangoria
-1 siblings, 0 replies; 24+ messages in thread
From: Ravi Bangoria @ 2019-05-11 2:47 UTC (permalink / raw)
To: peterz, jolsa, mpe, maddy; +Cc: acme, linux-kernel, linuxppc-dev, Ravi Bangoria
On 5/11/19 8:12 AM, Ravi Bangoria wrote:
> Consider a scenario where user creates two events:
>
> 1st event:
> attr.sample_type |= PERF_SAMPLE_BRANCH_STACK;
> attr.branch_sample_type = PERF_SAMPLE_BRANCH_ANY;
> fd = perf_event_open(attr, 0, 1, -1, 0);
>
> This sets cpuhw->bhrb_filter to 0 and returns valid fd.
>
> 2nd event:
> attr.sample_type |= PERF_SAMPLE_BRANCH_STACK;
> attr.branch_sample_type = PERF_SAMPLE_BRANCH_CALL;
> fd = perf_event_open(attr, 0, 1, -1, 0);
>
> It overrides cpuhw->bhrb_filter to -1 and returns with error.
>
> Now if power_pmu_enable() gets called by any path other than
> power_pmu_add(), ppmu->config_bhrb(-1) will set mmcra to -1.
>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Fixes: 3925f46bb590 ("powerpc/perf: Enable branch stack sampling framework")
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/2] powerpc/perf: Fix mmcra corruption by bhrb_filter
@ 2019-05-11 2:47 ` Ravi Bangoria
0 siblings, 0 replies; 24+ messages in thread
From: Ravi Bangoria @ 2019-05-11 2:47 UTC (permalink / raw)
To: peterz, jolsa, mpe, maddy; +Cc: Ravi Bangoria, linuxppc-dev, linux-kernel, acme
On 5/11/19 8:12 AM, Ravi Bangoria wrote:
> Consider a scenario where user creates two events:
>
> 1st event:
> attr.sample_type |= PERF_SAMPLE_BRANCH_STACK;
> attr.branch_sample_type = PERF_SAMPLE_BRANCH_ANY;
> fd = perf_event_open(attr, 0, 1, -1, 0);
>
> This sets cpuhw->bhrb_filter to 0 and returns valid fd.
>
> 2nd event:
> attr.sample_type |= PERF_SAMPLE_BRANCH_STACK;
> attr.branch_sample_type = PERF_SAMPLE_BRANCH_CALL;
> fd = perf_event_open(attr, 0, 1, -1, 0);
>
> It overrides cpuhw->bhrb_filter to -1 and returns with error.
>
> Now if power_pmu_enable() gets called by any path other than
> power_pmu_add(), ppmu->config_bhrb(-1) will set mmcra to -1.
>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Fixes: 3925f46bb590 ("powerpc/perf: Enable branch stack sampling framework")
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] perf ioctl: Add check for the sample_period value
2019-05-11 2:42 ` Ravi Bangoria
@ 2019-05-13 7:42 ` Peter Zijlstra
-1 siblings, 0 replies; 24+ messages in thread
From: Peter Zijlstra @ 2019-05-13 7:42 UTC (permalink / raw)
To: Ravi Bangoria; +Cc: jolsa, mpe, maddy, acme, linux-kernel, linuxppc-dev
On Sat, May 11, 2019 at 08:12:16AM +0530, Ravi Bangoria wrote:
> Add a check for sample_period value sent from userspace. Negative
> value does not make sense. And in powerpc arch code this could cause
> a recursive PMI leading to a hang (reported when running perf-fuzzer).
>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> ---
> kernel/events/core.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index abbd4b3b96c2..e44c90378940 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -5005,6 +5005,9 @@ static int perf_event_period(struct perf_event *event, u64 __user *arg)
> if (perf_event_check_period(event, value))
> return -EINVAL;
>
> + if (!event->attr.freq && (value & (1ULL << 63)))
> + return -EINVAL;
Well, perf_event_attr::sample_period is __u64. Would not be the site
using it as signed be the one in error?
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] perf ioctl: Add check for the sample_period value
@ 2019-05-13 7:42 ` Peter Zijlstra
0 siblings, 0 replies; 24+ messages in thread
From: Peter Zijlstra @ 2019-05-13 7:42 UTC (permalink / raw)
To: Ravi Bangoria; +Cc: maddy, linuxppc-dev, linux-kernel, acme, jolsa
On Sat, May 11, 2019 at 08:12:16AM +0530, Ravi Bangoria wrote:
> Add a check for sample_period value sent from userspace. Negative
> value does not make sense. And in powerpc arch code this could cause
> a recursive PMI leading to a hang (reported when running perf-fuzzer).
>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> ---
> kernel/events/core.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index abbd4b3b96c2..e44c90378940 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -5005,6 +5005,9 @@ static int perf_event_period(struct perf_event *event, u64 __user *arg)
> if (perf_event_check_period(event, value))
> return -EINVAL;
>
> + if (!event->attr.freq && (value & (1ULL << 63)))
> + return -EINVAL;
Well, perf_event_attr::sample_period is __u64. Would not be the site
using it as signed be the one in error?
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] perf ioctl: Add check for the sample_period value
2019-05-13 7:42 ` Peter Zijlstra
@ 2019-05-13 8:56 ` Peter Zijlstra
-1 siblings, 0 replies; 24+ messages in thread
From: Peter Zijlstra @ 2019-05-13 8:56 UTC (permalink / raw)
To: Ravi Bangoria; +Cc: jolsa, mpe, maddy, acme, linux-kernel, linuxppc-dev
On Mon, May 13, 2019 at 09:42:13AM +0200, Peter Zijlstra wrote:
> On Sat, May 11, 2019 at 08:12:16AM +0530, Ravi Bangoria wrote:
> > Add a check for sample_period value sent from userspace. Negative
> > value does not make sense. And in powerpc arch code this could cause
> > a recursive PMI leading to a hang (reported when running perf-fuzzer).
> >
> > Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> > ---
> > kernel/events/core.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/kernel/events/core.c b/kernel/events/core.c
> > index abbd4b3b96c2..e44c90378940 100644
> > --- a/kernel/events/core.c
> > +++ b/kernel/events/core.c
> > @@ -5005,6 +5005,9 @@ static int perf_event_period(struct perf_event *event, u64 __user *arg)
> > if (perf_event_check_period(event, value))
> > return -EINVAL;
> >
> > + if (!event->attr.freq && (value & (1ULL << 63)))
> > + return -EINVAL;
>
> Well, perf_event_attr::sample_period is __u64. Would not be the site
> using it as signed be the one in error?
You forgot to mention commit: 0819b2e30ccb9, so I guess this just makes
it consistent and is fine.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] perf ioctl: Add check for the sample_period value
@ 2019-05-13 8:56 ` Peter Zijlstra
0 siblings, 0 replies; 24+ messages in thread
From: Peter Zijlstra @ 2019-05-13 8:56 UTC (permalink / raw)
To: Ravi Bangoria; +Cc: maddy, linuxppc-dev, linux-kernel, acme, jolsa
On Mon, May 13, 2019 at 09:42:13AM +0200, Peter Zijlstra wrote:
> On Sat, May 11, 2019 at 08:12:16AM +0530, Ravi Bangoria wrote:
> > Add a check for sample_period value sent from userspace. Negative
> > value does not make sense. And in powerpc arch code this could cause
> > a recursive PMI leading to a hang (reported when running perf-fuzzer).
> >
> > Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> > ---
> > kernel/events/core.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/kernel/events/core.c b/kernel/events/core.c
> > index abbd4b3b96c2..e44c90378940 100644
> > --- a/kernel/events/core.c
> > +++ b/kernel/events/core.c
> > @@ -5005,6 +5005,9 @@ static int perf_event_period(struct perf_event *event, u64 __user *arg)
> > if (perf_event_check_period(event, value))
> > return -EINVAL;
> >
> > + if (!event->attr.freq && (value & (1ULL << 63)))
> > + return -EINVAL;
>
> Well, perf_event_attr::sample_period is __u64. Would not be the site
> using it as signed be the one in error?
You forgot to mention commit: 0819b2e30ccb9, so I guess this just makes
it consistent and is fine.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] perf ioctl: Add check for the sample_period value
2019-05-13 8:56 ` Peter Zijlstra
@ 2019-05-13 10:07 ` Ravi Bangoria
-1 siblings, 0 replies; 24+ messages in thread
From: Ravi Bangoria @ 2019-05-13 10:07 UTC (permalink / raw)
To: Peter Zijlstra
Cc: jolsa, mpe, maddy, acme, linux-kernel, linuxppc-dev, Ravi Bangoria
On 5/13/19 2:26 PM, Peter Zijlstra wrote:
> On Mon, May 13, 2019 at 09:42:13AM +0200, Peter Zijlstra wrote:
>> On Sat, May 11, 2019 at 08:12:16AM +0530, Ravi Bangoria wrote:
>>> Add a check for sample_period value sent from userspace. Negative
>>> value does not make sense. And in powerpc arch code this could cause
>>> a recursive PMI leading to a hang (reported when running perf-fuzzer).
>>>
>>> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
>>> ---
>>> kernel/events/core.c | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/kernel/events/core.c b/kernel/events/core.c
>>> index abbd4b3b96c2..e44c90378940 100644
>>> --- a/kernel/events/core.c
>>> +++ b/kernel/events/core.c
>>> @@ -5005,6 +5005,9 @@ static int perf_event_period(struct perf_event *event, u64 __user *arg)
>>> if (perf_event_check_period(event, value))
>>> return -EINVAL;
>>>
>>> + if (!event->attr.freq && (value & (1ULL << 63)))
>>> + return -EINVAL;
>>
>> Well, perf_event_attr::sample_period is __u64. Would not be the site
>> using it as signed be the one in error?
>
> You forgot to mention commit: 0819b2e30ccb9, so I guess this just makes
> it consistent and is fine.
>
Yeah, I was about to reply :)
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] perf ioctl: Add check for the sample_period value
@ 2019-05-13 10:07 ` Ravi Bangoria
0 siblings, 0 replies; 24+ messages in thread
From: Ravi Bangoria @ 2019-05-13 10:07 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Ravi Bangoria, maddy, linuxppc-dev, linux-kernel, acme, jolsa
On 5/13/19 2:26 PM, Peter Zijlstra wrote:
> On Mon, May 13, 2019 at 09:42:13AM +0200, Peter Zijlstra wrote:
>> On Sat, May 11, 2019 at 08:12:16AM +0530, Ravi Bangoria wrote:
>>> Add a check for sample_period value sent from userspace. Negative
>>> value does not make sense. And in powerpc arch code this could cause
>>> a recursive PMI leading to a hang (reported when running perf-fuzzer).
>>>
>>> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
>>> ---
>>> kernel/events/core.c | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/kernel/events/core.c b/kernel/events/core.c
>>> index abbd4b3b96c2..e44c90378940 100644
>>> --- a/kernel/events/core.c
>>> +++ b/kernel/events/core.c
>>> @@ -5005,6 +5005,9 @@ static int perf_event_period(struct perf_event *event, u64 __user *arg)
>>> if (perf_event_check_period(event, value))
>>> return -EINVAL;
>>>
>>> + if (!event->attr.freq && (value & (1ULL << 63)))
>>> + return -EINVAL;
>>
>> Well, perf_event_attr::sample_period is __u64. Would not be the site
>> using it as signed be the one in error?
>
> You forgot to mention commit: 0819b2e30ccb9, so I guess this just makes
> it consistent and is fine.
>
Yeah, I was about to reply :)
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/2] powerpc/perf: Fix mmcra corruption by bhrb_filter
2019-05-11 2:42 ` Ravi Bangoria
@ 2019-05-22 5:01 ` Madhavan Srinivasan
-1 siblings, 0 replies; 24+ messages in thread
From: Madhavan Srinivasan @ 2019-05-22 5:01 UTC (permalink / raw)
To: Ravi Bangoria, peterz, jolsa, mpe; +Cc: acme, linux-kernel, linuxppc-dev
On 11/05/19 8:12 AM, Ravi Bangoria wrote:
> Consider a scenario where user creates two events:
>
> 1st event:
> attr.sample_type |= PERF_SAMPLE_BRANCH_STACK;
> attr.branch_sample_type = PERF_SAMPLE_BRANCH_ANY;
> fd = perf_event_open(attr, 0, 1, -1, 0);
>
> This sets cpuhw->bhrb_filter to 0 and returns valid fd.
>
> 2nd event:
> attr.sample_type |= PERF_SAMPLE_BRANCH_STACK;
> attr.branch_sample_type = PERF_SAMPLE_BRANCH_CALL;
> fd = perf_event_open(attr, 0, 1, -1, 0);
>
> It overrides cpuhw->bhrb_filter to -1 and returns with error.
>
> Now if power_pmu_enable() gets called by any path other than
> power_pmu_add(), ppmu->config_bhrb(-1) will set mmcra to -1.
Reviewed-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> ---
> arch/powerpc/perf/core-book3s.c | 6 ++++--
> arch/powerpc/perf/power8-pmu.c | 3 +++
> arch/powerpc/perf/power9-pmu.c | 3 +++
> 3 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> index b0723002a396..8eb5dc5df62b 100644
> --- a/arch/powerpc/perf/core-book3s.c
> +++ b/arch/powerpc/perf/core-book3s.c
> @@ -1846,6 +1846,7 @@ static int power_pmu_event_init(struct perf_event *event)
> int n;
> int err;
> struct cpu_hw_events *cpuhw;
> + u64 bhrb_filter;
>
> if (!ppmu)
> return -ENOENT;
> @@ -1951,13 +1952,14 @@ static int power_pmu_event_init(struct perf_event *event)
> err = power_check_constraints(cpuhw, events, cflags, n + 1);
>
> if (has_branch_stack(event)) {
> - cpuhw->bhrb_filter = ppmu->bhrb_filter_map(
> + bhrb_filter = ppmu->bhrb_filter_map(
> event->attr.branch_sample_type);
>
> - if (cpuhw->bhrb_filter == -1) {
> + if (bhrb_filter == -1) {
> put_cpu_var(cpu_hw_events);
> return -EOPNOTSUPP;
> }
> + cpuhw->bhrb_filter = bhrb_filter;
> }
>
> put_cpu_var(cpu_hw_events);
> diff --git a/arch/powerpc/perf/power8-pmu.c b/arch/powerpc/perf/power8-pmu.c
> index d12a2db26353..d10feef93b6b 100644
> --- a/arch/powerpc/perf/power8-pmu.c
> +++ b/arch/powerpc/perf/power8-pmu.c
> @@ -29,6 +29,7 @@ enum {
> #define POWER8_MMCRA_IFM1 0x0000000040000000UL
> #define POWER8_MMCRA_IFM2 0x0000000080000000UL
> #define POWER8_MMCRA_IFM3 0x00000000C0000000UL
> +#define POWER8_MMCRA_BHRB_MASK 0x00000000C0000000UL
>
> /*
> * Raw event encoding for PowerISA v2.07 (Power8):
> @@ -243,6 +244,8 @@ static u64 power8_bhrb_filter_map(u64 branch_sample_type)
>
> static void power8_config_bhrb(u64 pmu_bhrb_filter)
> {
> + pmu_bhrb_filter &= POWER8_MMCRA_BHRB_MASK;
> +
> /* Enable BHRB filter in PMU */
> mtspr(SPRN_MMCRA, (mfspr(SPRN_MMCRA) | pmu_bhrb_filter));
> }
> diff --git a/arch/powerpc/perf/power9-pmu.c b/arch/powerpc/perf/power9-pmu.c
> index 030544e35959..f3987915cadc 100644
> --- a/arch/powerpc/perf/power9-pmu.c
> +++ b/arch/powerpc/perf/power9-pmu.c
> @@ -92,6 +92,7 @@ enum {
> #define POWER9_MMCRA_IFM1 0x0000000040000000UL
> #define POWER9_MMCRA_IFM2 0x0000000080000000UL
> #define POWER9_MMCRA_IFM3 0x00000000C0000000UL
> +#define POWER9_MMCRA_BHRB_MASK 0x00000000C0000000UL
>
> /* Nasty Power9 specific hack */
> #define PVR_POWER9_CUMULUS 0x00002000
> @@ -300,6 +301,8 @@ static u64 power9_bhrb_filter_map(u64 branch_sample_type)
>
> static void power9_config_bhrb(u64 pmu_bhrb_filter)
> {
> + pmu_bhrb_filter &= POWER9_MMCRA_BHRB_MASK;
> +
> /* Enable BHRB filter in PMU */
> mtspr(SPRN_MMCRA, (mfspr(SPRN_MMCRA) | pmu_bhrb_filter));
> }
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/2] powerpc/perf: Fix mmcra corruption by bhrb_filter
@ 2019-05-22 5:01 ` Madhavan Srinivasan
0 siblings, 0 replies; 24+ messages in thread
From: Madhavan Srinivasan @ 2019-05-22 5:01 UTC (permalink / raw)
To: Ravi Bangoria, peterz, jolsa, mpe; +Cc: linuxppc-dev, linux-kernel, acme
On 11/05/19 8:12 AM, Ravi Bangoria wrote:
> Consider a scenario where user creates two events:
>
> 1st event:
> attr.sample_type |= PERF_SAMPLE_BRANCH_STACK;
> attr.branch_sample_type = PERF_SAMPLE_BRANCH_ANY;
> fd = perf_event_open(attr, 0, 1, -1, 0);
>
> This sets cpuhw->bhrb_filter to 0 and returns valid fd.
>
> 2nd event:
> attr.sample_type |= PERF_SAMPLE_BRANCH_STACK;
> attr.branch_sample_type = PERF_SAMPLE_BRANCH_CALL;
> fd = perf_event_open(attr, 0, 1, -1, 0);
>
> It overrides cpuhw->bhrb_filter to -1 and returns with error.
>
> Now if power_pmu_enable() gets called by any path other than
> power_pmu_add(), ppmu->config_bhrb(-1) will set mmcra to -1.
Reviewed-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> ---
> arch/powerpc/perf/core-book3s.c | 6 ++++--
> arch/powerpc/perf/power8-pmu.c | 3 +++
> arch/powerpc/perf/power9-pmu.c | 3 +++
> 3 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> index b0723002a396..8eb5dc5df62b 100644
> --- a/arch/powerpc/perf/core-book3s.c
> +++ b/arch/powerpc/perf/core-book3s.c
> @@ -1846,6 +1846,7 @@ static int power_pmu_event_init(struct perf_event *event)
> int n;
> int err;
> struct cpu_hw_events *cpuhw;
> + u64 bhrb_filter;
>
> if (!ppmu)
> return -ENOENT;
> @@ -1951,13 +1952,14 @@ static int power_pmu_event_init(struct perf_event *event)
> err = power_check_constraints(cpuhw, events, cflags, n + 1);
>
> if (has_branch_stack(event)) {
> - cpuhw->bhrb_filter = ppmu->bhrb_filter_map(
> + bhrb_filter = ppmu->bhrb_filter_map(
> event->attr.branch_sample_type);
>
> - if (cpuhw->bhrb_filter == -1) {
> + if (bhrb_filter == -1) {
> put_cpu_var(cpu_hw_events);
> return -EOPNOTSUPP;
> }
> + cpuhw->bhrb_filter = bhrb_filter;
> }
>
> put_cpu_var(cpu_hw_events);
> diff --git a/arch/powerpc/perf/power8-pmu.c b/arch/powerpc/perf/power8-pmu.c
> index d12a2db26353..d10feef93b6b 100644
> --- a/arch/powerpc/perf/power8-pmu.c
> +++ b/arch/powerpc/perf/power8-pmu.c
> @@ -29,6 +29,7 @@ enum {
> #define POWER8_MMCRA_IFM1 0x0000000040000000UL
> #define POWER8_MMCRA_IFM2 0x0000000080000000UL
> #define POWER8_MMCRA_IFM3 0x00000000C0000000UL
> +#define POWER8_MMCRA_BHRB_MASK 0x00000000C0000000UL
>
> /*
> * Raw event encoding for PowerISA v2.07 (Power8):
> @@ -243,6 +244,8 @@ static u64 power8_bhrb_filter_map(u64 branch_sample_type)
>
> static void power8_config_bhrb(u64 pmu_bhrb_filter)
> {
> + pmu_bhrb_filter &= POWER8_MMCRA_BHRB_MASK;
> +
> /* Enable BHRB filter in PMU */
> mtspr(SPRN_MMCRA, (mfspr(SPRN_MMCRA) | pmu_bhrb_filter));
> }
> diff --git a/arch/powerpc/perf/power9-pmu.c b/arch/powerpc/perf/power9-pmu.c
> index 030544e35959..f3987915cadc 100644
> --- a/arch/powerpc/perf/power9-pmu.c
> +++ b/arch/powerpc/perf/power9-pmu.c
> @@ -92,6 +92,7 @@ enum {
> #define POWER9_MMCRA_IFM1 0x0000000040000000UL
> #define POWER9_MMCRA_IFM2 0x0000000080000000UL
> #define POWER9_MMCRA_IFM3 0x00000000C0000000UL
> +#define POWER9_MMCRA_BHRB_MASK 0x00000000C0000000UL
>
> /* Nasty Power9 specific hack */
> #define PVR_POWER9_CUMULUS 0x00002000
> @@ -300,6 +301,8 @@ static u64 power9_bhrb_filter_map(u64 branch_sample_type)
>
> static void power9_config_bhrb(u64 pmu_bhrb_filter)
> {
> + pmu_bhrb_filter &= POWER9_MMCRA_BHRB_MASK;
> +
> /* Enable BHRB filter in PMU */
> mtspr(SPRN_MMCRA, (mfspr(SPRN_MMCRA) | pmu_bhrb_filter));
> }
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/2] powerpc/perf: Fix mmcra corruption by bhrb_filter
2019-05-11 2:42 ` Ravi Bangoria
` (2 preceding siblings ...)
(?)
@ 2019-05-25 0:54 ` Michael Ellerman
-1 siblings, 0 replies; 24+ messages in thread
From: Michael Ellerman @ 2019-05-25 0:54 UTC (permalink / raw)
To: Ravi Bangoria, peterz, jolsa, maddy
Cc: Ravi Bangoria, linuxppc-dev, linux-kernel, acme
On Sat, 2019-05-11 at 02:42:17 UTC, Ravi Bangoria wrote:
> Consider a scenario where user creates two events:
>
> 1st event:
> attr.sample_type |= PERF_SAMPLE_BRANCH_STACK;
> attr.branch_sample_type = PERF_SAMPLE_BRANCH_ANY;
> fd = perf_event_open(attr, 0, 1, -1, 0);
>
> This sets cpuhw->bhrb_filter to 0 and returns valid fd.
>
> 2nd event:
> attr.sample_type |= PERF_SAMPLE_BRANCH_STACK;
> attr.branch_sample_type = PERF_SAMPLE_BRANCH_CALL;
> fd = perf_event_open(attr, 0, 1, -1, 0);
>
> It overrides cpuhw->bhrb_filter to -1 and returns with error.
>
> Now if power_pmu_enable() gets called by any path other than
> power_pmu_add(), ppmu->config_bhrb(-1) will set mmcra to -1.
>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> Reviewed-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
Applied to powerpc fixes, thanks.
https://git.kernel.org/powerpc/c/3202e35ec1c8fc19cea24253ff83edf7
cheers
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] perf ioctl: Add check for the sample_period value
2019-05-13 10:07 ` Ravi Bangoria
@ 2019-05-28 9:50 ` Michael Ellerman
-1 siblings, 0 replies; 24+ messages in thread
From: Michael Ellerman @ 2019-05-28 9:50 UTC (permalink / raw)
To: Ravi Bangoria, Peter Zijlstra
Cc: jolsa, maddy, acme, linux-kernel, linuxppc-dev, Ravi Bangoria
Ravi Bangoria <ravi.bangoria@linux.ibm.com> writes:
> On 5/13/19 2:26 PM, Peter Zijlstra wrote:
>> On Mon, May 13, 2019 at 09:42:13AM +0200, Peter Zijlstra wrote:
>>> On Sat, May 11, 2019 at 08:12:16AM +0530, Ravi Bangoria wrote:
>>>> Add a check for sample_period value sent from userspace. Negative
>>>> value does not make sense. And in powerpc arch code this could cause
>>>> a recursive PMI leading to a hang (reported when running perf-fuzzer).
>>>>
>>>> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
>>>> ---
>>>> kernel/events/core.c | 3 +++
>>>> 1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/kernel/events/core.c b/kernel/events/core.c
>>>> index abbd4b3b96c2..e44c90378940 100644
>>>> --- a/kernel/events/core.c
>>>> +++ b/kernel/events/core.c
>>>> @@ -5005,6 +5005,9 @@ static int perf_event_period(struct perf_event *event, u64 __user *arg)
>>>> if (perf_event_check_period(event, value))
>>>> return -EINVAL;
>>>>
>>>> + if (!event->attr.freq && (value & (1ULL << 63)))
>>>> + return -EINVAL;
>>>
>>> Well, perf_event_attr::sample_period is __u64. Would not be the site
>>> using it as signed be the one in error?
>>
>> You forgot to mention commit: 0819b2e30ccb9, so I guess this just makes
>> it consistent and is fine.
>>
>
> Yeah, I was about to reply :)
I've taken patch 2. You should probably do a v2 of patch 1 with an
updated change log that explains things fully?
cheers
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] perf ioctl: Add check for the sample_period value
@ 2019-05-28 9:50 ` Michael Ellerman
0 siblings, 0 replies; 24+ messages in thread
From: Michael Ellerman @ 2019-05-28 9:50 UTC (permalink / raw)
To: Ravi Bangoria, Peter Zijlstra
Cc: Ravi Bangoria, maddy, jolsa, linux-kernel, acme, linuxppc-dev
Ravi Bangoria <ravi.bangoria@linux.ibm.com> writes:
> On 5/13/19 2:26 PM, Peter Zijlstra wrote:
>> On Mon, May 13, 2019 at 09:42:13AM +0200, Peter Zijlstra wrote:
>>> On Sat, May 11, 2019 at 08:12:16AM +0530, Ravi Bangoria wrote:
>>>> Add a check for sample_period value sent from userspace. Negative
>>>> value does not make sense. And in powerpc arch code this could cause
>>>> a recursive PMI leading to a hang (reported when running perf-fuzzer).
>>>>
>>>> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
>>>> ---
>>>> kernel/events/core.c | 3 +++
>>>> 1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/kernel/events/core.c b/kernel/events/core.c
>>>> index abbd4b3b96c2..e44c90378940 100644
>>>> --- a/kernel/events/core.c
>>>> +++ b/kernel/events/core.c
>>>> @@ -5005,6 +5005,9 @@ static int perf_event_period(struct perf_event *event, u64 __user *arg)
>>>> if (perf_event_check_period(event, value))
>>>> return -EINVAL;
>>>>
>>>> + if (!event->attr.freq && (value & (1ULL << 63)))
>>>> + return -EINVAL;
>>>
>>> Well, perf_event_attr::sample_period is __u64. Would not be the site
>>> using it as signed be the one in error?
>>
>> You forgot to mention commit: 0819b2e30ccb9, so I guess this just makes
>> it consistent and is fine.
>>
>
> Yeah, I was about to reply :)
I've taken patch 2. You should probably do a v2 of patch 1 with an
updated change log that explains things fully?
cheers
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2] perf ioctl: Add check for the sample_period value
2019-05-28 9:50 ` Michael Ellerman
@ 2019-06-04 4:29 ` Ravi Bangoria
-1 siblings, 0 replies; 24+ messages in thread
From: Ravi Bangoria @ 2019-06-04 4:29 UTC (permalink / raw)
To: mpe, peterz; +Cc: jolsa, maddy, acme, linux-kernel, linuxppc-dev, Ravi Bangoria
perf_event_open() limits the sample_period to 63 bits. See
commit 0819b2e30ccb ("perf: Limit perf_event_attr::sample_period
to 63 bits"). Make ioctl() consistent with it.
Also on powerpc, negative sample_period could cause a recursive
PMIs leading to a hang (reported when running perf-fuzzer).
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
kernel/events/core.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index abbd4b3b96c2..e44c90378940 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -5005,6 +5005,9 @@ static int perf_event_period(struct perf_event *event, u64 __user *arg)
if (perf_event_check_period(event, value))
return -EINVAL;
+ if (!event->attr.freq && (value & (1ULL << 63)))
+ return -EINVAL;
+
event_function_call(event, __perf_event_period, &value);
return 0;
--
2.20.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2] perf ioctl: Add check for the sample_period value
@ 2019-06-04 4:29 ` Ravi Bangoria
0 siblings, 0 replies; 24+ messages in thread
From: Ravi Bangoria @ 2019-06-04 4:29 UTC (permalink / raw)
To: mpe, peterz; +Cc: Ravi Bangoria, maddy, jolsa, linux-kernel, acme, linuxppc-dev
perf_event_open() limits the sample_period to 63 bits. See
commit 0819b2e30ccb ("perf: Limit perf_event_attr::sample_period
to 63 bits"). Make ioctl() consistent with it.
Also on powerpc, negative sample_period could cause a recursive
PMIs leading to a hang (reported when running perf-fuzzer).
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
kernel/events/core.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index abbd4b3b96c2..e44c90378940 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -5005,6 +5005,9 @@ static int perf_event_period(struct perf_event *event, u64 __user *arg)
if (perf_event_check_period(event, value))
return -EINVAL;
+ if (!event->attr.freq && (value & (1ULL << 63)))
+ return -EINVAL;
+
event_function_call(event, __perf_event_period, &value);
return 0;
--
2.20.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v2] perf ioctl: Add check for the sample_period value
2019-06-04 4:29 ` Ravi Bangoria
@ 2019-06-17 8:38 ` Ravi Bangoria
-1 siblings, 0 replies; 24+ messages in thread
From: Ravi Bangoria @ 2019-06-17 8:38 UTC (permalink / raw)
To: mpe, peterz; +Cc: jolsa, maddy, acme, linux-kernel, linuxppc-dev, Ravi Bangoria
Peter / mpe,
Is the v2 looks good? If so, can anyone of you please pick this up.
On 6/4/19 9:59 AM, Ravi Bangoria wrote:
> perf_event_open() limits the sample_period to 63 bits. See
> commit 0819b2e30ccb ("perf: Limit perf_event_attr::sample_period
> to 63 bits"). Make ioctl() consistent with it.
>
> Also on powerpc, negative sample_period could cause a recursive
> PMIs leading to a hang (reported when running perf-fuzzer).
>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> ---
> kernel/events/core.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index abbd4b3b96c2..e44c90378940 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -5005,6 +5005,9 @@ static int perf_event_period(struct perf_event *event, u64 __user *arg)
> if (perf_event_check_period(event, value))
> return -EINVAL;
>
> + if (!event->attr.freq && (value & (1ULL << 63)))
> + return -EINVAL;
> +
> event_function_call(event, __perf_event_period, &value);
>
> return 0;
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2] perf ioctl: Add check for the sample_period value
@ 2019-06-17 8:38 ` Ravi Bangoria
0 siblings, 0 replies; 24+ messages in thread
From: Ravi Bangoria @ 2019-06-17 8:38 UTC (permalink / raw)
To: mpe, peterz; +Cc: Ravi Bangoria, maddy, jolsa, linux-kernel, acme, linuxppc-dev
Peter / mpe,
Is the v2 looks good? If so, can anyone of you please pick this up.
On 6/4/19 9:59 AM, Ravi Bangoria wrote:
> perf_event_open() limits the sample_period to 63 bits. See
> commit 0819b2e30ccb ("perf: Limit perf_event_attr::sample_period
> to 63 bits"). Make ioctl() consistent with it.
>
> Also on powerpc, negative sample_period could cause a recursive
> PMIs leading to a hang (reported when running perf-fuzzer).
>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> ---
> kernel/events/core.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index abbd4b3b96c2..e44c90378940 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -5005,6 +5005,9 @@ static int perf_event_period(struct perf_event *event, u64 __user *arg)
> if (perf_event_check_period(event, value))
> return -EINVAL;
>
> + if (!event->attr.freq && (value & (1ULL << 63)))
> + return -EINVAL;
> +
> event_function_call(event, __perf_event_period, &value);
>
> return 0;
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2] perf ioctl: Add check for the sample_period value
2019-06-17 8:38 ` Ravi Bangoria
@ 2019-06-18 12:28 ` Michael Ellerman
-1 siblings, 0 replies; 24+ messages in thread
From: Michael Ellerman @ 2019-06-18 12:28 UTC (permalink / raw)
To: Ravi Bangoria, peterz
Cc: jolsa, maddy, acme, linux-kernel, linuxppc-dev, Ravi Bangoria
Ravi Bangoria <ravi.bangoria@linux.ibm.com> writes:
> Peter / mpe,
>
> Is the v2 looks good? If so, can anyone of you please pick this up.
I usually wouldn't take it, it's generic perf code. Unless
peter/ingo/acme tell me otherwise.
It's sort of a bug fix for 0819b2e30ccb, should it have a fixes and/or
stable tag?
Fixes: 0819b2e30ccb ("perf: Limit perf_event_attr::sample_period to 63 bits")
Cc: stable@vger.kernel.org # v3.15+
cheers
> On 6/4/19 9:59 AM, Ravi Bangoria wrote:
>> perf_event_open() limits the sample_period to 63 bits. See
>> commit 0819b2e30ccb ("perf: Limit perf_event_attr::sample_period
>> to 63 bits"). Make ioctl() consistent with it.
>>
>> Also on powerpc, negative sample_period could cause a recursive
>> PMIs leading to a hang (reported when running perf-fuzzer).
>>
>> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
>> ---
>> kernel/events/core.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/kernel/events/core.c b/kernel/events/core.c
>> index abbd4b3b96c2..e44c90378940 100644
>> --- a/kernel/events/core.c
>> +++ b/kernel/events/core.c
>> @@ -5005,6 +5005,9 @@ static int perf_event_period(struct perf_event *event, u64 __user *arg)
>> if (perf_event_check_period(event, value))
>> return -EINVAL;
>>
>> + if (!event->attr.freq && (value & (1ULL << 63)))
>> + return -EINVAL;
>> +
>> event_function_call(event, __perf_event_period, &value);
>>
>> return 0;
>>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2] perf ioctl: Add check for the sample_period value
@ 2019-06-18 12:28 ` Michael Ellerman
0 siblings, 0 replies; 24+ messages in thread
From: Michael Ellerman @ 2019-06-18 12:28 UTC (permalink / raw)
To: Ravi Bangoria, peterz
Cc: Ravi Bangoria, maddy, jolsa, linux-kernel, acme, linuxppc-dev
Ravi Bangoria <ravi.bangoria@linux.ibm.com> writes:
> Peter / mpe,
>
> Is the v2 looks good? If so, can anyone of you please pick this up.
I usually wouldn't take it, it's generic perf code. Unless
peter/ingo/acme tell me otherwise.
It's sort of a bug fix for 0819b2e30ccb, should it have a fixes and/or
stable tag?
Fixes: 0819b2e30ccb ("perf: Limit perf_event_attr::sample_period to 63 bits")
Cc: stable@vger.kernel.org # v3.15+
cheers
> On 6/4/19 9:59 AM, Ravi Bangoria wrote:
>> perf_event_open() limits the sample_period to 63 bits. See
>> commit 0819b2e30ccb ("perf: Limit perf_event_attr::sample_period
>> to 63 bits"). Make ioctl() consistent with it.
>>
>> Also on powerpc, negative sample_period could cause a recursive
>> PMIs leading to a hang (reported when running perf-fuzzer).
>>
>> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
>> ---
>> kernel/events/core.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/kernel/events/core.c b/kernel/events/core.c
>> index abbd4b3b96c2..e44c90378940 100644
>> --- a/kernel/events/core.c
>> +++ b/kernel/events/core.c
>> @@ -5005,6 +5005,9 @@ static int perf_event_period(struct perf_event *event, u64 __user *arg)
>> if (perf_event_check_period(event, value))
>> return -EINVAL;
>>
>> + if (!event->attr.freq && (value & (1ULL << 63)))
>> + return -EINVAL;
>> +
>> event_function_call(event, __perf_event_period, &value);
>>
>> return 0;
>>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [tip:perf/urgent] perf/ioctl: Add check for the sample_period value
2019-06-04 4:29 ` Ravi Bangoria
(?)
(?)
@ 2019-06-25 8:19 ` tip-bot for Ravi Bangoria
-1 siblings, 0 replies; 24+ messages in thread
From: tip-bot for Ravi Bangoria @ 2019-06-25 8:19 UTC (permalink / raw)
To: linux-tip-commits
Cc: jolsa, hpa, peterz, alexander.shishkin, eranian, mingo,
ravi.bangoria, linux-kernel, tglx, vincent.weaver, acme,
torvalds
Commit-ID: 913a90bc5a3a06b1f04c337320e9aeee2328dd77
Gitweb: https://git.kernel.org/tip/913a90bc5a3a06b1f04c337320e9aeee2328dd77
Author: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
AuthorDate: Tue, 4 Jun 2019 09:59:53 +0530
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 24 Jun 2019 19:19:22 +0200
perf/ioctl: Add check for the sample_period value
perf_event_open() limits the sample_period to 63 bits. See:
0819b2e30ccb ("perf: Limit perf_event_attr::sample_period to 63 bits")
Make ioctl() consistent with it.
Also on PowerPC, negative sample_period could cause a recursive
PMIs leading to a hang (reported when running perf-fuzzer).
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vince Weaver <vincent.weaver@maine.edu>
Cc: acme@kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: maddy@linux.vnet.ibm.com
Cc: mpe@ellerman.id.au
Fixes: 0819b2e30ccb ("perf: Limit perf_event_attr::sample_period to 63 bits")
Link: https://lkml.kernel.org/r/20190604042953.914-1-ravi.bangoria@linux.ibm.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/events/core.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 2e32faac5511..8d1c62df20a7 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -5005,6 +5005,9 @@ static int perf_event_period(struct perf_event *event, u64 __user *arg)
if (perf_event_check_period(event, value))
return -EINVAL;
+ if (!event->attr.freq && (value & (1ULL << 63)))
+ return -EINVAL;
+
event_function_call(event, __perf_event_period, &value);
return 0;
^ permalink raw reply related [flat|nested] 24+ messages in thread
end of thread, other threads:[~2019-06-25 8:19 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-11 2:42 [PATCH 1/2] perf ioctl: Add check for the sample_period value Ravi Bangoria
2019-05-11 2:42 ` Ravi Bangoria
2019-05-11 2:42 ` [PATCH 2/2] powerpc/perf: Fix mmcra corruption by bhrb_filter Ravi Bangoria
2019-05-11 2:42 ` Ravi Bangoria
2019-05-11 2:47 ` Ravi Bangoria
2019-05-11 2:47 ` Ravi Bangoria
2019-05-22 5:01 ` Madhavan Srinivasan
2019-05-22 5:01 ` Madhavan Srinivasan
2019-05-25 0:54 ` Michael Ellerman
2019-05-13 7:42 ` [PATCH 1/2] perf ioctl: Add check for the sample_period value Peter Zijlstra
2019-05-13 7:42 ` Peter Zijlstra
2019-05-13 8:56 ` Peter Zijlstra
2019-05-13 8:56 ` Peter Zijlstra
2019-05-13 10:07 ` Ravi Bangoria
2019-05-13 10:07 ` Ravi Bangoria
2019-05-28 9:50 ` Michael Ellerman
2019-05-28 9:50 ` Michael Ellerman
2019-06-04 4:29 ` [PATCH v2] " Ravi Bangoria
2019-06-04 4:29 ` Ravi Bangoria
2019-06-17 8:38 ` Ravi Bangoria
2019-06-17 8:38 ` Ravi Bangoria
2019-06-18 12:28 ` Michael Ellerman
2019-06-18 12:28 ` Michael Ellerman
2019-06-25 8:19 ` [tip:perf/urgent] perf/ioctl: " tip-bot for Ravi Bangoria
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.