Re: [PATCH 07/10] uprobes/x86: Introduce arch_uretprobe_is_alive()

From: Oleg Nesterov <oleg@redhat.com>
To: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>,
	Anton Arapov <arapov@gmail.com>,
	David Long <dave.long@linaro.org>,
	Denys Vlasenko <dvlasenk@redhat.com>,
	"Frank Ch. Eigler" <fche@redhat.com>,
	Ingo Molnar <mingo@kernel.org>, Jan Willeke <willeke@de.ibm.com>,
	Jim Keniston <jkenisto@us.ibm.com>,
	Mark Wielaard <mjw@redhat.com>,
	Pratyush Anand <panand@redhat.com>,
	linux-kernel@vger.kernel.org,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>
Subject: Re: [PATCH 07/10] uprobes/x86: Introduce arch_uretprobe_is_alive()
Date: Wed, 20 May 2015 18:51:17 +0200	[thread overview]
Message-ID: <20150520165117.GA4284@redhat.com> (raw)
In-Reply-To: <20150513081123.GB5757@linux.vnet.ibm.com>

Srikar,

sorry for delay, vacation.

On 05/13, Srikar Dronamraju wrote:
>
> > No. I don't think arch_uretprobe_is_alive() above can work for powerpc,
> > at least the same way.
> >
> > The problem is, when the function is called, the ret-addr is not pushed
> > on stack. If it was, then arch_uretprobe_hijack_return_addr() on powerpc
> > is just wrong. But I guess it is correct ;)
> >
> > x86 is "simple". We know that the probed function should do "ret" and the
> > ret-addr lives on stack. This means that "regs->sp <= sp" is correct, it
> > can't be false-negative. Simply because if regs->sp > sp then *sp can be
> > never used by "ret". And everything above regs->sp can be overwritten by
> > a signal handler. powerpc/etc differs, they use the link register.
> >
>
> In ppc, the return address for the current function may not be in stack
> but in link register, but the return address for the previous functions
> end up in the stack.

Yes, yes, I understand. That is why I hope that this series can help
other arches too ;)

But note that at least this means that the "on_call" arg should be ignored,
although this is not the problem too.

> Lets assume main() had called foo(). Now when foo()
> calls bar (by using the b/bl instruction), we would save the current
> link register (that has address corresponding to main function) to the
> link register save area of the stack and update the stack pointer and
> the link register to an address to where we need to jump back in foo().

Yes. Now suppose that you ret-probe both main() and foo(). What happens
when foo() returns?

I guess it should cleanup the stack and remove the main's ret-addr from
stack, doesn't this mean that arch_uretprobe_is_alive(auret_for_main)
becomes false if we just use user_stack_pointer(regs) <= sp for every arch?
This will break handle_trampoline().

> > So. Lets do this per-arch. Try to do, actually. I am not even sure these
> > new hooks can actually help powerpc/etc. If not, we will have to switch
> > to "plan B".
>
> Okay, lets do it per-arch now and yes it can always be cleaned up later.

Yes, this just looks safer. At least this way we can't introduce the new
problems on !x86.

Oleg.