* [PATCH bpf-next 1/2] bpf: Check link_create parameter for multi_kprobe
@ 2025-03-31 9:47 Tao Chen
2025-03-31 9:47 ` [PATCH bpf-next 2/2] bpf: Check link_create parameter for multi_uprobe Tao Chen
0 siblings, 1 reply; 9+ messages in thread
From: Tao Chen @ 2025-03-31 9:47 UTC (permalink / raw)
To: song, jolsa, ast, daniel, andrii, martin.lau, eddyz87,
yonghong.song, john.fastabend, kpsingh, sdf, haoluo, rostedt,
mhiramat, mathieu.desnoyers, laoar.shao
Cc: bpf, linux-kernel, linux-trace-kernel, Tao Chen
The target_fd and flags in link_create no used in multi_kprobe
, return -EINVAL if they assigned, keep it same as other link
attach apis.
Fixes: 0dcac2725406 ("bpf: Add multi kprobe link")
Signed-off-by: Tao Chen <chen.dylane@linux.dev>
---
kernel/trace/bpf_trace.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 13bef2462..2f206a2a2 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -2993,6 +2993,9 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
if (sizeof(u64) != sizeof(void *))
return -EOPNOTSUPP;
+ if (attr->link_create.target_fd || attr->link_create.flags)
+ return -EINVAL;
+
if (!is_kprobe_multi(prog))
return -EINVAL;
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH bpf-next 2/2] bpf: Check link_create parameter for multi_uprobe 2025-03-31 9:47 [PATCH bpf-next 1/2] bpf: Check link_create parameter for multi_kprobe Tao Chen @ 2025-03-31 9:47 ` Tao Chen 2025-04-01 11:03 ` Jiri Olsa 0 siblings, 1 reply; 9+ messages in thread From: Tao Chen @ 2025-03-31 9:47 UTC (permalink / raw) To: song, jolsa, ast, daniel, andrii, martin.lau, eddyz87, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, rostedt, mhiramat, mathieu.desnoyers, laoar.shao Cc: bpf, linux-kernel, linux-trace-kernel, Tao Chen The target_fd and flags in link_create no used in multi_uprobe , return -EINVAL if they assigned, keep it same as other link attach apis. Fixes: 89ae89f53d20 ("bpf: Add multi uprobe link") Signed-off-by: Tao Chen <chen.dylane@linux.dev> --- kernel/trace/bpf_trace.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index 2f206a2a2..f7ebf17e3 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -3385,6 +3385,9 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr if (sizeof(u64) != sizeof(void *)) return -EOPNOTSUPP; + if (attr->link_create.target_fd || attr->link_create.flags) + return -EINVAL; + if (!is_uprobe_multi(prog)) return -EINVAL; -- 2.43.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next 2/2] bpf: Check link_create parameter for multi_uprobe 2025-03-31 9:47 ` [PATCH bpf-next 2/2] bpf: Check link_create parameter for multi_uprobe Tao Chen @ 2025-04-01 11:03 ` Jiri Olsa 2025-04-01 12:40 ` Tao Chen 0 siblings, 1 reply; 9+ messages in thread From: Jiri Olsa @ 2025-04-01 11:03 UTC (permalink / raw) To: Tao Chen Cc: song, ast, daniel, andrii, martin.lau, eddyz87, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, rostedt, mhiramat, mathieu.desnoyers, laoar.shao, bpf, linux-kernel, linux-trace-kernel On Mon, Mar 31, 2025 at 05:47:45PM +0800, Tao Chen wrote: > The target_fd and flags in link_create no used in multi_uprobe > , return -EINVAL if they assigned, keep it same as other link > attach apis. > > Fixes: 89ae89f53d20 ("bpf: Add multi uprobe link") > Signed-off-by: Tao Chen <chen.dylane@linux.dev> > --- > kernel/trace/bpf_trace.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > index 2f206a2a2..f7ebf17e3 100644 > --- a/kernel/trace/bpf_trace.c > +++ b/kernel/trace/bpf_trace.c > @@ -3385,6 +3385,9 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr > if (sizeof(u64) != sizeof(void *)) > return -EOPNOTSUPP; > > + if (attr->link_create.target_fd || attr->link_create.flags) > + return -EINVAL; I think the CI is failing because usdt code does uprobe multi detection with target_fd = -1 and it fails and perf-uprobe fallback will fail on not having enough file descriptors but I think at this stage we will brake some user apps by introducing this check, link ebpf go library, which passes 0 jirka > + > if (!is_uprobe_multi(prog)) > return -EINVAL; > > -- > 2.43.0 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next 2/2] bpf: Check link_create parameter for multi_uprobe 2025-04-01 11:03 ` Jiri Olsa @ 2025-04-01 12:40 ` Tao Chen 2025-04-01 22:06 ` Andrii Nakryiko 0 siblings, 1 reply; 9+ messages in thread From: Tao Chen @ 2025-04-01 12:40 UTC (permalink / raw) To: Jiri Olsa Cc: song, ast, daniel, andrii, martin.lau, eddyz87, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, rostedt, mhiramat, mathieu.desnoyers, laoar.shao, bpf, linux-kernel, linux-trace-kernel 在 2025/4/1 19:03, Jiri Olsa 写道: > On Mon, Mar 31, 2025 at 05:47:45PM +0800, Tao Chen wrote: >> The target_fd and flags in link_create no used in multi_uprobe >> , return -EINVAL if they assigned, keep it same as other link >> attach apis. >> >> Fixes: 89ae89f53d20 ("bpf: Add multi uprobe link") >> Signed-off-by: Tao Chen <chen.dylane@linux.dev> >> --- >> kernel/trace/bpf_trace.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c >> index 2f206a2a2..f7ebf17e3 100644 >> --- a/kernel/trace/bpf_trace.c >> +++ b/kernel/trace/bpf_trace.c >> @@ -3385,6 +3385,9 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr >> if (sizeof(u64) != sizeof(void *)) >> return -EOPNOTSUPP; >> >> + if (attr->link_create.target_fd || attr->link_create.flags) >> + return -EINVAL; > > I think the CI is failing because usdt code does uprobe multi detection > with target_fd = -1 and it fails and perf-uprobe fallback will fail on > not having enough file descriptors > Hi jiri As you said, i found it, thanks. static int probe_uprobe_multi_link(int token_fd) { LIBBPF_OPTS(bpf_prog_load_opts, load_opts, .expected_attach_type = BPF_TRACE_UPROBE_MULTI, .token_fd = token_fd, .prog_flags = token_fd ? BPF_F_TOKEN_FD : 0, ); LIBBPF_OPTS(bpf_link_create_opts, link_opts); struct bpf_insn insns[] = { BPF_MOV64_IMM(BPF_REG_0, 0), BPF_EXIT_INSN(), }; int prog_fd, link_fd, err; unsigned long offset = 0; prog_fd = bpf_prog_load(BPF_PROG_TYPE_KPROBE, NULL, "GPL", insns, ARRAY_SIZE(insns), &load_opts); if (prog_fd < 0) return -errno; /* Creating uprobe in '/' binary should fail with -EBADF. */ link_opts.uprobe_multi.path = "/"; link_opts.uprobe_multi.offsets = &offset; link_opts.uprobe_multi.cnt = 1; link_fd = bpf_link_create(prog_fd, -1, BPF_TRACE_UPROBE_MULTI, &link_opts); > but I think at this stage we will brake some user apps by introducing > this check, link ebpf go library, which passes 0 > So is it ok just check the flags? > jirka > > >> + >> if (!is_uprobe_multi(prog)) >> return -EINVAL; >> >> -- >> 2.43.0 >> -- Best Regards Tao Chen ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next 2/2] bpf: Check link_create parameter for multi_uprobe 2025-04-01 12:40 ` Tao Chen @ 2025-04-01 22:06 ` Andrii Nakryiko 2025-04-02 9:01 ` Jiri Olsa 0 siblings, 1 reply; 9+ messages in thread From: Andrii Nakryiko @ 2025-04-01 22:06 UTC (permalink / raw) To: Tao Chen Cc: Jiri Olsa, song, ast, daniel, andrii, martin.lau, eddyz87, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, rostedt, mhiramat, mathieu.desnoyers, laoar.shao, bpf, linux-kernel, linux-trace-kernel On Tue, Apr 1, 2025 at 5:40 AM Tao Chen <chen.dylane@linux.dev> wrote: > > 在 2025/4/1 19:03, Jiri Olsa 写道: > > On Mon, Mar 31, 2025 at 05:47:45PM +0800, Tao Chen wrote: > >> The target_fd and flags in link_create no used in multi_uprobe > >> , return -EINVAL if they assigned, keep it same as other link > >> attach apis. > >> > >> Fixes: 89ae89f53d20 ("bpf: Add multi uprobe link") > >> Signed-off-by: Tao Chen <chen.dylane@linux.dev> > >> --- > >> kernel/trace/bpf_trace.c | 3 +++ > >> 1 file changed, 3 insertions(+) > >> > >> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > >> index 2f206a2a2..f7ebf17e3 100644 > >> --- a/kernel/trace/bpf_trace.c > >> +++ b/kernel/trace/bpf_trace.c > >> @@ -3385,6 +3385,9 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr > >> if (sizeof(u64) != sizeof(void *)) > >> return -EOPNOTSUPP; > >> > >> + if (attr->link_create.target_fd || attr->link_create.flags) > >> + return -EINVAL; > > > > I think the CI is failing because usdt code does uprobe multi detection > > with target_fd = -1 and it fails and perf-uprobe fallback will fail on > > not having enough file descriptors > > > > Hi jiri > > As you said, i found it, thanks. > > static int probe_uprobe_multi_link(int token_fd) > { > LIBBPF_OPTS(bpf_prog_load_opts, load_opts, > .expected_attach_type = BPF_TRACE_UPROBE_MULTI, > .token_fd = token_fd, > .prog_flags = token_fd ? BPF_F_TOKEN_FD : 0, > ); > LIBBPF_OPTS(bpf_link_create_opts, link_opts); > struct bpf_insn insns[] = { > BPF_MOV64_IMM(BPF_REG_0, 0), > BPF_EXIT_INSN(), > }; > int prog_fd, link_fd, err; > unsigned long offset = 0; > > prog_fd = bpf_prog_load(BPF_PROG_TYPE_KPROBE, NULL, "GPL", > insns, ARRAY_SIZE(insns), &load_opts); > if (prog_fd < 0) > return -errno; > > /* Creating uprobe in '/' binary should fail with -EBADF. */ > link_opts.uprobe_multi.path = "/"; > link_opts.uprobe_multi.offsets = &offset; > link_opts.uprobe_multi.cnt = 1; > > link_fd = bpf_link_create(prog_fd, -1, BPF_TRACE_UPROBE_MULTI, > &link_opts); > > > but I think at this stage we will brake some user apps by introducing > > this check, link ebpf go library, which passes 0 > > > > So is it ok just check the flags? good catch, Jiri! Yep, let's validate just flags? pw-bot: cr > > > jirka > > > > > >> + > >> if (!is_uprobe_multi(prog)) > >> return -EINVAL; > >> > >> -- > >> 2.43.0 > >> > > > -- > Best Regards > Tao Chen > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next 2/2] bpf: Check link_create parameter for multi_uprobe 2025-04-01 22:06 ` Andrii Nakryiko @ 2025-04-02 9:01 ` Jiri Olsa 2025-04-02 19:19 ` Jiri Olsa 0 siblings, 1 reply; 9+ messages in thread From: Jiri Olsa @ 2025-04-02 9:01 UTC (permalink / raw) To: Andrii Nakryiko Cc: Tao Chen, Jiri Olsa, song, ast, daniel, andrii, martin.lau, eddyz87, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, rostedt, mhiramat, mathieu.desnoyers, laoar.shao, bpf, linux-kernel, linux-trace-kernel On Tue, Apr 01, 2025 at 03:06:22PM -0700, Andrii Nakryiko wrote: > On Tue, Apr 1, 2025 at 5:40 AM Tao Chen <chen.dylane@linux.dev> wrote: > > > > 在 2025/4/1 19:03, Jiri Olsa 写道: > > > On Mon, Mar 31, 2025 at 05:47:45PM +0800, Tao Chen wrote: > > >> The target_fd and flags in link_create no used in multi_uprobe > > >> , return -EINVAL if they assigned, keep it same as other link > > >> attach apis. > > >> > > >> Fixes: 89ae89f53d20 ("bpf: Add multi uprobe link") > > >> Signed-off-by: Tao Chen <chen.dylane@linux.dev> > > >> --- > > >> kernel/trace/bpf_trace.c | 3 +++ > > >> 1 file changed, 3 insertions(+) > > >> > > >> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > > >> index 2f206a2a2..f7ebf17e3 100644 > > >> --- a/kernel/trace/bpf_trace.c > > >> +++ b/kernel/trace/bpf_trace.c > > >> @@ -3385,6 +3385,9 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr > > >> if (sizeof(u64) != sizeof(void *)) > > >> return -EOPNOTSUPP; > > >> > > >> + if (attr->link_create.target_fd || attr->link_create.flags) > > >> + return -EINVAL; > > > > > > I think the CI is failing because usdt code does uprobe multi detection > > > with target_fd = -1 and it fails and perf-uprobe fallback will fail on > > > not having enough file descriptors > > > > > > > Hi jiri > > > > As you said, i found it, thanks. > > > > static int probe_uprobe_multi_link(int token_fd) > > { > > LIBBPF_OPTS(bpf_prog_load_opts, load_opts, > > .expected_attach_type = BPF_TRACE_UPROBE_MULTI, > > .token_fd = token_fd, > > .prog_flags = token_fd ? BPF_F_TOKEN_FD : 0, > > ); > > LIBBPF_OPTS(bpf_link_create_opts, link_opts); > > struct bpf_insn insns[] = { > > BPF_MOV64_IMM(BPF_REG_0, 0), > > BPF_EXIT_INSN(), > > }; > > int prog_fd, link_fd, err; > > unsigned long offset = 0; > > > > prog_fd = bpf_prog_load(BPF_PROG_TYPE_KPROBE, NULL, "GPL", > > insns, ARRAY_SIZE(insns), &load_opts); > > if (prog_fd < 0) > > return -errno; > > > > /* Creating uprobe in '/' binary should fail with -EBADF. */ > > link_opts.uprobe_multi.path = "/"; > > link_opts.uprobe_multi.offsets = &offset; > > link_opts.uprobe_multi.cnt = 1; > > > > link_fd = bpf_link_create(prog_fd, -1, BPF_TRACE_UPROBE_MULTI, > > &link_opts); > > > > > but I think at this stage we will brake some user apps by introducing > > > this check, link ebpf go library, which passes 0 > > > > > > > So is it ok just check the flags? > > good catch, Jiri! Yep, let's validate just flags? I think so.. I'll test that with ebpf/go to make sure we are safe at least there ;-) I'll let you know jirka > > pw-bot: cr > > > > > > jirka > > > > > > > > >> + > > >> if (!is_uprobe_multi(prog)) > > >> return -EINVAL; > > >> > > >> -- > > >> 2.43.0 > > >> > > > > > > -- > > Best Regards > > Tao Chen > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next 2/2] bpf: Check link_create parameter for multi_uprobe 2025-04-02 9:01 ` Jiri Olsa @ 2025-04-02 19:19 ` Jiri Olsa 2025-04-02 19:25 ` Jiri Olsa 2025-04-05 23:05 ` Tao Chen 0 siblings, 2 replies; 9+ messages in thread From: Jiri Olsa @ 2025-04-02 19:19 UTC (permalink / raw) To: Jiri Olsa Cc: Andrii Nakryiko, Tao Chen, song, ast, daniel, andrii, martin.lau, eddyz87, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, rostedt, mhiramat, mathieu.desnoyers, laoar.shao, bpf, linux-kernel, linux-trace-kernel On Wed, Apr 02, 2025 at 11:01:48AM +0200, Jiri Olsa wrote: > On Tue, Apr 01, 2025 at 03:06:22PM -0700, Andrii Nakryiko wrote: > > On Tue, Apr 1, 2025 at 5:40 AM Tao Chen <chen.dylane@linux.dev> wrote: > > > > > > 在 2025/4/1 19:03, Jiri Olsa 写道: > > > > On Mon, Mar 31, 2025 at 05:47:45PM +0800, Tao Chen wrote: > > > >> The target_fd and flags in link_create no used in multi_uprobe > > > >> , return -EINVAL if they assigned, keep it same as other link > > > >> attach apis. > > > >> > > > >> Fixes: 89ae89f53d20 ("bpf: Add multi uprobe link") > > > >> Signed-off-by: Tao Chen <chen.dylane@linux.dev> > > > >> --- > > > >> kernel/trace/bpf_trace.c | 3 +++ > > > >> 1 file changed, 3 insertions(+) > > > >> > > > >> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > > > >> index 2f206a2a2..f7ebf17e3 100644 > > > >> --- a/kernel/trace/bpf_trace.c > > > >> +++ b/kernel/trace/bpf_trace.c > > > >> @@ -3385,6 +3385,9 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr > > > >> if (sizeof(u64) != sizeof(void *)) > > > >> return -EOPNOTSUPP; > > > >> > > > >> + if (attr->link_create.target_fd || attr->link_create.flags) > > > >> + return -EINVAL; > > > > > > > > I think the CI is failing because usdt code does uprobe multi detection > > > > with target_fd = -1 and it fails and perf-uprobe fallback will fail on > > > > not having enough file descriptors > > > > > > > > > > Hi jiri > > > > > > As you said, i found it, thanks. > > > > > > static int probe_uprobe_multi_link(int token_fd) > > > { > > > LIBBPF_OPTS(bpf_prog_load_opts, load_opts, > > > .expected_attach_type = BPF_TRACE_UPROBE_MULTI, > > > .token_fd = token_fd, > > > .prog_flags = token_fd ? BPF_F_TOKEN_FD : 0, > > > ); > > > LIBBPF_OPTS(bpf_link_create_opts, link_opts); > > > struct bpf_insn insns[] = { > > > BPF_MOV64_IMM(BPF_REG_0, 0), > > > BPF_EXIT_INSN(), > > > }; > > > int prog_fd, link_fd, err; > > > unsigned long offset = 0; > > > > > > prog_fd = bpf_prog_load(BPF_PROG_TYPE_KPROBE, NULL, "GPL", > > > insns, ARRAY_SIZE(insns), &load_opts); > > > if (prog_fd < 0) > > > return -errno; > > > > > > /* Creating uprobe in '/' binary should fail with -EBADF. */ > > > link_opts.uprobe_multi.path = "/"; > > > link_opts.uprobe_multi.offsets = &offset; > > > link_opts.uprobe_multi.cnt = 1; > > > > > > link_fd = bpf_link_create(prog_fd, -1, BPF_TRACE_UPROBE_MULTI, > > > &link_opts); > > > > > > > but I think at this stage we will brake some user apps by introducing > > > > this check, link ebpf go library, which passes 0 > > > > > > > > > > So is it ok just check the flags? > > > > good catch, Jiri! Yep, let's validate just flags? > > I think so.. I'll test that with ebpf/go to make sure we are safe > at least there ;-) I'll let you know sorry, got stuck.. link_create.flags are initialized to zero, so I think flags check should be fine (at least for ebpf/go) jirka ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next 2/2] bpf: Check link_create parameter for multi_uprobe 2025-04-02 19:19 ` Jiri Olsa @ 2025-04-02 19:25 ` Jiri Olsa 2025-04-05 23:05 ` Tao Chen 1 sibling, 0 replies; 9+ messages in thread From: Jiri Olsa @ 2025-04-02 19:25 UTC (permalink / raw) To: Jiri Olsa, Timo Beckers Cc: Andrii Nakryiko, Tao Chen, song, ast, daniel, andrii, martin.lau, eddyz87, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, rostedt, mhiramat, mathieu.desnoyers, laoar.shao, bpf, linux-kernel, linux-trace-kernel On Wed, Apr 02, 2025 at 09:19:45PM +0200, Jiri Olsa wrote: > On Wed, Apr 02, 2025 at 11:01:48AM +0200, Jiri Olsa wrote: > > On Tue, Apr 01, 2025 at 03:06:22PM -0700, Andrii Nakryiko wrote: > > > On Tue, Apr 1, 2025 at 5:40 AM Tao Chen <chen.dylane@linux.dev> wrote: > > > > > > > > 在 2025/4/1 19:03, Jiri Olsa 写道: > > > > > On Mon, Mar 31, 2025 at 05:47:45PM +0800, Tao Chen wrote: > > > > >> The target_fd and flags in link_create no used in multi_uprobe > > > > >> , return -EINVAL if they assigned, keep it same as other link > > > > >> attach apis. > > > > >> > > > > >> Fixes: 89ae89f53d20 ("bpf: Add multi uprobe link") > > > > >> Signed-off-by: Tao Chen <chen.dylane@linux.dev> > > > > >> --- > > > > >> kernel/trace/bpf_trace.c | 3 +++ > > > > >> 1 file changed, 3 insertions(+) > > > > >> > > > > >> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > > > > >> index 2f206a2a2..f7ebf17e3 100644 > > > > >> --- a/kernel/trace/bpf_trace.c > > > > >> +++ b/kernel/trace/bpf_trace.c > > > > >> @@ -3385,6 +3385,9 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr > > > > >> if (sizeof(u64) != sizeof(void *)) > > > > >> return -EOPNOTSUPP; > > > > >> > > > > >> + if (attr->link_create.target_fd || attr->link_create.flags) > > > > >> + return -EINVAL; > > > > > > > > > > I think the CI is failing because usdt code does uprobe multi detection > > > > > with target_fd = -1 and it fails and perf-uprobe fallback will fail on > > > > > not having enough file descriptors > > > > > > > > > > > > > Hi jiri > > > > > > > > As you said, i found it, thanks. > > > > > > > > static int probe_uprobe_multi_link(int token_fd) > > > > { > > > > LIBBPF_OPTS(bpf_prog_load_opts, load_opts, > > > > .expected_attach_type = BPF_TRACE_UPROBE_MULTI, > > > > .token_fd = token_fd, > > > > .prog_flags = token_fd ? BPF_F_TOKEN_FD : 0, > > > > ); > > > > LIBBPF_OPTS(bpf_link_create_opts, link_opts); > > > > struct bpf_insn insns[] = { > > > > BPF_MOV64_IMM(BPF_REG_0, 0), > > > > BPF_EXIT_INSN(), > > > > }; > > > > int prog_fd, link_fd, err; > > > > unsigned long offset = 0; > > > > > > > > prog_fd = bpf_prog_load(BPF_PROG_TYPE_KPROBE, NULL, "GPL", > > > > insns, ARRAY_SIZE(insns), &load_opts); > > > > if (prog_fd < 0) > > > > return -errno; > > > > > > > > /* Creating uprobe in '/' binary should fail with -EBADF. */ > > > > link_opts.uprobe_multi.path = "/"; > > > > link_opts.uprobe_multi.offsets = &offset; > > > > link_opts.uprobe_multi.cnt = 1; > > > > > > > > link_fd = bpf_link_create(prog_fd, -1, BPF_TRACE_UPROBE_MULTI, > > > > &link_opts); > > > > > > > > > but I think at this stage we will brake some user apps by introducing > > > > > this check, link ebpf go library, which passes 0 > > > > > > > > > > > > > So is it ok just check the flags? > > > > > > good catch, Jiri! Yep, let's validate just flags? > > > > I think so.. I'll test that with ebpf/go to make sure we are safe > > at least there ;-) I'll let you know > > sorry, got stuck.. link_create.flags are initialized to zero, > so I think flags check should be fine (at least for ebpf/go) sry forgot.. adding Timo to the loop (ebpf/go) jirka ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next 2/2] bpf: Check link_create parameter for multi_uprobe 2025-04-02 19:19 ` Jiri Olsa 2025-04-02 19:25 ` Jiri Olsa @ 2025-04-05 23:05 ` Tao Chen 1 sibling, 0 replies; 9+ messages in thread From: Tao Chen @ 2025-04-05 23:05 UTC (permalink / raw) To: Jiri Olsa Cc: Andrii Nakryiko, song, ast, daniel, andrii, martin.lau, eddyz87, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, rostedt, mhiramat, mathieu.desnoyers, laoar.shao, bpf, linux-kernel, linux-trace-kernel 在 2025/4/3 03:19, Jiri Olsa 写道: > On Wed, Apr 02, 2025 at 11:01:48AM +0200, Jiri Olsa wrote: >> On Tue, Apr 01, 2025 at 03:06:22PM -0700, Andrii Nakryiko wrote: >>> On Tue, Apr 1, 2025 at 5:40 AM Tao Chen <chen.dylane@linux.dev> wrote: >>>> >>>> 在 2025/4/1 19:03, Jiri Olsa 写道: >>>>> On Mon, Mar 31, 2025 at 05:47:45PM +0800, Tao Chen wrote: >>>>>> The target_fd and flags in link_create no used in multi_uprobe >>>>>> , return -EINVAL if they assigned, keep it same as other link >>>>>> attach apis. >>>>>> >>>>>> Fixes: 89ae89f53d20 ("bpf: Add multi uprobe link") >>>>>> Signed-off-by: Tao Chen <chen.dylane@linux.dev> >>>>>> --- >>>>>> kernel/trace/bpf_trace.c | 3 +++ >>>>>> 1 file changed, 3 insertions(+) >>>>>> >>>>>> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c >>>>>> index 2f206a2a2..f7ebf17e3 100644 >>>>>> --- a/kernel/trace/bpf_trace.c >>>>>> +++ b/kernel/trace/bpf_trace.c >>>>>> @@ -3385,6 +3385,9 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr >>>>>> if (sizeof(u64) != sizeof(void *)) >>>>>> return -EOPNOTSUPP; >>>>>> >>>>>> + if (attr->link_create.target_fd || attr->link_create.flags) >>>>>> + return -EINVAL; >>>>> >>>>> I think the CI is failing because usdt code does uprobe multi detection >>>>> with target_fd = -1 and it fails and perf-uprobe fallback will fail on >>>>> not having enough file descriptors >>>>> >>>> >>>> Hi jiri >>>> >>>> As you said, i found it, thanks. >>>> >>>> static int probe_uprobe_multi_link(int token_fd) >>>> { >>>> LIBBPF_OPTS(bpf_prog_load_opts, load_opts, >>>> .expected_attach_type = BPF_TRACE_UPROBE_MULTI, >>>> .token_fd = token_fd, >>>> .prog_flags = token_fd ? BPF_F_TOKEN_FD : 0, >>>> ); >>>> LIBBPF_OPTS(bpf_link_create_opts, link_opts); >>>> struct bpf_insn insns[] = { >>>> BPF_MOV64_IMM(BPF_REG_0, 0), >>>> BPF_EXIT_INSN(), >>>> }; >>>> int prog_fd, link_fd, err; >>>> unsigned long offset = 0; >>>> >>>> prog_fd = bpf_prog_load(BPF_PROG_TYPE_KPROBE, NULL, "GPL", >>>> insns, ARRAY_SIZE(insns), &load_opts); >>>> if (prog_fd < 0) >>>> return -errno; >>>> >>>> /* Creating uprobe in '/' binary should fail with -EBADF. */ >>>> link_opts.uprobe_multi.path = "/"; >>>> link_opts.uprobe_multi.offsets = &offset; >>>> link_opts.uprobe_multi.cnt = 1; >>>> >>>> link_fd = bpf_link_create(prog_fd, -1, BPF_TRACE_UPROBE_MULTI, >>>> &link_opts); >>>> >>>>> but I think at this stage we will brake some user apps by introducing >>>>> this check, link ebpf go library, which passes 0 >>>>> >>>> >>>> So is it ok just check the flags? >>> >>> good catch, Jiri! Yep, let's validate just flags? >> >> I think so.. I'll test that with ebpf/go to make sure we are safe >> at least there ;-) I'll let you know > > sorry, got stuck.. link_create.flags are initialized to zero, > so I think flags check should be fine (at least for ebpf/go) Thank you very much for your detailed check. I will send it v2. > > jirka -- Best Regards Tao Chen ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-04-05 23:06 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-03-31 9:47 [PATCH bpf-next 1/2] bpf: Check link_create parameter for multi_kprobe Tao Chen 2025-03-31 9:47 ` [PATCH bpf-next 2/2] bpf: Check link_create parameter for multi_uprobe Tao Chen 2025-04-01 11:03 ` Jiri Olsa 2025-04-01 12:40 ` Tao Chen 2025-04-01 22:06 ` Andrii Nakryiko 2025-04-02 9:01 ` Jiri Olsa 2025-04-02 19:19 ` Jiri Olsa 2025-04-02 19:25 ` Jiri Olsa 2025-04-05 23:05 ` Tao Chen
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).