[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAEf4BzadsmOTas7BdF-J+de7AqsoccY1o6e0pUBkRuWH+53DiQ@mail.gmail.com>
Date: Fri, 4 Mar 2022 15:11:01 -0800
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Jiri Olsa <jolsa@...nel.org>
Cc: Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Andrii Nakryiko <andrii@...nel.org>,
Masami Hiramatsu <mhiramat@...nel.org>,
Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
lkml <linux-kernel@...r.kernel.org>,
Martin KaFai Lau <kafai@...com>,
Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
John Fastabend <john.fastabend@...il.com>,
KP Singh <kpsingh@...omium.org>,
Steven Rostedt <rostedt@...dmis.org>
Subject: Re: [PATCH 02/10] bpf: Add multi kprobe link
On Tue, Feb 22, 2022 at 9:06 AM Jiri Olsa <jolsa@...nel.org> wrote:
>
> Adding new link type BPF_LINK_TYPE_KPROBE_MULTI that attaches kprobe
> program through fprobe API.
>
> The fprobe API allows to attach probe on multiple functions at once
> very fast, because it works on top of ftrace. On the other hand this
> limits the probe point to the function entry or return.
>
> The kprobe program gets the same pt_regs input ctx as when it's attached
> through the perf API.
>
> Adding new attach type BPF_TRACE_KPROBE_MULTI that allows attachment
> kprobe to multiple function with new link.
>
> User provides array of addresses or symbols with count to attach the
> kprobe program to. The new link_create uapi interface looks like:
>
> struct {
> __aligned_u64 syms;
> __aligned_u64 addrs;
> __u32 cnt;
> __u32 flags;
> } kprobe_multi;
>
> The flags field allows single BPF_TRACE_KPROBE_MULTI bit to create
> return multi kprobe.
>
> Signed-off-by: Masami Hiramatsu <mhiramat@...nel.org>
> Signed-off-by: Jiri Olsa <jolsa@...nel.org>
> ---
LGTM.
Acked-by: Andrii Nakryiko <andrii@...nel.org>
> include/linux/bpf_types.h | 1 +
> include/linux/trace_events.h | 6 +
> include/uapi/linux/bpf.h | 13 ++
> kernel/bpf/syscall.c | 26 +++-
> kernel/trace/bpf_trace.c | 211 +++++++++++++++++++++++++++++++++
> tools/include/uapi/linux/bpf.h | 13 ++
> 6 files changed, 265 insertions(+), 5 deletions(-)
>
[...]
> +
> +static int
> +kprobe_multi_resolve_syms(const void *usyms, u32 cnt,
> + unsigned long *addrs)
> +{
> + unsigned long addr, size;
> + const char **syms;
> + int err = -ENOMEM;
> + unsigned int i;
> + char *func;
> +
> + size = cnt * sizeof(*syms);
> + syms = kvzalloc(size, GFP_KERNEL);
> + if (!syms)
> + return -ENOMEM;
> +
> + func = kmalloc(KSYM_NAME_LEN, GFP_KERNEL);
> + if (!func)
> + goto error;
> +
> + if (copy_from_user(syms, usyms, size)) {
> + err = -EFAULT;
> + goto error;
> + }
> +
> + for (i = 0; i < cnt; i++) {
> + err = strncpy_from_user(func, syms[i], KSYM_NAME_LEN);
> + if (err == KSYM_NAME_LEN)
> + err = -E2BIG;
> + if (err < 0)
> + goto error;
> +
> + err = -EINVAL;
> + if (func[0] == '\0')
> + goto error;
wouldn't empty string be handled by kallsyms_lookup_name?
> + addr = kallsyms_lookup_name(func);
> + if (!addr)
> + goto error;
> + if (!kallsyms_lookup_size_offset(addr, &size, NULL))
> + size = MCOUNT_INSN_SIZE;
> + addr = ftrace_location_range(addr, addr + size - 1);
> + if (!addr)
> + goto error;
> + addrs[i] = addr;
> + }
> +
> + err = 0;
> +error:
> + kvfree(syms);
> + kfree(func);
> + return err;
> +}
> +
[...]
Powered by blists - more mailing lists