[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z-z8_HlpMk39SHUD@krava>
Date: Wed, 2 Apr 2025 11:01:48 +0200
From: Jiri Olsa <olsajiri@...il.com>
To: Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc: Tao Chen <chen.dylane@...ux.dev>, Jiri Olsa <olsajiri@...il.com>,
song@...nel.org, ast@...nel.org, daniel@...earbox.net,
andrii@...nel.org, martin.lau@...ux.dev, eddyz87@...il.com,
yonghong.song@...ux.dev, john.fastabend@...il.com,
kpsingh@...nel.org, sdf@...ichev.me, haoluo@...gle.com,
rostedt@...dmis.org, mhiramat@...nel.org,
mathieu.desnoyers@...icios.com, laoar.shao@...il.com,
bpf@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-trace-kernel@...r.kernel.org
Subject: Re: [PATCH bpf-next 2/2] bpf: Check link_create parameter for
multi_uprobe
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@...ux.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@...ux.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
> >
Powered by blists - more mailing lists