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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id AA86BC433F5 for ; Wed, 20 Apr 2022 23:24:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1377311AbiDTX1D (ORCPT ); Wed, 20 Apr 2022 19:27:03 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60930 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1344735AbiDTX07 (ORCPT ); Wed, 20 Apr 2022 19:26:59 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A142C1D315; Wed, 20 Apr 2022 16:24:11 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 38DA861B07; Wed, 20 Apr 2022 23:24:11 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id AE565C385A1; Wed, 20 Apr 2022 23:24:06 +0000 (UTC) Date: Wed, 20 Apr 2022 19:24:05 -0400 From: Steven Rostedt To: Xu Kuohai Cc: , , , , , Catalin Marinas , Will Deacon , 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 , Mark Rutland , Ard Biesheuvel , Pasha Tatashin , Peter Collingbourne , Daniel Kiss , Sudeep Holla , Steven Price , Marc Zyngier , Mark Brown , Kumar Kartikeya Dwivedi , Delyan Kratunov Subject: Re: [PATCH bpf-next v2 2/6] ftrace: Fix deadloop caused by direct call in ftrace selftest Message-ID: <20220420192405.4e43a966@gandalf.local.home> In-Reply-To: <20220414162220.1985095-3-xukuohai@huawei.com> References: <20220414162220.1985095-1-xukuohai@huawei.com> <20220414162220.1985095-3-xukuohai@huawei.com> X-Mailer: Claws Mail 3.17.8 (GTK+ 2.24.33; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Thu, 14 Apr 2022 12:22:16 -0400 Xu Kuohai wrote: > After direct call is enabled for arm64, ftrace selftest enters a > dead loop: > > : > 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 make trace_direct_tramp() a weak symbol and rewrite it for > arm64. > > To detect this issue directly, call DYN_FTRACE_TEST_NAME() before > register_ftrace_graph(). > > Reported-by: Li Huafei > Signed-off-by: Xu Kuohai > --- > arch/arm64/kernel/entry-ftrace.S | 10 ++++++++++ > kernel/trace/trace_selftest.c | 4 +++- > 2 files changed, 13 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/kernel/entry-ftrace.S b/arch/arm64/kernel/entry-ftrace.S > index dfe62c55e3a2..e58eb06ec9b2 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..38b0d5c9a1e0 100644 > --- a/kernel/trace/trace_selftest.c > +++ b/kernel/trace/trace_selftest.c > @@ -785,7 +785,7 @@ static struct fgraph_ops fgraph_ops __initdata = { > }; > > #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS > -noinline __noclone static void trace_direct_tramp(void) { } > +void __weak trace_direct_tramp(void) { } > #endif > > /* > @@ -868,6 +868,8 @@ trace_selftest_startup_function_graph(struct tracer *trace, > if (ret) > goto out; > > + DYN_FTRACE_TEST_NAME(); This doesn't look like it belongs in this patch. -- Steve > + > ret = register_ftrace_graph(&fgraph_ops); > if (ret) { > warn_failed_init_tracer(trace, ret);