netdev - Re: [PATCHv2 RFC bpf-next 0/7] bpf: Add support for ftrace probe

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: <YH8GxNi5VuYjwNmK@krava>
Date:   Tue, 20 Apr 2021 18:52:20 +0200
From:   Jiri Olsa <jolsa@...hat.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     Steven Rostedt <rostedt@...dmis.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Andrii Nakryiko <andrii.nakryiko@...il.com>,
        Jiri Olsa <jolsa@...nel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andriin@...com>,
        Networking <netdev@...r.kernel.org>, bpf <bpf@...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>, Daniel Xu <dxu@...uu.xyz>,
        Jesper Brouer <jbrouer@...hat.com>,
        Toke Høiland-Jørgensen <toke@...hat.com>,
        Viktor Malik <vmalik@...hat.com>
Subject: Re: [PATCHv2 RFC bpf-next 0/7] bpf: Add support for ftrace probe

On Tue, Apr 20, 2021 at 08:33:43AM -0700, Alexei Starovoitov wrote:
> On Tue, Apr 20, 2021 at 5:51 AM Jiri Olsa <jolsa@...hat.com> wrote:
> >
> > On Fri, Apr 16, 2021 at 12:48:34PM -0400, Steven Rostedt wrote:
> > > On Sat, 17 Apr 2021 00:03:04 +0900
> > > Masami Hiramatsu <mhiramat@...nel.org> wrote:
> > >
> > > > > Anyway, IIRC, Masami wasn't sure that the full regs was ever needed for the
> > > > > return (who cares about the registers on return, except for the return
> > > > > value?)
> > > >
> > > > I think kretprobe and ftrace are for a bit different usage. kretprobe can be
> > > > used for something like debugger. In that case, accessing full regs stack
> > > > will be more preferrable. (BTW, what the not "full regs" means? Does that
> > > > save partial registers?)
> > >
> > > When the REGS flag is not set in the ftrace_ops (where kprobes uses the
> > > REGS flags), the regs parameter is not a full set of regs, but holds just
> > > enough to get access to the parameters. This just happened to be what was
> > > saved in the mcount/fentry trampoline, anyway, because tracing the start of
> > > the program, you had to save the arguments before calling the trace code,
> > > otherwise you would corrupt the parameters of the function being traced.
> > >
> > > I just tweaked it so that by default, the ftrace callbacks now have access
> > > to the saved regs (call ftrace_regs, to not let a callback get confused and
> > > think it has full regs when it does not).
> > >
> > > Now for the exit of a function, what does having the full pt_regs give you?
> > > Besides the information to get the return value, the rest of the regs are
> > > pretty much meaningless. Is there any example that someone wants access to
> > > the regs at the end of a function besides getting the return value?
> >
> > for ebpf program attached to the function exit we need the functions's
> > arguments.. so original registers from time when the function was entered,
> > we don't need registers state at the time function is returning
> >
> > as we discussed in another email, we could save input registers in
> > fgraph_ops entry handler and load them in exit handler before calling
> > ebpf program
> 
> I don't see how you can do it without BTF.
> The mass-attach feature should prepare generic 6 or so arguments
> from all functions it attached to.
> On x86-64 it's trivial because 6 regs are the same.
> On arm64 is now more challenging since return value regs overlaps with
> first argument, so bpf trampoline (when it's ready for arm64) will look
> a bit different than bpf trampoline on x86-64 to preserve arg0, arg1,
> ..arg6, ret
> 64-bit values that bpf prog expects to see.
> On x86-32 it's even more trickier, since the same 6 args need to be copied
> from a combination of regs and stack.
> This is not some hypothetical case. We already use BTF in x86-32 JIT
> and btf_func_model was introduced specifically to handle such cases.
> So I really don't see how ftrace can do that just yet. It has to understand BTF
> of all of the funcs it attaches to otherwise it's just saving all regs.
> That approach was a pain to deal with.

ok, my idea was to get regs from the ftrace and have arch specific code
to prepare 6 (or less) args for ebpf program.. that part would be
already in bpf code

so you'd like to see this functionality directly in ftrace, so we don't
save unneeded regs, is that right?

jirka

> Just look at bpf code samples with ugly per architecture macros to access regs.
> BPF trampoline solved it and I don't think going back to per-arch macros
> is an option at this point.
>