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

[Srikar Dronamraju <srikar@linux.vnet.ibm.com>]

> >
> > The above weak function should work with ppc.
> 
> I don't think so. Even if I know nothing about !x86.
> 
> > Infact I see only 2 arch
> > that define CONFIG_STACK_GROWSUP
> 
> Ah, please forget about GROWSUP, this is not the problem.
> 

Ok

> > We even seem to use this assumption when kprobe_tracer/uprobe_tracer
> > fetch arguments from stack. See fetch_kernel_stack_address() /
> > fetch_user_stack_address() and get_user_stack_nth().
> 
> But this all is completely different.
> 
> 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. 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().

In ppc,
- Stack grows from higher addresses down towards lower addresses.
- Most function invocations create a new stack frame
- Except for leaf functions that don't have many local variables

I dont see a relation why storing the return address in the link
register would cause issues with arch_uretprobe_is_alive().
So I am pretty sure that arch_uretprobe_is_alive should work.

> Just for example. Lets look at prepare_uretprobe(). Suppose it adds the
> new return_instance to ->return_instances list. Note that on 86
> arch_uretprobe_is_alive(&new_ri->auret) is obviously (and correctly) true.
> Is it also true on powerpc? I am not sure, I think it is not. Yes, this
> doesn't really matter in prepare_uretprobe(), but this will matter if
> the new ret-addr won't be saved on stack when we hit the next bp.
> 
> 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.

> 
> If x86 can share the same code with (say) powerpc, we can always cleanup
> this later, this is trivial. Right now I'd like to ensure that if the
> same or similar logic can work on powerpc, it only needs to touch the
> code in arch/powerpc.
> 
> Oleg.
> 

-- 
Thanks and Regards
Srikar Dronamraju