| 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
| ||
|
Message-ID: <YgI0E1D6v0RZa5/Z@krava>
Date: Tue, 8 Feb 2022 10:12:51 +0100
From: Jiri Olsa <jolsa@...hat.com>
To: Andrii Nakryiko <andrii.nakryiko@...il.com>
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>,
Jiri Olsa <olsajiri@...il.com>
Subject: Re: [PATCH 6/8] libbpf: Add bpf_program__attach_kprobe_opts for
multi kprobes
On Mon, Feb 07, 2022 at 10:59:29AM -0800, Andrii Nakryiko wrote:
SNIP
> > +struct fprobe_resolve {
> > + const char *name;
> > + __u64 *addrs;
> > + __u32 alloc;
> > + __u32 cnt;
> > +};
> > +
> > +static bool glob_matches(const char *glob, const char *s)
>
> we've since added more generic glob_match() implementation (see
> test_progs.c), let's copy/paste that one (it's actually shorter and
> doesn't do hacky input args modification). Let's maybe also add '?'
> handling (it's trivial). Both original code in perf and the one in
> test_progs.c are GPL-2.0-only, so let's also get acks from original
> authors.
ok, will check
>
> > +{
> > + int n = strlen(glob);
> > +
> > + if (n == 1 && glob[0] == '*')
> > + return true;
> > +
> > + if (glob[0] == '*' && glob[n - 1] == '*') {
> > + const char *subs;
> > + /* substring match */
> > +
> > + /* this is hacky, but we don't want to allocate
> > + * for no good reason
> > + */
> > + ((char *)glob)[n - 1] = '\0';
> > + subs = strstr(s, glob + 1);
> > + ((char *)glob)[n - 1] = '*';
> > +
> > + return subs != NULL;
> > + } else if (glob[0] == '*') {
> > + size_t nn = strlen(s);
> > + /* suffix match */
> > +
> > + /* too short for a given suffix */
> > + if (nn < n - 1)
> > + return false;
> > + return strcmp(s + nn - (n - 1), glob + 1) == 0;
> > + } else if (glob[n - 1] == '*') {
> > + /* prefix match */
> > + return strncmp(s, glob, n - 1) == 0;
> > + } else {
> > + /* exact match */
> > + return strcmp(glob, s) == 0;
> > + }
> > +}
> > +
> > +static int resolve_fprobe_cb(void *arg, unsigned long long sym_addr,
> > + char sym_type, const char *sym_name)
> > +{
> > + struct fprobe_resolve *res = arg;
> > + __u64 *p;
> > +
> > + if (!glob_matches(res->name, sym_name))
> > + return 0;
> > +
> > + if (res->cnt == res->alloc) {
> > + res->alloc = max((__u32) 16, res->alloc * 3 / 2);
> > + p = libbpf_reallocarray(res->addrs, res->alloc, sizeof(__u32));
> > + if (!p)
> > + return -ENOMEM;
> > + res->addrs = p;
> > + }
>
> please use libbpf_ensure_mem() instead
ok
>
>
> > + res->addrs[res->cnt++] = sym_addr;
> > + return 0;
> > +}
> > +
> > +static struct bpf_link *
> > +attach_fprobe_opts(const struct bpf_program *prog,
> > + const char *func_name,
>
> func_glob or func_pattern?
ok
>
> > + const struct bpf_kprobe_opts *kopts)
> > +{
> > + DECLARE_LIBBPF_OPTS(bpf_link_create_opts, opts);
> > + struct fprobe_resolve res = {
> > + .name = func_name,
> > + };
> > + struct bpf_link *link = NULL;
> > + char errmsg[STRERR_BUFSIZE];
> > + int err, link_fd, prog_fd;
> > + bool retprobe;
> > +
> > + err = libbpf__kallsyms_parse(&res, resolve_fprobe_cb);
> > + if (err)
> > + goto error;
> > + if (!res.cnt) {
> > + err = -ENOENT;
> > + goto error;
> > + }
> > +
> > + retprobe = OPTS_GET(kopts, retprobe, false);
> > +
> > + opts.fprobe.addrs = (__u64) res.addrs;
>
> ptr_to_u64()
ok
>
> > + opts.fprobe.cnt = res.cnt;
> > + opts.flags = retprobe ? BPF_F_FPROBE_RETURN : 0;
> > +
> > + link = calloc(1, sizeof(*link));
> > + if (!link) {
> > + err = -ENOMEM;
> > + goto error;
> > + }
> > + link->detach = &bpf_link__detach_fd;
> > +
> > + prog_fd = bpf_program__fd(prog);
> > + link_fd = bpf_link_create(prog_fd, 0, BPF_TRACE_FPROBE, &opts);
> > + if (link_fd < 0) {
> > + err = -errno;
> > + pr_warn("prog '%s': failed to attach to %s: %s\n",
> > + prog->name, res.name,
> > + libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> > + goto error;
> > + }
> > + link->fd = link_fd;
> > + free(res.addrs);
> > + return link;
> > +
> > +error:
> > + free(link);
> > + free(res.addrs);
> > + return libbpf_err_ptr(err);
> > +}
> > +
> > struct bpf_link *
> > bpf_program__attach_kprobe_opts(const struct bpf_program *prog,
> > const char *func_name,
> > @@ -10047,6 +10166,9 @@ bpf_program__attach_kprobe_opts(const struct bpf_program *prog,
> > if (!OPTS_VALID(opts, bpf_kprobe_opts))
> > return libbpf_err_ptr(-EINVAL);
> >
> > + if (prog->expected_attach_type == BPF_TRACE_FPROBE)
> > + return attach_fprobe_opts(prog, func_name, opts);
> > +
> > retprobe = OPTS_GET(opts, retprobe, false);
> > offset = OPTS_GET(opts, offset, 0);
> > pe_opts.bpf_cookie = OPTS_GET(opts, bpf_cookie, 0);
> > @@ -10112,6 +10234,14 @@ struct bpf_link *bpf_program__attach_kprobe(const struct bpf_program *prog,
> > return bpf_program__attach_kprobe_opts(prog, func_name, &opts);
> > }
> >
> > +static int init_kprobe(struct bpf_program *prog, long cookie)
> > +{
> > + /* If we have wildcard, switch to fprobe link. */
> > + if (strchr(prog->sec_name, '*'))
>
> ugh... :( maybe let's have a separate SEC("kprobe.multi/<glob>") and
> same for kretprobe?
I agree new SEC type is more clear ;-) ok
thanks,
jirka
Powered by blists - more mailing lists