From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.1 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6AA5DC2D0DC for ; Thu, 26 Dec 2019 09:26:15 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 3C12B20838 for ; Thu, 26 Dec 2019 09:26:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1577352375; bh=6asAChZGLqkA6kNuizNIWoIAN2lunfQ0xFCnSMNqwvU=; h=Date:From:To:Cc:Subject:In-Reply-To:References:List-ID:From; b=0vfdPtEg56F7XAw7SExfTejKBQZ/W2WY5V9xI6/05AVIB/4HSTdSpcvWSR6yR/q0t LIrEkOPhy+tTrJSCR32V2UM2i/eR4Nl1TgfVcdV5+mKK7PJCoEiHKkPGiuVBEF+bJw 3AsL95eKCP9GSXzScxSOyjaOCkdyoongmqYbmyt8= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726579AbfLZJ0O (ORCPT ); Thu, 26 Dec 2019 04:26:14 -0500 Received: from mail.kernel.org ([198.145.29.99]:55464 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726023AbfLZJ0O (ORCPT ); Thu, 26 Dec 2019 04:26:14 -0500 Received: from devnote2 (NE2965lan1.rev.em-net.ne.jp [210.141.244.193]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id BA2AA20828; Thu, 26 Dec 2019 09:26:09 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1577352372; bh=6asAChZGLqkA6kNuizNIWoIAN2lunfQ0xFCnSMNqwvU=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=GDEijHdH+fS2OpJ8NizE3hyQLJMY7yVxc6vR8dsGzeVCgPbbShA69taRiT00O2/Q/ eYVHv071VAMVsV58S2RK4tfqDrqIX/ifiHfUexpBx69iQ9bIXU/r4CPiWQPpJNBLcZ 6myeBCBMdGencmtdph9n1UWcRWDVcpHjmXCS8EvY= Date: Thu, 26 Dec 2019 18:26:07 +0900 From: Masami Hiramatsu To: Jisheng Zhang Cc: Mark Rutland , Jonathan Corbet , Catalin Marinas , "linux-doc@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Anil S Keshavamurthy , Ingo Molnar , Steven Rostedt , "Naveen N. Rao" , Will Deacon , "David S. Miller" , "linux-arm-kernel@lists.infradead.org" Subject: Re: [PATCH v7 3/3] arm64: implement KPROBES_ON_FTRACE Message-Id: <20191226182607.06770598a00507090a046951@kernel.org> In-Reply-To: <20191226121108.0cd1b078@xhacker.debian> References: <20191225172625.69811b3e@xhacker.debian> <20191225173001.6c0e3fb2@xhacker.debian> <20191226115707.902545688aa90b34e2e550b3@kernel.org> <20191226110348.146bb80b@xhacker.debian> <20191226121108.0cd1b078@xhacker.debian> X-Mailer: Sylpheed 3.5.1 (GTK+ 2.24.32; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 26 Dec 2019 04:25:24 +0000 Jisheng Zhang wrote: > > > > +/* > > > > + * In arm64 FTRACE_WITH_REGS implementation, we patch two nop instructions: > > > > + * the lr saver and bl ftrace-entry. Both these instructions are claimed > > > > + * by ftrace and we should allow probing on either instruction. > > > > > > No, the 2nd bl ftrace-entry must not be probed. > > > The pair of lr-saver and bl ftrace-entry is tightly coupled. You can not > > > decouple it. > > > > This is the key. different viewing of this results in different implementation. > > I'm just wondering why are the two instructions considered as coupled. I think > > here we met similar situation as powerpc: https://lkml.org/lkml/2019/6/18/646 > > the "mflr r0" equals to lr-saver here, branch to _mcount equals to bl ftrace-entry > > could you please kindly comment more? > > > > Thanks in advance > > > > hmm, I think I may get some part of your opinion. In v7 implementation: > > if probe on func+4, that's bl ftrace-entry, similar as mcount call on > other architectures, we allow this probe as normal. > > if probe on func+0, the first param ip in kprobe_ftrace_handler() points > to func+4(this is adjusted by ftrace), regs->ip points to func+8, so in > kprobe_ftrace_handler() we modify regs->ip to func+0 to call kprobe > pre handler, then modify regs->ip to func+8 to call kprobe post handler. > As can be seen, the first two instructions are considered as a virtual > mcount call. From this point of view, lr saver and the bl > is coupled. Yes, this is good. But probing on func+4 is meaningless. Both func+0 and func+4 call a handler with same pt_regs. And it should have the stack pointer which is NOT modified by lr-saver and regs->lr must point original call address. (ftrace regs caller must do this fixup for supporting live patching correctly) And in this case, func+4 has fake pt_regs because it skips lr-saver's effects. And even if you fixed up the pt_regs, there is another problem of what user expects on the target instructions. As you know, dynamic ftrace will fill the instruction with NOP (2 NOPs in arm64), in this case, maybe pt_regs are same except pc on func+0 and func+4. But if ftrace already enabled on the function, user will see there are lr-saver and bl, oops. In this case we have to change pt_regs between func+0 and func+4. So it depends on the current mode. However, IMHO, it is not worth to pay such simulation cost. No one want to probe such simulated intermediate address. It is easy to expect the result from the code. Moreover, the func+4 will not appear on debuginfo because those 2 special insturctions are just appended by the compiler, not generated by the code. So I don't think we need to support func+4. We only need func+0, or func+8 (this must be same as func+0 except regs->pc anyway) Thank you, > > If we split patch3 into two: > one to support kprobes func+4 > the second to support kprobe on func+0 > it would be much clearer. > > Then the key here is whether we could allow both kprobes on func+0 and func+4 > > Thanks -- Masami Hiramatsu From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.0 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id DAB07C2D0DA for ; Thu, 26 Dec 2019 09:26:16 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id AB9CC20882 for ; Thu, 26 Dec 2019 09:26:16 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="NTpPMNfl"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="GDEijHdH" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org AB9CC20882 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Mime-Version:References:In-Reply-To: Message-Id:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=QjxefHXD+MjnvZrlPFjKjFkae0ah4iLuNuitKYVuQjo=; b=NTpPMNfl25xCzD 1FPmLlIpcmxobk7VHcRCcz3aWI1ClxQsR1F+V3lWn6PByHbdbhk2H+dk9dIweXPvVct/fce/siVRp 105yZ4oDKqXLMaSLjjOPnC1Y+q3sxWnVoKZmKuKCoAQ7nsAHkHy1+KoTKrbnx01TNqZrWpD8lGjw8 MiBLhoLhNfokCdR4XOEG6j+lcaeq6zRadfrBbup0omLznOu+et0ITZXmnhOiKemD6F6fmdRAec8vW QOvTq1x3DeqPgJdEX1ZPKyCV2uKgaOg5tQTZIG4bL43FWXL+F9LTLivB3w7qSSS+iJcRV54L/KZZd /HIes8REGDiHTV8xrbhg==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1ikPPM-0003cZ-Ap; Thu, 26 Dec 2019 09:26:16 +0000 Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1ikPPJ-0003bu-3g for linux-arm-kernel@lists.infradead.org; Thu, 26 Dec 2019 09:26:14 +0000 Received: from devnote2 (NE2965lan1.rev.em-net.ne.jp [210.141.244.193]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id BA2AA20828; Thu, 26 Dec 2019 09:26:09 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1577352372; bh=6asAChZGLqkA6kNuizNIWoIAN2lunfQ0xFCnSMNqwvU=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=GDEijHdH+fS2OpJ8NizE3hyQLJMY7yVxc6vR8dsGzeVCgPbbShA69taRiT00O2/Q/ eYVHv071VAMVsV58S2RK4tfqDrqIX/ifiHfUexpBx69iQ9bIXU/r4CPiWQPpJNBLcZ 6myeBCBMdGencmtdph9n1UWcRWDVcpHjmXCS8EvY= Date: Thu, 26 Dec 2019 18:26:07 +0900 From: Masami Hiramatsu To: Jisheng Zhang Subject: Re: [PATCH v7 3/3] arm64: implement KPROBES_ON_FTRACE Message-Id: <20191226182607.06770598a00507090a046951@kernel.org> In-Reply-To: <20191226121108.0cd1b078@xhacker.debian> References: <20191225172625.69811b3e@xhacker.debian> <20191225173001.6c0e3fb2@xhacker.debian> <20191226115707.902545688aa90b34e2e550b3@kernel.org> <20191226110348.146bb80b@xhacker.debian> <20191226121108.0cd1b078@xhacker.debian> X-Mailer: Sylpheed 3.5.1 (GTK+ 2.24.32; x86_64-pc-linux-gnu) Mime-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20191226_012613_192545_FFFAED0C X-CRM114-Status: GOOD ( 22.50 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , "linux-doc@vger.kernel.org" , Catalin Marinas , Jonathan Corbet , "linux-kernel@vger.kernel.org" , Anil S Keshavamurthy , Ingo Molnar , Steven Rostedt , "Naveen N. Rao" , Will Deacon , "David S. Miller" , "linux-arm-kernel@lists.infradead.org" Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, 26 Dec 2019 04:25:24 +0000 Jisheng Zhang wrote: > > > > +/* > > > > + * In arm64 FTRACE_WITH_REGS implementation, we patch two nop instructions: > > > > + * the lr saver and bl ftrace-entry. Both these instructions are claimed > > > > + * by ftrace and we should allow probing on either instruction. > > > > > > No, the 2nd bl ftrace-entry must not be probed. > > > The pair of lr-saver and bl ftrace-entry is tightly coupled. You can not > > > decouple it. > > > > This is the key. different viewing of this results in different implementation. > > I'm just wondering why are the two instructions considered as coupled. I think > > here we met similar situation as powerpc: https://lkml.org/lkml/2019/6/18/646 > > the "mflr r0" equals to lr-saver here, branch to _mcount equals to bl ftrace-entry > > could you please kindly comment more? > > > > Thanks in advance > > > > hmm, I think I may get some part of your opinion. In v7 implementation: > > if probe on func+4, that's bl ftrace-entry, similar as mcount call on > other architectures, we allow this probe as normal. > > if probe on func+0, the first param ip in kprobe_ftrace_handler() points > to func+4(this is adjusted by ftrace), regs->ip points to func+8, so in > kprobe_ftrace_handler() we modify regs->ip to func+0 to call kprobe > pre handler, then modify regs->ip to func+8 to call kprobe post handler. > As can be seen, the first two instructions are considered as a virtual > mcount call. From this point of view, lr saver and the bl > is coupled. Yes, this is good. But probing on func+4 is meaningless. Both func+0 and func+4 call a handler with same pt_regs. And it should have the stack pointer which is NOT modified by lr-saver and regs->lr must point original call address. (ftrace regs caller must do this fixup for supporting live patching correctly) And in this case, func+4 has fake pt_regs because it skips lr-saver's effects. And even if you fixed up the pt_regs, there is another problem of what user expects on the target instructions. As you know, dynamic ftrace will fill the instruction with NOP (2 NOPs in arm64), in this case, maybe pt_regs are same except pc on func+0 and func+4. But if ftrace already enabled on the function, user will see there are lr-saver and bl, oops. In this case we have to change pt_regs between func+0 and func+4. So it depends on the current mode. However, IMHO, it is not worth to pay such simulation cost. No one want to probe such simulated intermediate address. It is easy to expect the result from the code. Moreover, the func+4 will not appear on debuginfo because those 2 special insturctions are just appended by the compiler, not generated by the code. So I don't think we need to support func+4. We only need func+0, or func+8 (this must be same as func+0 except regs->pc anyway) Thank you, > > If we split patch3 into two: > one to support kprobes func+4 > the second to support kprobe on func+0 > it would be much clearer. > > Then the key here is whether we could allow both kprobes on func+0 and func+4 > > Thanks -- Masami Hiramatsu _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel