LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 1/2] perf ioctl: Add check for the sample_period value
@ 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-13 7:42 ` [PATCH 1/2] perf ioctl: Add check for the sample_period value Peter Zijlstra
0 siblings, 2 replies; 13+ 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 [flat|nested] 13+ messages in thread
* [PATCH 2/2] powerpc/perf: Fix mmcra corruption by bhrb_filter
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:47 ` Ravi Bangoria
` (2 more replies)
2019-05-13 7:42 ` [PATCH 1/2] perf ioctl: Add check for the sample_period value Peter Zijlstra
1 sibling, 3 replies; 13+ 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 [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] powerpc/perf: Fix mmcra corruption by bhrb_filter
2019-05-11 2:42 ` [PATCH 2/2] powerpc/perf: Fix mmcra corruption by bhrb_filter Ravi Bangoria
@ 2019-05-11 2:47 ` Ravi Bangoria
2019-05-22 5:01 ` Madhavan Srinivasan
2019-05-25 0:54 ` Michael Ellerman
2 siblings, 0 replies; 13+ 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] 13+ messages in thread
* Re: [PATCH 1/2] perf ioctl: Add check for the sample_period value
2019-05-11 2:42 [PATCH 1/2] perf ioctl: Add check for the sample_period value Ravi Bangoria
2019-05-11 2:42 ` [PATCH 2/2] powerpc/perf: Fix mmcra corruption by bhrb_filter Ravi Bangoria
@ 2019-05-13 7:42 ` Peter Zijlstra
2019-05-13 8:56 ` Peter Zijlstra
1 sibling, 1 reply; 13+ 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] 13+ messages in thread
* Re: [PATCH 1/2] perf ioctl: Add check for the sample_period value
2019-05-13 7:42 ` [PATCH 1/2] perf ioctl: Add check for the sample_period value Peter Zijlstra
@ 2019-05-13 8:56 ` Peter Zijlstra
2019-05-13 10:07 ` Ravi Bangoria
0 siblings, 1 reply; 13+ 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] 13+ 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
2019-05-28 9:50 ` Michael Ellerman
0 siblings, 1 reply; 13+ 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] 13+ messages in thread
* Re: [PATCH 2/2] powerpc/perf: Fix mmcra corruption by bhrb_filter
2019-05-11 2:42 ` [PATCH 2/2] powerpc/perf: Fix mmcra corruption by bhrb_filter Ravi Bangoria
2019-05-11 2:47 ` Ravi Bangoria
@ 2019-05-22 5:01 ` Madhavan Srinivasan
2019-05-25 0:54 ` Michael Ellerman
2 siblings, 0 replies; 13+ 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] 13+ messages in thread
* Re: [PATCH 2/2] powerpc/perf: Fix mmcra corruption by bhrb_filter
2019-05-11 2:42 ` [PATCH 2/2] powerpc/perf: Fix mmcra corruption by bhrb_filter Ravi Bangoria
2019-05-11 2:47 ` Ravi Bangoria
2019-05-22 5:01 ` Madhavan Srinivasan
@ 2019-05-25 0:54 ` Michael Ellerman
2 siblings, 0 replies; 13+ 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] 13+ 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
2019-06-04 4:29 ` [PATCH v2] " Ravi Bangoria
0 siblings, 1 reply; 13+ 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] 13+ 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
2019-06-17 8:38 ` Ravi Bangoria
2019-06-25 8:19 ` [tip:perf/urgent] perf/ioctl: " tip-bot for Ravi Bangoria
0 siblings, 2 replies; 13+ 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 [flat|nested] 13+ messages in thread
* Re: [PATCH v2] perf ioctl: Add check for the sample_period value
2019-06-04 4:29 ` [PATCH v2] " Ravi Bangoria
@ 2019-06-17 8:38 ` Ravi Bangoria
2019-06-18 12:28 ` Michael Ellerman
2019-06-25 8:19 ` [tip:perf/urgent] perf/ioctl: " tip-bot for Ravi Bangoria
1 sibling, 1 reply; 13+ 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] 13+ 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
0 siblings, 0 replies; 13+ 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] 13+ messages in thread
* [tip:perf/urgent] perf/ioctl: Add check for the sample_period value
2019-06-04 4:29 ` [PATCH v2] " Ravi Bangoria
2019-06-17 8:38 ` Ravi Bangoria
@ 2019-06-25 8:19 ` tip-bot for Ravi Bangoria
1 sibling, 0 replies; 13+ 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 [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-06-25 8:19 UTC | newest]
Thread overview: 13+ 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 ` [PATCH 2/2] powerpc/perf: Fix mmcra corruption by bhrb_filter Ravi Bangoria
2019-05-11 2:47 ` Ravi Bangoria
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 8:56 ` Peter Zijlstra
2019-05-13 10:07 ` Ravi Bangoria
2019-05-28 9:50 ` Michael Ellerman
2019-06-04 4:29 ` [PATCH v2] " Ravi Bangoria
2019-06-17 8:38 ` Ravi Bangoria
2019-06-18 12:28 ` Michael Ellerman
2019-06-25 8:19 ` [tip:perf/urgent] perf/ioctl: " tip-bot for Ravi Bangoria
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).