linux-kernel - Re: [PATCH bpf-next 2/2] bpf: Check link_create parameter for multi

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEf4BzbOirQiAmowckX8OeiFUTR8yfkO6m+kY96VMy5f9rG26A@mail.gmail.com>
Date: Tue, 1 Apr 2025 15:06:22 -0700
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Tao Chen <chen.dylane@...ux.dev>
Cc: 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 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?

pw-bot: cr

>
> > jirka
> >
> >
> >> +
> >>      if (!is_uprobe_multi(prog))
> >>              return -EINVAL;
> >>
> >> --
> >> 2.43.0
> >>
>
>
> --
> Best Regards
> Tao Chen
>