[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <659945fe-3ed8-4c1c-8d25-99a187bbda8a@linux.dev>
Date: Sun, 6 Apr 2025 07:05:54 +0800
From: Tao Chen <chen.dylane@...ux.dev>
To: Jiri Olsa <olsajiri@...il.com>
Cc: Andrii Nakryiko <andrii.nakryiko@...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
在 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@...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
>
> 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
Powered by blists - more mailing lists