From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751415AbbEJMWW (ORCPT ); Sun, 10 May 2015 08:22:22 -0400 Received: from mx1.redhat.com ([209.132.183.28]:47940 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751169AbbEJMWV (ORCPT ); Sun, 10 May 2015 08:22:21 -0400 Date: Sun, 10 May 2015 14:21:47 +0200 From: Oleg Nesterov To: Srikar Dronamraju Cc: Ananth N Mavinakayanahalli , Anton Arapov , David Long , Denys Vlasenko , "Frank Ch. Eigler" , Ingo Molnar , Jan Willeke , Jim Keniston , Mark Wielaard , Pratyush Anand , linux-kernel@vger.kernel.org, Benjamin Herrenschmidt Subject: Re: [PATCH 07/10] uprobes/x86: Introduce arch_uretprobe_is_alive() Message-ID: <20150510122147.GA2493@redhat.com> References: <20150504124835.GA22462@redhat.com> <20150504124914.GA22512@redhat.com> <20150507110852.GF30396@linux.vnet.ibm.com> <20150507171119.GC18652@redhat.com> <20150508113058.GA5757@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150508113058.GA5757@linux.vnet.ibm.com> User-Agent: Mutt/1.5.18 (2008-05-17) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/08, Srikar Dronamraju wrote: > > > Yes, and we can do this without changing arch_uretprobe_hijack_return_addr() > > interface (which imo should be changed anyway, but this is off-topic). > > > > > and handle_trampoline() would call something like > > > > > > arch_uretprobe_is_alive(next->sp, regs); > > > > > > bool __weak arch_uretprobe_is_alive(unsigned long sp, struct pt_regs *regs) > > > { > > > return user_stack_pointer(regs) <= sp; > > > } > > > > The problem is, I simply do not know if this is right on !x86. > > > > And. I wanted to ensure that if (say) arch/ppc needs something else to > > save/check in hijack/alive, then this architecture can just add the new > > members in arch_uretprobe and change the arch_ helpers. > > 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. > 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. 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". 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.