linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org
Cc: Ingo Molnar <mingo@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	stable@vger.kernel.org, Song Liu <song@kernel.org>
Subject: [for-next][PATCH 19/23] ftrace: Clean up hash direct_functions on register failures
Date: Fri, 27 May 2022 22:50:47 -0400	[thread overview]
Message-ID: <20220528025252.116449583@goodmis.org> (raw)
In-Reply-To: 20220528025028.850906216@goodmis.org

From: Song Liu <song@kernel.org>

We see the following GPF when register_ftrace_direct fails:

[ ] general protection fault, probably for non-canonical address \
  0x200000000000010: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC PTI
[...]
[ ] RIP: 0010:ftrace_find_rec_direct+0x53/0x70
[ ] Code: 48 c1 e0 03 48 03 42 08 48 8b 10 31 c0 48 85 d2 74 [...]
[ ] RSP: 0018:ffffc9000138bc10 EFLAGS: 00010206
[ ] RAX: 0000000000000000 RBX: ffffffff813e0df0 RCX: 000000000000003b
[ ] RDX: 0200000000000000 RSI: 000000000000000c RDI: ffffffff813e0df0
[ ] RBP: ffffffffa00a3000 R08: ffffffff81180ce0 R09: 0000000000000001
[ ] R10: ffffc9000138bc18 R11: 0000000000000001 R12: ffffffff813e0df0
[ ] R13: ffffffff813e0df0 R14: ffff888171b56400 R15: 0000000000000000
[ ] FS:  00007fa9420c7780(0000) GS:ffff888ff6a00000(0000) knlGS:000000000
[ ] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ ] CR2: 000000000770d000 CR3: 0000000107d50003 CR4: 0000000000370ee0
[ ] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ ] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ ] Call Trace:
[ ]  <TASK>
[ ]  register_ftrace_direct+0x54/0x290
[ ]  ? render_sigset_t+0xa0/0xa0
[ ]  bpf_trampoline_update+0x3f5/0x4a0
[ ]  ? 0xffffffffa00a3000
[ ]  bpf_trampoline_link_prog+0xa9/0x140
[ ]  bpf_tracing_prog_attach+0x1dc/0x450
[ ]  bpf_raw_tracepoint_open+0x9a/0x1e0
[ ]  ? find_held_lock+0x2d/0x90
[ ]  ? lock_release+0x150/0x430
[ ]  __sys_bpf+0xbd6/0x2700
[ ]  ? lock_is_held_type+0xd8/0x130
[ ]  __x64_sys_bpf+0x1c/0x20
[ ]  do_syscall_64+0x3a/0x80
[ ]  entry_SYSCALL_64_after_hwframe+0x44/0xae
[ ] RIP: 0033:0x7fa9421defa9
[ ] Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 9 f8 [...]
[ ] RSP: 002b:00007ffed743bd78 EFLAGS: 00000246 ORIG_RAX: 0000000000000141
[ ] RAX: ffffffffffffffda RBX: 00000000069d2480 RCX: 00007fa9421defa9
[ ] RDX: 0000000000000078 RSI: 00007ffed743bd80 RDI: 0000000000000011
[ ] RBP: 00007ffed743be00 R08: 0000000000bb7270 R09: 0000000000000000
[ ] R10: 00000000069da210 R11: 0000000000000246 R12: 0000000000000001
[ ] R13: 00007ffed743c4b0 R14: 00000000069d2480 R15: 0000000000000001
[ ]  </TASK>
[ ] Modules linked in: klp_vm(OK)
[ ] ---[ end trace 0000000000000000 ]---

One way to trigger this is:
  1. load a livepatch that patches kernel function xxx;
  2. run bpftrace -e 'kfunc:xxx {}', this will fail (expected for now);
  3. repeat #2 => gpf.

This is because the entry is added to direct_functions, but not removed.
Fix this by remove the entry from direct_functions when
register_ftrace_direct fails.

Also remove the last trailing space from ftrace.c, so we don't have to
worry about it anymore.

Link: https://lkml.kernel.org/r/20220524170839.900849-1-song@kernel.org

Cc: stable@vger.kernel.org
Fixes: 763e34e74bb7 ("ftrace: Add register_ftrace_direct()")
Signed-off-by: Song Liu <song@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 kernel/trace/ftrace.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index fb8f08b4bd41..d653ef4febc5 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -4454,7 +4454,7 @@ int ftrace_func_mapper_add_ip(struct ftrace_func_mapper *mapper,
  * @ip: The instruction pointer address to remove the data from
  *
  * Returns the data if it is found, otherwise NULL.
- * Note, if the data pointer is used as the data itself, (see 
+ * Note, if the data pointer is used as the data itself, (see
  * ftrace_func_mapper_find_ip(), then the return value may be meaningless,
  * if the data pointer was set to zero.
  */
@@ -5188,8 +5188,6 @@ int register_ftrace_direct(unsigned long ip, unsigned long addr)
 		goto out_unlock;
 
 	ret = ftrace_set_filter_ip(&direct_ops, ip, 0, 0);
-	if (ret)
-		remove_hash_entry(direct_functions, entry);
 
 	if (!ret && !(direct_ops.flags & FTRACE_OPS_FL_ENABLED)) {
 		ret = register_ftrace_function(&direct_ops);
@@ -5198,6 +5196,7 @@ int register_ftrace_direct(unsigned long ip, unsigned long addr)
 	}
 
 	if (ret) {
+		remove_hash_entry(direct_functions, entry);
 		kfree(entry);
 		if (!direct->count) {
 			list_del_rcu(&direct->next);
-- 
2.35.1

  parent reply	other threads:[~2022-05-28  2:54 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-28  2:50 [for-next][PATCH 00/23] tracing: Last minute fixes and updates for 5.19 Steven Rostedt
2022-05-28  2:50 ` [for-next][PATCH 01/23] tracing: Have event format check not flag %p* on __get_dynamic_array() Steven Rostedt
2022-05-28  2:50 ` [for-next][PATCH 02/23] tracing: Reset the function filter after completing trampoline/graph selftest Steven Rostedt
2022-05-28  2:50 ` [for-next][PATCH 03/23] tracing: Use trace_create_file() to simplify creation of tracefs entries Steven Rostedt
2022-05-28  2:50 ` [for-next][PATCH 04/23] ftrace: Deal with error return code of the ftrace_process_locs() function Steven Rostedt
2022-05-28  2:50 ` [for-next][PATCH 05/23] tracing: Use strim() to remove whitespace instead of doing it manually Steven Rostedt
2022-05-28  2:50 ` [for-next][PATCH 06/23] tracing: Fix potential double free in create_var_ref() Steven Rostedt
2022-05-28  2:50 ` [for-next][PATCH 07/23] tracing: Fix return value of trace_pid_write() Steven Rostedt
2022-05-28  2:50 ` [for-next][PATCH 09/23] tracing/timerlat: Notify IRQ new max latency only if stop tracing is set Steven Rostedt
2022-05-28  2:50 ` [for-next][PATCH 10/23] tracing/timerlat: Print stacktrace in the IRQ handler if needed Steven Rostedt
2022-05-28  2:50 ` [for-next][PATCH 11/23] tracing/timerlat: Do not wakeup the thread if the trace stops at the IRQ Steven Rostedt
2022-05-28  2:50 ` [for-next][PATCH 12/23] tracing: Change "char *" string form to "char []" Steven Rostedt
2022-05-28  2:50 ` [for-next][PATCH 13/23] tracing: Cleanup code by removing init "char *name" Steven Rostedt
2022-05-28  2:50 ` [for-next][PATCH 14/23] ftrace: Remove return value of ftrace_arch_modify_*() Steven Rostedt
2022-05-28  2:50 ` [for-next][PATCH 15/23] ftrace: Fix typo in comment Steven Rostedt
2022-05-28  2:50 ` [for-next][PATCH 16/23] tracing: Initialize integer variable to prevent garbage return value Steven Rostedt
2022-05-28  2:50 ` [for-next][PATCH 17/23] tracing: Disable kcov on trace_preemptirq.c Steven Rostedt
2022-05-28  2:50 ` [for-next][PATCH 18/23] tracing: Fix comments of create_filter() Steven Rostedt
2022-05-28  2:50 ` Steven Rostedt [this message]
2022-05-28  2:50 ` [for-next][PATCH 20/23] x86,tracing: Remove unused headers Steven Rostedt
2022-05-28  2:50 ` [for-next][PATCH 21/23] x86/traceponit: Fix comment about irq vector tracepoints Steven Rostedt
2022-05-28  2:50 ` [for-next][PATCH 22/23] tracing: Fix comments for event_trigger_separate_filter() Steven Rostedt
2022-05-28  2:50 ` [for-next][PATCH 23/23] ftrace: Add FTRACE_MCOUNT_MAX_OFFSET to avoid adding weak function Steven Rostedt
2022-05-28  3:17 ` [for-next][PATCH 00/23] tracing: Last minute fixes and updates for 5.19 Steven Rostedt

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=20220528025252.116449583@goodmis.org \
    --to=rostedt@goodmis.org \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=song@kernel.org \
    --cc=stable@vger.kernel.org \
    /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;
as well as URLs for NNTP newsgroup(s).