linux-kernel - Re: [PATCH 03/10] ftrace: Add register_ftrace

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]

Date:   Fri, 8 Nov 2019 18:29:09 -0800
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        X86 ML <x86@...nel.org>, Nadav Amit <nadav.amit@...il.com>,
        Andy Lutomirski <luto@...nel.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Song Liu <songliubraving@...com>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Daniel Bristot de Oliveira <bristot@...hat.com>,
        Josh Poimboeuf <jpoimboe@...hat.com>
Subject: Re: [PATCH 03/10] ftrace: Add register_ftrace_direct()

On Fri, Nov 08, 2019 at 04:28:37PM -0500, Steven Rostedt wrote:
> From: "Steven Rostedt (VMware)" <rostedt@...dmis.org>
> 
> Add the start of the functionality to allow other trampolines to use the
> ftrace mcount/fentry/nop location. This adds two new functions:
> 
>  register_ftrace_direct() and unregister_ftrace_direct()
> 
> Both take two parameters: the first is the instruction address of where the
> mcount/fentry/nop exists, and the second is the trampoline to have that
> location called.
> 
> This will handle cases where ftrace is already used on that same location,
> and will make it still work, where the registered direct called trampoline
> will get called after all the registered ftrace callers are handled.
> 
> Currently, it will not allow for IP_MODIFY functions to be called at the
> same locations, which include some kprobes and live kernel patching.
> 
> At this point, no architecture supports this. This is only the start of
> implementing the framework.
> 
> Signed-off-by: Steven Rostedt (VMware) <rostedt@...dmis.org>
...
> +struct ftrace_ops direct_ops = {
> +	.func		= call_direct_funcs,
> +	.flags		= FTRACE_OPS_FL_IPMODIFY | FTRACE_OPS_FL_RECURSION_SAFE
> +			  | FTRACE_OPS_FL_DIRECT | FTRACE_OPS_FL_SAVE_REGS
> +			  | FTRACE_OPS_FL_PERMANENT,
> +};

The whole set looks great. Thank you for adding FL_PERMANENT to it.
Is there a way to do a replacement of direct call?
If I use unregister(old)+register(new) some events will be missed.
If I use register(new)+unregister(old) for short period of time both new and
old will be triggering on all cpus which will likely confuse bpf tracing.
Something like modify_ftrace_direct() should solve it. It's still racy. In a
sense that some cpus will be executing old while other cpus will be executing
new, but per-cpu there will be no double accounting. How difficult would be
to add such feature?