* [PATCH bpf] bpf: Force kprobe multi expected_attach_type for kprobe_multi link
@ 2023-06-13 11:31 Jiri Olsa
2023-06-13 14:35 ` Yonghong Song
2023-06-16 16:53 ` Andrii Nakryiko
0 siblings, 2 replies; 4+ messages in thread
From: Jiri Olsa @ 2023-06-13 11:31 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo
We currently allow to create perf link for program with
expected_attach_type == BPF_TRACE_KPROBE_MULTI.
This will cause crash when we call helpers like get_attach_cookie or
get_func_ip in such program, because it will call the kprobe_multi's
version (current->bpf_ctx context setup) of those helpers while it
expects perf_link's current->bpf_ctx context setup.
Making sure that we use BPF_TRACE_KPROBE_MULTI expected_attach_type
only for programs attaching through kprobe_multi link.
Fixes: ca74823c6e16 ("bpf: Add cookie support to programs attached with kprobe multi link")
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
kernel/bpf/syscall.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 0c21d0d8efe4..e8fe04a5db93 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -4675,6 +4675,11 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr)
ret = bpf_perf_link_attach(attr, prog);
break;
case BPF_PROG_TYPE_KPROBE:
+ if (prog->expected_attach_type == BPF_TRACE_KPROBE_MULTI &&
+ attr->link_create.attach_type != BPF_TRACE_KPROBE_MULTI) {
+ ret = -EINVAL;
+ goto out;
+ }
if (attr->link_create.attach_type == BPF_PERF_EVENT)
ret = bpf_perf_link_attach(attr, prog);
else
--
2.40.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH bpf] bpf: Force kprobe multi expected_attach_type for kprobe_multi link
2023-06-13 11:31 [PATCH bpf] bpf: Force kprobe multi expected_attach_type for kprobe_multi link Jiri Olsa
@ 2023-06-13 14:35 ` Yonghong Song
2023-06-16 16:53 ` Andrii Nakryiko
1 sibling, 0 replies; 4+ messages in thread
From: Yonghong Song @ 2023-06-13 14:35 UTC (permalink / raw)
To: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo
On 6/13/23 4:31 AM, Jiri Olsa wrote:
> We currently allow to create perf link for program with
> expected_attach_type == BPF_TRACE_KPROBE_MULTI.
>
> This will cause crash when we call helpers like get_attach_cookie or
> get_func_ip in such program, because it will call the kprobe_multi's
> version (current->bpf_ctx context setup) of those helpers while it
> expects perf_link's current->bpf_ctx context setup.
>
> Making sure that we use BPF_TRACE_KPROBE_MULTI expected_attach_type
> only for programs attaching through kprobe_multi link.
>
> Fixes: ca74823c6e16 ("bpf: Add cookie support to programs attached with kprobe multi link")
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Acked-by: Yonghong Song <yhs@fb.com>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH bpf] bpf: Force kprobe multi expected_attach_type for kprobe_multi link
2023-06-13 11:31 [PATCH bpf] bpf: Force kprobe multi expected_attach_type for kprobe_multi link Jiri Olsa
2023-06-13 14:35 ` Yonghong Song
@ 2023-06-16 16:53 ` Andrii Nakryiko
2023-06-17 19:16 ` Jiri Olsa
1 sibling, 1 reply; 4+ messages in thread
From: Andrii Nakryiko @ 2023-06-16 16:53 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo
On Tue, Jun 13, 2023 at 4:31 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> We currently allow to create perf link for program with
> expected_attach_type == BPF_TRACE_KPROBE_MULTI.
>
> This will cause crash when we call helpers like get_attach_cookie or
> get_func_ip in such program, because it will call the kprobe_multi's
> version (current->bpf_ctx context setup) of those helpers while it
> expects perf_link's current->bpf_ctx context setup.
>
> Making sure that we use BPF_TRACE_KPROBE_MULTI expected_attach_type
> only for programs attaching through kprobe_multi link.
>
> Fixes: ca74823c6e16 ("bpf: Add cookie support to programs attached with kprobe multi link")
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> kernel/bpf/syscall.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 0c21d0d8efe4..e8fe04a5db93 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -4675,6 +4675,11 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr)
> ret = bpf_perf_link_attach(attr, prog);
> break;
> case BPF_PROG_TYPE_KPROBE:
> + if (prog->expected_attach_type == BPF_TRACE_KPROBE_MULTI &&
> + attr->link_create.attach_type != BPF_TRACE_KPROBE_MULTI) {
> + ret = -EINVAL;
> + goto out;
> + }
there is a separate expected attach type validation switch above this,
shouldn't this go there? We also have
bpf_prog_attach_check_attach_type() call above as well, and tbh by now
I'm not sure why we have like three places to check conditions like
this... But I'd put this check in either
bpf_prog_attach_check_attach_type() or in the dedicated switch for
attach_type checks.
> if (attr->link_create.attach_type == BPF_PERF_EVENT)
> ret = bpf_perf_link_attach(attr, prog);
> else
> --
> 2.40.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH bpf] bpf: Force kprobe multi expected_attach_type for kprobe_multi link
2023-06-16 16:53 ` Andrii Nakryiko
@ 2023-06-17 19:16 ` Jiri Olsa
0 siblings, 0 replies; 4+ messages in thread
From: Jiri Olsa @ 2023-06-17 19:16 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo
On Fri, Jun 16, 2023 at 09:53:00AM -0700, Andrii Nakryiko wrote:
> On Tue, Jun 13, 2023 at 4:31 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > We currently allow to create perf link for program with
> > expected_attach_type == BPF_TRACE_KPROBE_MULTI.
> >
> > This will cause crash when we call helpers like get_attach_cookie or
> > get_func_ip in such program, because it will call the kprobe_multi's
> > version (current->bpf_ctx context setup) of those helpers while it
> > expects perf_link's current->bpf_ctx context setup.
> >
> > Making sure that we use BPF_TRACE_KPROBE_MULTI expected_attach_type
> > only for programs attaching through kprobe_multi link.
> >
> > Fixes: ca74823c6e16 ("bpf: Add cookie support to programs attached with kprobe multi link")
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> > kernel/bpf/syscall.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> > index 0c21d0d8efe4..e8fe04a5db93 100644
> > --- a/kernel/bpf/syscall.c
> > +++ b/kernel/bpf/syscall.c
> > @@ -4675,6 +4675,11 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr)
> > ret = bpf_perf_link_attach(attr, prog);
> > break;
> > case BPF_PROG_TYPE_KPROBE:
> > + if (prog->expected_attach_type == BPF_TRACE_KPROBE_MULTI &&
> > + attr->link_create.attach_type != BPF_TRACE_KPROBE_MULTI) {
> > + ret = -EINVAL;
> > + goto out;
> > + }
>
> there is a separate expected attach type validation switch above this,
> shouldn't this go there? We also have
> bpf_prog_attach_check_attach_type() call above as well, and tbh by now
> I'm not sure why we have like three places to check conditions like
> this... But I'd put this check in either
> bpf_prog_attach_check_attach_type() or in the dedicated switch for
> attach_type checks.
>
bpf_prog_attach_check_attach_type looks good, will move it there
thanks,
jirka
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-06-17 19:17 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-13 11:31 [PATCH bpf] bpf: Force kprobe multi expected_attach_type for kprobe_multi link Jiri Olsa
2023-06-13 14:35 ` Yonghong Song
2023-06-16 16:53 ` Andrii Nakryiko
2023-06-17 19:16 ` Jiri Olsa
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox