Re: [PATCH 4/5 v3] ftrace/x86_32: Clean up ftrace_regs_caller

From: Steven Rostedt <rostedt@goodmis.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Ingo Molnar <mingo@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Andy Lutomirski <luto@amacapital.net>,
	Josh Poimboeuf <jpoimboe@redhat.com>
Subject: Re: [PATCH 4/5 v3] ftrace/x86_32: Clean up ftrace_regs_caller
Date: Thu, 16 Mar 2017 15:49:00 -0400	[thread overview]
Message-ID: <20170316154900.47de5e34@gandalf.local.home> (raw)
In-Reply-To: <CA+55aFyZGC90uTgwq8T1oy7_Ld_AbJqa3vzVqsM-2iLrtKFYaA@mail.gmail.com>

On Thu, 16 Mar 2017 12:28:13 -0700
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Thu, Mar 16, 2017 at 12:19 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > The thing is we don't return, we jump to the location that may be
> > modified to run the function graph tracer.  
> 
> Hmm.
> 
> How about just making the stack frame a tiny bit bigger, and getting
> rid of *all* the games.

Unfortunately, x86_32 causes us to play these games. The
ftrace_regs_caller is used by kprobes to simulate an int3 breakpoint.
That means I need to have it behave exactly the same as int3. On
x86_32, the int3 does not save ESP or SS for that matter, but instead
kprobes, et al. uses the address of the regs->sp parameter to get the
stack location when the interrupt triggered (coming from kernel).

That means, I can't duplicate pt_regs any place else. The pt_regs passed
into the caller (kprobes) has to have &regs->esp be equal to the stack
address when the int3 was hit. Or in this case, when fentry was called.

> 
> IOW, just duplicate the return address, and make the entry code do
> 
>         pushfl
>         pushl   $__KERNEL_CS
>         pushl   8(%esp)         /* Save the return ip *again* */
>         pushl   $0
>         pushl   %gs
>         pushl   %fs
>         pushl   %es
>         pushl   %ds
>         pushl   %eax
>         ....
> 
> and not have any silly code to modify the old stack frame at all. Just
> skip the values (all the segments, ORIG_EAX, duplicated return
> address, __KERNEL_CS), and you can finish off with a "popf", and all
> you have left i the original return ip that you didn't touch.

Most likely those will not be touched, but as ftrace_regs_caller (which
is much heavier weight than ftrace_caller) must act the same as an
int3. If a kprobes callback modifies any of those, the
ftrace_regs_caller must act the same as if it was done by int3.

Now, most likely a kprobe's callback will never touch those. But we
never know.

-- Steve