linux-kernel - Re: [RFC][PATCH 1/2 v2] ftrace/x86: Allow for arguments to be passed in to REGS by default

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] [day] [month] [year] [list]

Date:   Thu, 29 Oct 2020 17:07:36 +0900
From:   Masami Hiramatsu <mhiramat@...nel.org>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     linux-kernel@...r.kernel.org,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Jiri Kosina <jikos@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...nel.org>,
        Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Jiri Olsa <jolsa@...hat.com>,
        Josh Poimboeuf <jpoimboe@...hat.com>
Subject: Re: [RFC][PATCH 1/2 v2] ftrace/x86: Allow for arguments to be
 passed in to REGS by default

On Wed, 28 Oct 2020 11:29:16 -0400
Steven Rostedt <rostedt@...dmis.org> wrote:

> Hi Masami,
> 
> Talking with Peter and Thomas on IRC, where they really don't like
> passing a partial pt_regs around, got me thinking of redoing the REGS
> parameter of ftrace. Kprobes is the only user that requires the full
> registers being saved, and that's only because some kprobe user might
> want them.

Yes, kprobes can be used for debugging use case. I think we can skip
coller-saved registers (because ftrace is embedded by compiler and it
may save such registers correctly), but we still need a pt_regs on memory
to access it.

> 
> On Wed, 28 Oct 2020 10:25:02 -0400
> Steven Rostedt <rostedt@...dmis.org> wrote:
> >
> >  typedef void (*ftrace_func_t)(unsigned long ip, unsigned long parent_ip,
> >  			      struct ftrace_ops *op, struct pt_regs *regs);
> >  
> 
> 
> Most registers of pt_regs at a start of a function is rather useless.
> What if we got rid of FL_SAVE_REGS all together and had a "ftrace_regs"
> structure passed in that would have only access to all the argument
> registers, the stack pointer and the instruction pointer?

Yeah, that's OK to me. If someone wants to investigate suspicious compiler
bug, they should not put a kprobe on the fentry call site. (in most cases
symbol+5 will put a breakpoint right after fentry nop)

> Then kprobes could just create its own pt_regs, fill in all the data
> from ftrace_regs and then fill the rest with zeros or possibly whatever
> the values currently are (does it really matter what those registers
> are?), including flags.

That sounds good to me.

> 
> Not only would this simplify the code, it would probably allow moving
> more of the kprobe code from the arch specific to the generic code, and
> remove a lot of duplication.

Ah, right.

> 
> This would also help speed up the processing of live kernel patching.
> 
> And best of all, it would give everything access to the arguments of a
> function and a stack pointer with out (ab)using pt_regs.
> 
> Do you think this would be feasible?

Yes, I agreed.

Thank you,

> 
> -- Steve


-- 
Masami Hiramatsu <mhiramat@...nel.org>