From: Mark Rutland <mark.rutland@arm.com>
To: Florent Revest <revest@chromium.org>
Cc: linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
bpf@vger.kernel.org, catalin.marinas@arm.com, will@kernel.org,
rostedt@goodmis.org, mhiramat@kernel.org, ast@kernel.org,
daniel@iogearbox.net, andrii@kernel.org, kpsingh@kernel.org,
jolsa@kernel.org, xukuohai@huaweicloud.com
Subject: Re: [PATCH 5/8] ftrace: Make DIRECT_CALLS work WITH_ARGS and !WITH_REGS
Date: Thu, 2 Feb 2023 16:56:57 +0000 [thread overview]
Message-ID: <Y9vrWUM8ypNNwHyv@FVFF77S0Q05N.cambridge.arm.com> (raw)
In-Reply-To: <Y9vcua0+JzjmTICO@FVFF77S0Q05N.cambridge.arm.com>
On Thu, Feb 02, 2023 at 03:54:33PM +0000, Mark Rutland wrote:
> On Wed, Feb 01, 2023 at 05:34:17PM +0100, Florent Revest wrote:
> > -#define MULTI_FLAGS (FTRACE_OPS_FL_DIRECT | FTRACE_OPS_FL_SAVE_REGS)
> > +#define MULTI_FLAGS (FTRACE_OPS_FL_DIRECT)
>
> Unfortunately, I think this is broken for architectures where:
>
> * DYNAMIC_FTRACE_WITH_DIRECT_CALLS=y
> * DYNAMIC_FTRACE_WITH_REGS=y
> * DYNAMIC_FTRACE_WITH_ARGS=n
>
> ... since those might pass a NULL ftrace_regs around, and so when using the
> list ops arch_ftrace_set_direct_caller() might blow up accessing an element of
> ftrace_regs.
>
> It looks like 32-bit x86 is the only case with that combination, and its
> ftrace_caller implementation passes a NULL regs, so I reckon that'll blow up.
> However, it looks like there aren't any FTRACE_DIRECT samples wired up for
> 32-bit x86, so I'm not aware of a test case we can use.
FWIW, the FTRACE_STARTUP_TEST tickles this:
[ 1.896209] Testing tracer function_graph:
[ 2.900282] BUG: kernel NULL pointer dereference, address: 0000002c
[ 2.901171] #PF: supervisor write access in kernel mode
[ 2.901171] #PF: error_code(0x0002) - not-present page
[ 2.901171] *pde = 00000000
[ 2.901171] Oops: 0002 [#1] PREEMPT SMP
[ 2.901171] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.2.0-rc3-00014-gcfd6340c71ce #1
[ 2.901171] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-2 04/01/2014
[ 2.901171] EIP: call_direct_funcs+0xd/0x1c
[ 2.901171] Code: 00 00 00 00 90 a9 00 00 00 01 0f 84 d7 fe ff ff 0d 00 00 80 00 89 46 04 e9 d2 fe ff ff 8b 41 64 85 c0 74 11 55 89 e5 8b 55 08 <89> 42 2c 5d c3 8d b6 00 00 00 00 c3 8d 76 00 89 c1 89 b
[ 2.901171] EAX: cc3620e8 EBX: c1147e44 ECX: c1147e44 EDX: 00000000
[ 2.901171] ESI: fffffeff EDI: cc354208 EBP: c1147dbc ESP: c1147dbc
[ 2.901171] DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068 EFLAGS: 00010286
[ 2.901171] CR0: 80050033 CR2: 0000002c CR3: 0d703000 CR4: 00350ed0
[ 2.901171] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
[ 2.901171] DR6: fffe0ff0 DR7: 00000400
[ 2.901171] Call Trace:
[ 2.901171] arch_ftrace_ops_list_func+0xf5/0x1bc
[ 2.901171] ? ftrace_enable_ftrace_graph_caller+0x3b/0x44
[ 2.901171] ? trace_selftest_startup_function_graph+0x1d9/0x298
[ 2.901171] ? syscall_unregfunc+0xa0/0xa0
[ 2.901171] ftrace_call+0x5/0x13
[ 2.901171] trace_selftest_dynamic_test_func+0x5/0xc
[ 2.901171] trace_selftest_startup_function_graph+0x1d9/0x298
[ 2.901171] ? trace_selftest_dynamic_test_func+0x5/0xc
[ 2.901171] ? trace_selftest_startup_function_graph+0x1d9/0x298
[ 2.901171] ? ftrace_check_record+0x340/0x340
[ 2.901171] ? ftrace_check_record+0x340/0x340
[ 2.901171] ? ftrace_stub_graph+0x4/0x4
[ 2.901171] ? trace_selftest_test_regs_func+0x18/0x18
[ 2.901171] run_tracer_selftest+0x7d/0x1bc
[ 2.901171] ? graph_depth_read+0x90/0x90
[ 2.901171] register_tracer+0xd3/0x284
[ 2.901171] ? register_trace_event+0xf6/0x180
[ 2.901171] ? init_graph_tracefs+0x38/0x38
[ 2.901171] init_graph_trace+0x56/0x78
[ 2.901171] do_one_initcall+0x53/0x204
[ 2.901171] ? parse_args+0x143/0x3ec
[ 2.901171] ? __kmem_cache_alloc_node+0x2d/0x224
[ 2.901171] kernel_init_freeable+0x198/0x2bc
[ 2.901171] ? rdinit_setup+0x30/0x30
[ 2.901171] ? rest_init+0xb0/0xb0
[ 2.901171] kernel_init+0x1a/0x1d0
[ 2.901171] ? schedule_tail_wrapper+0x9/0xc
[ 2.901171] ret_from_fork+0x1c/0x28
[ 2.901171] Modules linked in:
[ 2.901171] CR2: 000000000000002c
[ 2.901171] ---[ end trace 0000000000000000 ]---
[ 2.901171] EIP: call_direct_funcs+0xd/0x1c
[ 2.901171] Code: 00 00 00 00 90 a9 00 00 00 01 0f 84 d7 fe ff ff 0d 00 00 80 00 89 46 04 e9 d2 fe ff ff 8b 41 64 85 c0 74 11 55 89 e5 8b 55 08 <89> 42 2c 5d c3 8d b6 00 00 00 00 c3 8d 76 00 89 c1 89 b
[ 2.901171] EAX: cc3620e8 EBX: c1147e44 ECX: c1147e44 EDX: 00000000
[ 2.901171] ESI: fffffeff EDI: cc354208 EBP: c1147dbc ESP: c1147dbc
[ 2.901171] DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068 EFLAGS: 00010286
[ 2.901171] CR0: 80050033 CR2: 0000002c CR3: 0d703000 CR4: 00350ed0
[ 2.901171] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
[ 2.901171] DR6: fffe0ff0 DR7: 00000400
[ 2.901171] note: swapper/0[1] exited with preempt_count 1
[ 2.901175] Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000009
[ 2.902171] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000009 ]---
The below diff solved that for me.
Thanks,
Mark.
---->8----
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 84f717f8959e..3d2156e335d7 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -241,6 +241,12 @@ enum {
FTRACE_OPS_FL_DIRECT = BIT(17),
};
+#ifndef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
+#define FTRACE_OPS_FL_SAVE_ARGS FTRACE_OPS_FL_SAVE_REGS
+#else
+#define FTRACE_OPS_FL_SAVE_ARGS 0
+#endif
+
/*
* FTRACE_OPS_CMD_* commands allow the ftrace core logic to request changes
* to a ftrace_ops. Note, the requests may fail.
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 73b6f6489ba1..8e739303b6a2 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -5282,7 +5282,7 @@ static LIST_HEAD(ftrace_direct_funcs);
static int register_ftrace_function_nolock(struct ftrace_ops *ops);
-#define MULTI_FLAGS (FTRACE_OPS_FL_DIRECT)
+#define MULTI_FLAGS (FTRACE_OPS_FL_DIRECT | FTRACE_OPS_FL_SAVE_ARGS)
static int check_direct_multi(struct ftrace_ops *ops)
{
next prev parent reply other threads:[~2023-02-02 16:57 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-01 16:34 [PATCH 0/8] Add ftrace direct call for arm64 Florent Revest
2023-02-01 16:34 ` [PATCH 1/8] ftrace: Replace uses of _ftrace_direct APIs with _ftrace_direct_multi Florent Revest
2023-02-02 15:01 ` Mark Rutland
2023-02-02 17:37 ` Florent Revest
2023-02-07 15:21 ` Florent Revest
2023-02-07 15:35 ` Steven Rostedt
2023-02-07 16:19 ` Florent Revest
2023-02-01 16:34 ` [PATCH 2/8] ftrace: Remove the legacy _ftrace_direct API Florent Revest
2023-02-02 15:11 ` Mark Rutland
2023-02-01 16:34 ` [PATCH 3/8] ftrace: Rename _ftrace_direct_multi APIs to _ftrace_direct APIs Florent Revest
2023-02-02 15:17 ` Mark Rutland
2023-02-01 16:34 ` [PATCH 4/8] ftrace: Store direct called addresses in their ops Florent Revest
2023-02-02 15:29 ` Mark Rutland
2023-02-02 17:41 ` Florent Revest
2023-02-01 16:34 ` [PATCH 5/8] ftrace: Make DIRECT_CALLS work WITH_ARGS and !WITH_REGS Florent Revest
2023-02-02 15:54 ` Mark Rutland
2023-02-02 16:56 ` Mark Rutland [this message]
2023-02-02 18:19 ` Florent Revest
2023-02-03 10:03 ` Mark Rutland
2023-02-03 11:01 ` Florent Revest
2023-02-02 18:18 ` Florent Revest
2023-02-01 16:34 ` [PATCH 6/8] ftrace: Fix dead loop caused by direct call in ftrace selftest Florent Revest
2023-02-02 19:03 ` Mark Rutland
2023-02-03 12:35 ` Florent Revest
2023-02-03 15:37 ` Mark Rutland
2023-02-06 16:25 ` Florent Revest
2023-02-01 16:34 ` [PATCH 7/8] arm64: ftrace: Add direct call support Florent Revest
2023-02-03 15:34 ` Mark Rutland
2023-02-06 16:25 ` Florent Revest
2023-02-01 16:34 ` [PATCH 8/8] arm64: ftrace: Add direct called trampoline samples support Florent Revest
2023-02-02 8:36 ` [PATCH 0/8] Add ftrace direct call for arm64 Xu Kuohai
2023-02-02 10:50 ` Daniel Borkmann
2023-02-02 17:32 ` Florent Revest
2023-02-02 20:06 ` Steven Rostedt
2023-02-03 9:49 ` Mark Rutland
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y9vrWUM8ypNNwHyv@FVFF77S0Q05N.cambridge.arm.com \
--to=mark.rutland@arm.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=catalin.marinas@arm.com \
--cc=daniel@iogearbox.net \
--cc=jolsa@kernel.org \
--cc=kpsingh@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-trace-kernel@vger.kernel.org \
--cc=mhiramat@kernel.org \
--cc=revest@chromium.org \
--cc=rostedt@goodmis.org \
--cc=will@kernel.org \
--cc=xukuohai@huaweicloud.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®