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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 72572C433EF for ; Thu, 26 May 2022 09:46:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:From:References:CC:To: Subject:MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=91HzmuarTXPBkk5Jbf7dcT1XNxHQL4VZOjiiAkLJtd8=; b=Afr5wcGQk35RxS AQZfFwzJgYjtm3A1sTBnNm+CwSIVRZf1w0bu3lAUUqi4LD6tW6jkdgo/5K5XuLm/wYiqQv7MWiAfk 6csBND0UZ2Q+YTHd2wV53nreljo0it8qYsJvpmwRuSn4ubEqf/xzYANX00XNDcsO8/XOIduJ6Mc7w T//DHBEKaCESsqMbyH/qnkm7c6e5KG/09tqHjNT1EUfrqhH/oaQ1wBWCPvpFqu5f8qbFxgy2fRdRL uiK2pNcUt4jjQQ7eLvbxuVIP6hTx012Clge+4iUrfO8jnDmNfy269JUq8tm3WnIGG0A0DAWYGqabk zox8rOT6suWgzZefJM3w==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nuA3j-00EFUR-Ga; Thu, 26 May 2022 09:45:35 +0000 Received: from szxga01-in.huawei.com ([45.249.212.187]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nuA3T-00EFM5-4M for linux-arm-kernel@lists.infradead.org; Thu, 26 May 2022 09:45:23 +0000 Received: from kwepemi500013.china.huawei.com (unknown [172.30.72.54]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4L830X0wP5zgYPK; Thu, 26 May 2022 17:43:40 +0800 (CST) Received: from [10.67.111.192] (10.67.111.192) by kwepemi500013.china.huawei.com (7.221.188.120) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.24; Thu, 26 May 2022 17:45:10 +0800 Message-ID: Date: Thu, 26 May 2022 17:45:10 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.9.0 Subject: Re: [PATCH bpf-next v5 2/6] ftrace: Fix deadloop caused by direct call in ftrace selftest Content-Language: en-US To: Mark Rutland CC: , , , , , Catalin Marinas , Will Deacon , Steven Rostedt , Ingo Molnar , Daniel Borkmann , Alexei Starovoitov , Zi Shen Lim , Andrii Nakryiko , Martin KaFai Lau , Song Liu , Yonghong Song , John Fastabend , KP Singh , "David S . Miller" , Hideaki YOSHIFUJI , David Ahern , Thomas Gleixner , Borislav Petkov , Dave Hansen , , , Shuah Khan , Jakub Kicinski , Jesper Dangaard Brouer , Pasha Tatashin , Ard Biesheuvel , Daniel Kiss , Steven Price , Sudeep Holla , Marc Zyngier , Peter Collingbourne , Mark Brown , Delyan Kratunov , Kumar Kartikeya Dwivedi References: <20220518131638.3401509-1-xukuohai@huawei.com> <20220518131638.3401509-3-xukuohai@huawei.com> From: Xu Kuohai In-Reply-To: X-Originating-IP: [10.67.111.192] X-ClientProxiedBy: dggems704-chm.china.huawei.com (10.3.19.181) To kwepemi500013.china.huawei.com (7.221.188.120) X-CFilter-Loop: Reflected X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220526_024519_628759_CEF4C8B7 X-CRM114-Status: GOOD ( 21.79 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 5/25/2022 9:43 PM, Mark Rutland wrote: > On Wed, May 18, 2022 at 09:16:34AM -0400, Xu Kuohai wrote: >> After direct call is enabled for arm64, ftrace selftest enters a >> dead loop: > > IIUC this means that patch 1 alone is broken, and presumably this patch should > have been part of it? No, patch 1 is not broken. This patch fixes bug in the selftest trampoline, not bug in patch 1. > >> : >> 00 bti c >> 01 mov x9, x30 : >> 02 bl ----------> ret >> | >> lr/x30 is 03, return to 03 >> | >> 03 mov w0, #0x0 <-----------------------------| >> | | >> | dead loop! | >> | | >> 04 ret ---- lr/x30 is still 03, go back to 03 ----| >> >> The reason is that when the direct caller trace_direct_tramp() returns >> to the patched function trace_selftest_dynamic_test_func(), lr is still >> the address after the instrumented instruction in the patched function, >> so when the patched function exits, it returns to itself! >> >> To fix this issue, we need to restore lr before trace_direct_tramp() >> exits, so rewrite a dedicated trace_direct_tramp() for arm64. > > As mentioned on patch 1 I'd prefer we solved this through indirection, which > would avoid the need for this and would make things more robust generally by > keeping the unusual calling convention private to the patch-site and regular > trampoline. > IIUC, we still need to restore x30 before returning from the trampoline even through indirection, so this bug is still there. > Thanks, > Mark. > >> Reported-by: Li Huafei >> Signed-off-by: Xu Kuohai >> --- >> arch/arm64/include/asm/ftrace.h | 10 ++++++++++ >> arch/arm64/kernel/entry-ftrace.S | 10 ++++++++++ >> kernel/trace/trace_selftest.c | 2 ++ >> 3 files changed, 22 insertions(+) >> >> diff --git a/arch/arm64/include/asm/ftrace.h b/arch/arm64/include/asm/ftrace.h >> index 14a35a5df0a1..6f6b184e72fb 100644 >> --- a/arch/arm64/include/asm/ftrace.h >> +++ b/arch/arm64/include/asm/ftrace.h >> @@ -126,6 +126,16 @@ static inline bool arch_syscall_match_sym_name(const char *sym, >> */ >> return !strcmp(sym + 8, name); >> } >> + >> +#ifdef CONFIG_FTRACE_SELFTEST >> +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS >> + >> +#define trace_direct_tramp trace_direct_tramp >> +extern void trace_direct_tramp(void); >> + >> +#endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */ >> +#endif /* CONFIG_FTRACE_SELFTEST */ >> + >> #endif /* ifndef __ASSEMBLY__ */ >> >> #endif /* __ASM_FTRACE_H */ >> diff --git a/arch/arm64/kernel/entry-ftrace.S b/arch/arm64/kernel/entry-ftrace.S >> index dfe62c55e3a2..a47e87d4d3dd 100644 >> --- a/arch/arm64/kernel/entry-ftrace.S >> +++ b/arch/arm64/kernel/entry-ftrace.S >> @@ -357,3 +357,13 @@ SYM_CODE_START(return_to_handler) >> ret >> SYM_CODE_END(return_to_handler) >> #endif /* CONFIG_FUNCTION_GRAPH_TRACER */ >> + >> +#ifdef CONFIG_FTRACE_SELFTEST >> +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS >> +SYM_FUNC_START(trace_direct_tramp) >> + mov x10, x30 >> + mov x30, x9 >> + ret x10 >> +SYM_FUNC_END(trace_direct_tramp) >> +#endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */ >> +#endif /* CONFIG_FTRACE_SELFTEST */ >> diff --git a/kernel/trace/trace_selftest.c b/kernel/trace/trace_selftest.c >> index abcadbe933bb..e7ccd0d10c39 100644 >> --- a/kernel/trace/trace_selftest.c >> +++ b/kernel/trace/trace_selftest.c >> @@ -785,8 +785,10 @@ static struct fgraph_ops fgraph_ops __initdata = { >> }; >> >> #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS >> +#ifndef trace_direct_tramp >> noinline __noclone static void trace_direct_tramp(void) { } >> #endif >> +#endif >> >> /* >> * Pretty much the same than for the function tracer from which the selftest >> -- >> 2.30.2 >> > . _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel