* [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).