* [PATCH v3 net-next] bpf: fix bpf_perf_event_read() helper
@ 2015-10-23 0:10 Alexei Starovoitov
2015-10-23 2:21 ` Wangnan (F)
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Alexei Starovoitov @ 2015-10-23 0:10 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Peter Zijlstra, Wang Nan, He Kuang, Kaixu Xia,
Daniel Borkmann, netdev, linux-kernel
Fix safety checks for bpf_perf_event_read():
- only non-inherited events can be added to perf_event_array map
(do this check statically at map insertion time)
- dynamically check that event is local and !pmu->count
Otherwise buggy bpf program can cause kernel splat.
Also fix error path after perf_event_attrs()
and remove redundant 'extern'.
Fixes: 35578d798400 ("bpf: Implement function bpf_perf_event_read() that get the selected hardware PMU conuter")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
v2->v3:
. refactor checks based on Wangnan's and Peter's feedback
while refactoring realized that these two issues need fixes as well:
. fix perf_event_attrs() error path
. remove redundant extern
v1->v2: fix compile in case of !CONFIG_PERF_EVENTS
Even in the worst case the crash is not possible.
Only warn_on_once, so imo net-next is ok.
include/linux/bpf.h | 1 -
kernel/bpf/arraymap.c | 25 ++++++++++++++++---------
kernel/trace/bpf_trace.c | 7 ++++++-
3 files changed, 22 insertions(+), 11 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index e3a51b74e275..75718fa28260 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -194,7 +194,6 @@ extern const struct bpf_func_proto bpf_map_lookup_elem_proto;
extern const struct bpf_func_proto bpf_map_update_elem_proto;
extern const struct bpf_func_proto bpf_map_delete_elem_proto;
-extern const struct bpf_func_proto bpf_perf_event_read_proto;
extern const struct bpf_func_proto bpf_get_prandom_u32_proto;
extern const struct bpf_func_proto bpf_get_smp_processor_id_proto;
extern const struct bpf_func_proto bpf_tail_call_proto;
diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
index e3cfe46b074f..3f4c99e06c6b 100644
--- a/kernel/bpf/arraymap.c
+++ b/kernel/bpf/arraymap.c
@@ -292,16 +292,23 @@ static void *perf_event_fd_array_get_ptr(struct bpf_map *map, int fd)
attr = perf_event_attrs(event);
if (IS_ERR(attr))
- return (void *)attr;
+ goto err;
- if (attr->type != PERF_TYPE_RAW &&
- !(attr->type == PERF_TYPE_SOFTWARE &&
- attr->config == PERF_COUNT_SW_BPF_OUTPUT) &&
- attr->type != PERF_TYPE_HARDWARE) {
- perf_event_release_kernel(event);
- return ERR_PTR(-EINVAL);
- }
- return event;
+ if (attr->inherit)
+ goto err;
+
+ if (attr->type == PERF_TYPE_RAW)
+ return event;
+
+ if (attr->type == PERF_TYPE_HARDWARE)
+ return event;
+
+ if (attr->type == PERF_TYPE_SOFTWARE &&
+ attr->config == PERF_COUNT_SW_BPF_OUTPUT)
+ return event;
+err:
+ perf_event_release_kernel(event);
+ return ERR_PTR(-EINVAL);
}
static void perf_event_fd_array_put_ptr(void *ptr)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 47febbe7998e..003df3887287 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -199,6 +199,11 @@ static u64 bpf_perf_event_read(u64 r1, u64 index, u64 r3, u64 r4, u64 r5)
if (!event)
return -ENOENT;
+ /* make sure event is local and doesn't have pmu::count */
+ if (event->oncpu != smp_processor_id() ||
+ event->pmu->count)
+ return -EINVAL;
+
/*
* we don't know if the function is run successfully by the
* return value. It can be judged in other places, such as
@@ -207,7 +212,7 @@ static u64 bpf_perf_event_read(u64 r1, u64 index, u64 r3, u64 r4, u64 r5)
return perf_event_read_local(event);
}
-const struct bpf_func_proto bpf_perf_event_read_proto = {
+static const struct bpf_func_proto bpf_perf_event_read_proto = {
.func = bpf_perf_event_read,
.gpl_only = false,
.ret_type = RET_INTEGER,
--
1.7.9.5
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3 net-next] bpf: fix bpf_perf_event_read() helper
2015-10-23 0:10 [PATCH v3 net-next] bpf: fix bpf_perf_event_read() helper Alexei Starovoitov
@ 2015-10-23 2:21 ` Wangnan (F)
2015-10-23 2:30 ` Alexei Starovoitov
2015-10-23 3:47 ` Wangnan (F)
` (2 subsequent siblings)
3 siblings, 1 reply; 13+ messages in thread
From: Wangnan (F) @ 2015-10-23 2:21 UTC (permalink / raw)
To: Alexei Starovoitov, David S. Miller
Cc: Ingo Molnar, Peter Zijlstra, He Kuang, Kaixu Xia,
Daniel Borkmann, netdev, linux-kernel
On 2015/10/23 8:10, Alexei Starovoitov wrote:
> Fix safety checks for bpf_perf_event_read():
> - only non-inherited events can be added to perf_event_array map
> (do this check statically at map insertion time)
> - dynamically check that event is local and !pmu->count
> Otherwise buggy bpf program can cause kernel splat.
>
> Also fix error path after perf_event_attrs()
> and remove redundant 'extern'.
>
> Fixes: 35578d798400 ("bpf: Implement function bpf_perf_event_read() that get the selected hardware PMU conuter")
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
> v2->v3:
> . refactor checks based on Wangnan's and Peter's feedback
> while refactoring realized that these two issues need fixes as well:
> . fix perf_event_attrs() error path
> . remove redundant extern
>
> v1->v2: fix compile in case of !CONFIG_PERF_EVENTS
>
> Even in the worst case the crash is not possible.
> Only warn_on_once, so imo net-next is ok.
>
> include/linux/bpf.h | 1 -
> kernel/bpf/arraymap.c | 25 ++++++++++++++++---------
> kernel/trace/bpf_trace.c | 7 ++++++-
> 3 files changed, 22 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index e3a51b74e275..75718fa28260 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -194,7 +194,6 @@ extern const struct bpf_func_proto bpf_map_lookup_elem_proto;
> extern const struct bpf_func_proto bpf_map_update_elem_proto;
> extern const struct bpf_func_proto bpf_map_delete_elem_proto;
>
> -extern const struct bpf_func_proto bpf_perf_event_read_proto;
> extern const struct bpf_func_proto bpf_get_prandom_u32_proto;
> extern const struct bpf_func_proto bpf_get_smp_processor_id_proto;
> extern const struct bpf_func_proto bpf_tail_call_proto;
> diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
> index e3cfe46b074f..3f4c99e06c6b 100644
> --- a/kernel/bpf/arraymap.c
> +++ b/kernel/bpf/arraymap.c
> @@ -292,16 +292,23 @@ static void *perf_event_fd_array_get_ptr(struct bpf_map *map, int fd)
>
> attr = perf_event_attrs(event);
> if (IS_ERR(attr))
> - return (void *)attr;
> + goto err;
>
> - if (attr->type != PERF_TYPE_RAW &&
> - !(attr->type == PERF_TYPE_SOFTWARE &&
> - attr->config == PERF_COUNT_SW_BPF_OUTPUT) &&
> - attr->type != PERF_TYPE_HARDWARE) {
> - perf_event_release_kernel(event);
> - return ERR_PTR(-EINVAL);
> - }
> - return event;
> + if (attr->inherit)
> + goto err;
> +
Since Peter suggest it is pointless for a system-wide perf_event
has inherit bit set [1], I think it should be safe to enable
system-wide perf_event pass this check?
I'll check code to make sure.
[1]
http://lkml.kernel.org/r/20151022124142.GQ17308@twins.programming.kicks-ass.net
> + if (attr->type == PERF_TYPE_RAW)
> + return event;
> +
> + if (attr->type == PERF_TYPE_HARDWARE)
> + return event;
> +
> + if (attr->type == PERF_TYPE_SOFTWARE &&
> + attr->config == PERF_COUNT_SW_BPF_OUTPUT)
> + return event;
> +err:
> + perf_event_release_kernel(event);
> + return ERR_PTR(-EINVAL);
> }
>
> static void perf_event_fd_array_put_ptr(void *ptr)
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 47febbe7998e..003df3887287 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -199,6 +199,11 @@ static u64 bpf_perf_event_read(u64 r1, u64 index, u64 r3, u64 r4, u64 r5)
> if (!event)
> return -ENOENT;
>
> + /* make sure event is local and doesn't have pmu::count */
> + if (event->oncpu != smp_processor_id() ||
> + event->pmu->count)
> + return -EINVAL;
> +
> /*
> * we don't know if the function is run successfully by the
> * return value. It can be judged in other places, such as
> @@ -207,7 +212,7 @@ static u64 bpf_perf_event_read(u64 r1, u64 index, u64 r3, u64 r4, u64 r5)
> return perf_event_read_local(event);
> }
>
> -const struct bpf_func_proto bpf_perf_event_read_proto = {
> +static const struct bpf_func_proto bpf_perf_event_read_proto = {
> .func = bpf_perf_event_read,
> .gpl_only = false,
> .ret_type = RET_INTEGER,
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 net-next] bpf: fix bpf_perf_event_read() helper
2015-10-23 2:21 ` Wangnan (F)
@ 2015-10-23 2:30 ` Alexei Starovoitov
0 siblings, 0 replies; 13+ messages in thread
From: Alexei Starovoitov @ 2015-10-23 2:30 UTC (permalink / raw)
To: Wangnan (F), David S. Miller
Cc: Ingo Molnar, Peter Zijlstra, He Kuang, Kaixu Xia,
Daniel Borkmann, netdev, linux-kernel
On 10/22/15 7:21 PM, Wangnan (F) wrote:
>> + if (attr->inherit)
>> + goto err;
>> +
>
> Since Peter suggest it is pointless for a system-wide perf_event
> has inherit bit set [1], I think it should be safe to enable
> system-wide perf_event pass this check?
here we don't know whether it's system wide or not, so the check
is needed.
The patch is the fix that should have been there from day one.
We must be safe first and relax later.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 net-next] bpf: fix bpf_perf_event_read() helper
2015-10-23 0:10 [PATCH v3 net-next] bpf: fix bpf_perf_event_read() helper Alexei Starovoitov
2015-10-23 2:21 ` Wangnan (F)
@ 2015-10-23 3:47 ` Wangnan (F)
2015-10-23 12:03 ` Peter Zijlstra
2015-10-27 4:50 ` David Miller
3 siblings, 0 replies; 13+ messages in thread
From: Wangnan (F) @ 2015-10-23 3:47 UTC (permalink / raw)
To: Alexei Starovoitov, David S. Miller
Cc: Ingo Molnar, Peter Zijlstra, He Kuang, Kaixu Xia,
Daniel Borkmann, netdev, linux-kernel
On 2015/10/23 8:10, Alexei Starovoitov wrote:
> Fix safety checks for bpf_perf_event_read():
> - only non-inherited events can be added to perf_event_array map
> (do this check statically at map insertion time)
> - dynamically check that event is local and !pmu->count
> Otherwise buggy bpf program can cause kernel splat.
>
> Also fix error path after perf_event_attrs()
> and remove redundant 'extern'.
>
> Fixes: 35578d798400 ("bpf: Implement function bpf_perf_event_read() that get the selected hardware PMU conuter")
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Tested-by: Wang Nan <wangnan0@huawei.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 net-next] bpf: fix bpf_perf_event_read() helper
2015-10-23 0:10 [PATCH v3 net-next] bpf: fix bpf_perf_event_read() helper Alexei Starovoitov
2015-10-23 2:21 ` Wangnan (F)
2015-10-23 3:47 ` Wangnan (F)
@ 2015-10-23 12:03 ` Peter Zijlstra
2015-10-23 14:42 ` Alexei Starovoitov
2015-10-27 4:50 ` David Miller
3 siblings, 1 reply; 13+ messages in thread
From: Peter Zijlstra @ 2015-10-23 12:03 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S. Miller, Ingo Molnar, Wang Nan, He Kuang, Kaixu Xia,
Daniel Borkmann, netdev, linux-kernel
On Thu, Oct 22, 2015 at 05:10:14PM -0700, Alexei Starovoitov wrote:
> +++ b/kernel/trace/bpf_trace.c
> @@ -199,6 +199,11 @@ static u64 bpf_perf_event_read(u64 r1, u64 index, u64 r3, u64 r4, u64 r5)
> if (!event)
> return -ENOENT;
>
> + /* make sure event is local and doesn't have pmu::count */
> + if (event->oncpu != smp_processor_id() ||
> + event->pmu->count)
> + return -EINVAL;
> +
> /*
> * we don't know if the function is run successfully by the
> * return value. It can be judged in other places, such as
I might want to go turn that into a helper function to keep !perf code
from poking around in the event itself, but its ok for now I suppose.
> @@ -207,7 +212,7 @@ static u64 bpf_perf_event_read(u64 r1, u64 index, u64 r3, u64 r4, u64 r5)
> return perf_event_read_local(event);
> }
So the bpf_perf_event_read() returns the count value, does this not also
mean that returning -EINVAL here is also 'wrong'?
I mean, sure an actual count value that high is unlikely, but its still
a broken interface.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 net-next] bpf: fix bpf_perf_event_read() helper
2015-10-23 12:03 ` Peter Zijlstra
@ 2015-10-23 14:42 ` Alexei Starovoitov
2015-10-23 14:54 ` Peter Zijlstra
2015-10-25 9:21 ` Ingo Molnar
0 siblings, 2 replies; 13+ messages in thread
From: Alexei Starovoitov @ 2015-10-23 14:42 UTC (permalink / raw)
To: Peter Zijlstra
Cc: David S. Miller, Ingo Molnar, Wang Nan, He Kuang, Kaixu Xia,
Daniel Borkmann, netdev, linux-kernel
On 10/23/15 5:03 AM, Peter Zijlstra wrote:
> So the bpf_perf_event_read() returns the count value, does this not also
> mean that returning -EINVAL here is also 'wrong'?
>
> I mean, sure an actual count value that high is unlikely, but its still
> a broken interface.
Agree. that's not pretty interface. I wish I looked at it more carefully
when it was introduced. Now it's too late to change.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 net-next] bpf: fix bpf_perf_event_read() helper
2015-10-23 14:42 ` Alexei Starovoitov
@ 2015-10-23 14:54 ` Peter Zijlstra
2015-10-25 9:21 ` Ingo Molnar
1 sibling, 0 replies; 13+ messages in thread
From: Peter Zijlstra @ 2015-10-23 14:54 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S. Miller, Ingo Molnar, Wang Nan, He Kuang, Kaixu Xia,
Daniel Borkmann, netdev, linux-kernel
On Fri, Oct 23, 2015 at 07:42:22AM -0700, Alexei Starovoitov wrote:
> On 10/23/15 5:03 AM, Peter Zijlstra wrote:
> >So the bpf_perf_event_read() returns the count value, does this not also
> >mean that returning -EINVAL here is also 'wrong'?
> >
> >I mean, sure an actual count value that high is unlikely, but its still
> >a broken interface.
>
> Agree. that's not pretty interface. I wish I looked at it more carefully
> when it was introduced. Now it's too late to change.
Right; and I figure changing the function signature is not done because
the eBPF stuff is ABI? Unfortunate indeed.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 net-next] bpf: fix bpf_perf_event_read() helper
2015-10-23 14:42 ` Alexei Starovoitov
2015-10-23 14:54 ` Peter Zijlstra
@ 2015-10-25 9:21 ` Ingo Molnar
2015-10-25 16:23 ` Alexei Starovoitov
1 sibling, 1 reply; 13+ messages in thread
From: Ingo Molnar @ 2015-10-25 9:21 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Peter Zijlstra, David S. Miller, Wang Nan, He Kuang, Kaixu Xia,
Daniel Borkmann, netdev, linux-kernel
* Alexei Starovoitov <ast@plumgrid.com> wrote:
> On 10/23/15 5:03 AM, Peter Zijlstra wrote:
> >So the bpf_perf_event_read() returns the count value, does this not also
> >mean that returning -EINVAL here is also 'wrong'?
> >
> >I mean, sure an actual count value that high is unlikely, but its still
> >a broken interface.
>
> Agree. that's not pretty interface. I wish I looked at it more carefully
> when it was introduced. Now it's too late to change.
So I really, really think eBPF needs to have an easy to use mechanism to phase out
old ABI components and introducing new (better) ones!
Then old crap can be de-emphasised and eventually removed, instead of having to
live with crap forever ...
Thanks,
Ingo
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 net-next] bpf: fix bpf_perf_event_read() helper
2015-10-25 9:21 ` Ingo Molnar
@ 2015-10-25 16:23 ` Alexei Starovoitov
2015-10-26 12:32 ` Peter Zijlstra
0 siblings, 1 reply; 13+ messages in thread
From: Alexei Starovoitov @ 2015-10-25 16:23 UTC (permalink / raw)
To: Ingo Molnar
Cc: Peter Zijlstra, David S. Miller, Wang Nan, He Kuang, Kaixu Xia,
Daniel Borkmann, netdev, linux-kernel
On 10/25/15 2:21 AM, Ingo Molnar wrote:
> Then old crap can be de-emphasised and eventually removed, instead of having to
> live with crap forever ...
strongly disagree. none of the helpers are 'crap'.
bpf_perf_event_read() muxes of -EINVAL into return value, but it's non
ambiguous to the program whether it got an error or real counter value.
So it's not pretty, but it's a reasonable trade off.
Properly written bpf programs will not be hitting the error path (which
is there for safety and protection against buggy programs) and will
consume return value without any extra checks.
bpf_perf_event_read() could have been done via passing a pointer to
stack where counter value can be stored, but that is much slower,
since program would need to init the stack and pass pointers while
helpers are not inlined, so the cost of return via stack is higher
than returning by value. In this case bpf_perf_event_read() can be hot,
so makes sense to optimize and sacrifice 'pretty' factor.
All existing helpers have use cases behind them and none overlap,
so not a single one can be 'deprecated'.
In general I don't think it's worth to make an exception in the kernel
that some interfaces are not ABI. That will give a bad impression on
the kernel overall. Either we have generic deprecation mechanism for
everything or none and my vote is for none.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 net-next] bpf: fix bpf_perf_event_read() helper
2015-10-25 16:23 ` Alexei Starovoitov
@ 2015-10-26 12:32 ` Peter Zijlstra
2015-10-26 12:54 ` Wangnan (F)
0 siblings, 1 reply; 13+ messages in thread
From: Peter Zijlstra @ 2015-10-26 12:32 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Ingo Molnar, David S. Miller, Wang Nan, He Kuang, Kaixu Xia,
Daniel Borkmann, netdev, linux-kernel
On Sun, Oct 25, 2015 at 09:23:36AM -0700, Alexei Starovoitov wrote:
> bpf_perf_event_read() muxes of -EINVAL into return value, but it's non
> ambiguous to the program whether it got an error or real counter value.
How can that be, the (u64)-EINVAL value is a valid counter value..
unlikely maybe, but still quite possible.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 net-next] bpf: fix bpf_perf_event_read() helper
2015-10-26 12:32 ` Peter Zijlstra
@ 2015-10-26 12:54 ` Wangnan (F)
2015-10-26 21:28 ` Alexei Starovoitov
0 siblings, 1 reply; 13+ messages in thread
From: Wangnan (F) @ 2015-10-26 12:54 UTC (permalink / raw)
To: Peter Zijlstra, Alexei Starovoitov
Cc: Ingo Molnar, David S. Miller, He Kuang, Kaixu Xia,
Daniel Borkmann, netdev, linux-kernel
On 2015/10/26 20:32, Peter Zijlstra wrote:
> On Sun, Oct 25, 2015 at 09:23:36AM -0700, Alexei Starovoitov wrote:
>> bpf_perf_event_read() muxes of -EINVAL into return value, but it's non
>> ambiguous to the program whether it got an error or real counter value.
> How can that be, the (u64)-EINVAL value is a valid counter value..
> unlikely maybe, but still quite possible.
In our real usecase we simply treat return value larger than
0x7fffffffffffffff
as error result. We can make it even larger, for example, to
0xffffffffffff0000.
Resuling values can be pre-processed by a script to filter potential
error result
out so it is not a very big problem for our real usecases.
For a better interface, I suggest
u64 bpf_perf_event_read(bool *perror);
which still returns counter value through its return value but put error
code
to stack. Then BPF program can pass NULL to the function if BPF problem
doesn't want to deal with error code.
Thank you.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 net-next] bpf: fix bpf_perf_event_read() helper
2015-10-26 12:54 ` Wangnan (F)
@ 2015-10-26 21:28 ` Alexei Starovoitov
0 siblings, 0 replies; 13+ messages in thread
From: Alexei Starovoitov @ 2015-10-26 21:28 UTC (permalink / raw)
To: Wangnan (F), Peter Zijlstra
Cc: Ingo Molnar, David S. Miller, He Kuang, Kaixu Xia,
Daniel Borkmann, netdev, linux-kernel
On 10/26/15 5:54 AM, Wangnan (F) wrote:
>
>
> On 2015/10/26 20:32, Peter Zijlstra wrote:
>> On Sun, Oct 25, 2015 at 09:23:36AM -0700, Alexei Starovoitov wrote:
>>> bpf_perf_event_read() muxes of -EINVAL into return value, but it's non
>>> ambiguous to the program whether it got an error or real counter value.
>> How can that be, the (u64)-EINVAL value is a valid counter value..
>> unlikely maybe, but still quite possible.
> In our real usecase we simply treat return value larger than
> 0x7fffffffffffffff
> as error result. We can make it even larger, for example, to
> 0xffffffffffff0000.
either above or write the program that index is valid, then you
don't need to check for errors.
> Resuling values can be pre-processed by a script to filter potential
> error result
> out so it is not a very big problem for our real usecases.
>
> For a better interface, I suggest
>
> u64 bpf_perf_event_read(bool *perror);
>
> which still returns counter value through its return value but put error
> code
> to stack. Then BPF program can pass NULL to the function if BPF problem
> doesn't want to deal with error code.
no. we're not going to introduce another interface for this.
The current one is fine. Don't pass incorrect index and you won't see
einval. Returning ints or bools via stack is much slower.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 net-next] bpf: fix bpf_perf_event_read() helper
2015-10-23 0:10 [PATCH v3 net-next] bpf: fix bpf_perf_event_read() helper Alexei Starovoitov
` (2 preceding siblings ...)
2015-10-23 12:03 ` Peter Zijlstra
@ 2015-10-27 4:50 ` David Miller
3 siblings, 0 replies; 13+ messages in thread
From: David Miller @ 2015-10-27 4:50 UTC (permalink / raw)
To: ast
Cc: mingo, a.p.zijlstra, wangnan0, hekuang, xiakaixu, daniel, netdev,
linux-kernel
From: Alexei Starovoitov <ast@plumgrid.com>
Date: Thu, 22 Oct 2015 17:10:14 -0700
> Fix safety checks for bpf_perf_event_read():
> - only non-inherited events can be added to perf_event_array map
> (do this check statically at map insertion time)
> - dynamically check that event is local and !pmu->count
> Otherwise buggy bpf program can cause kernel splat.
>
> Also fix error path after perf_event_attrs()
> and remove redundant 'extern'.
>
> Fixes: 35578d798400 ("bpf: Implement function bpf_perf_event_read() that get the selected hardware PMU conuter")
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Applied, although my tendancy is to agree with the sentiment that you must
respect the entire universe of valid 64-bit counter values. I do not buy
the arguments about values overlapping error codes being unlikely or not
worth worrying about.
Just FYI...
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2015-10-27 4:34 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-23 0:10 [PATCH v3 net-next] bpf: fix bpf_perf_event_read() helper Alexei Starovoitov
2015-10-23 2:21 ` Wangnan (F)
2015-10-23 2:30 ` Alexei Starovoitov
2015-10-23 3:47 ` Wangnan (F)
2015-10-23 12:03 ` Peter Zijlstra
2015-10-23 14:42 ` Alexei Starovoitov
2015-10-23 14:54 ` Peter Zijlstra
2015-10-25 9:21 ` Ingo Molnar
2015-10-25 16:23 ` Alexei Starovoitov
2015-10-26 12:32 ` Peter Zijlstra
2015-10-26 12:54 ` Wangnan (F)
2015-10-26 21:28 ` Alexei Starovoitov
2015-10-27 4:50 ` David Miller
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).