All of lore.kernel.org
 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>,
	Yang Jihong <yangjihong1@huawei.com>
Subject: [for-linus][PATCH 04/10] ftrace: Fix NULL pointer dereference in is_ftrace_trampoline when ftrace is dead
Date: Sat, 20 Aug 2022 20:07:41 -0400	[thread overview]
Message-ID: <20220821000845.047156494@goodmis.org> (raw)
In-Reply-To: 20220821000737.328590235@goodmis.org

From: Yang Jihong <yangjihong1@huawei.com>

ftrace_startup does not remove ops from ftrace_ops_list when
ftrace_startup_enable fails:

register_ftrace_function
  ftrace_startup
    __register_ftrace_function
      ...
      add_ftrace_ops(&ftrace_ops_list, ops)
      ...
    ...
    ftrace_startup_enable // if ftrace failed to modify, ftrace_disabled is set to 1
    ...
  return 0 // ops is in the ftrace_ops_list.

When ftrace_disabled = 1, unregister_ftrace_function simply returns without doing anything:
unregister_ftrace_function
  ftrace_shutdown
    if (unlikely(ftrace_disabled))
            return -ENODEV;  // return here, __unregister_ftrace_function is not executed,
                             // as a result, ops is still in the ftrace_ops_list
    __unregister_ftrace_function
    ...

If ops is dynamically allocated, it will be free later, in this case,
is_ftrace_trampoline accesses NULL pointer:

is_ftrace_trampoline
  ftrace_ops_trampoline
    do_for_each_ftrace_op(op, ftrace_ops_list) // OOPS! op may be NULL!

Syzkaller reports as follows:
[ 1203.506103] BUG: kernel NULL pointer dereference, address: 000000000000010b
[ 1203.508039] #PF: supervisor read access in kernel mode
[ 1203.508798] #PF: error_code(0x0000) - not-present page
[ 1203.509558] PGD 800000011660b067 P4D 800000011660b067 PUD 130fb8067 PMD 0
[ 1203.510560] Oops: 0000 [#1] SMP KASAN PTI
[ 1203.511189] CPU: 6 PID: 29532 Comm: syz-executor.2 Tainted: G    B   W         5.10.0 #8
[ 1203.512324] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
[ 1203.513895] RIP: 0010:is_ftrace_trampoline+0x26/0xb0
[ 1203.514644] Code: ff eb d3 90 41 55 41 54 49 89 fc 55 53 e8 f2 00 fd ff 48 8b 1d 3b 35 5d 03 e8 e6 00 fd ff 48 8d bb 90 00 00 00 e8 2a 81 26 00 <48> 8b ab 90 00 00 00 48 85 ed 74 1d e8 c9 00 fd ff 48 8d bb 98 00
[ 1203.518838] RSP: 0018:ffffc900012cf960 EFLAGS: 00010246
[ 1203.520092] RAX: 0000000000000000 RBX: 000000000000007b RCX: ffffffff8a331866
[ 1203.521469] RDX: 0000000000000000 RSI: 0000000000000008 RDI: 000000000000010b
[ 1203.522583] RBP: 0000000000000000 R08: 0000000000000000 R09: ffffffff8df18b07
[ 1203.523550] R10: fffffbfff1be3160 R11: 0000000000000001 R12: 0000000000478399
[ 1203.524596] R13: 0000000000000000 R14: ffff888145088000 R15: 0000000000000008
[ 1203.525634] FS:  00007f429f5f4700(0000) GS:ffff8881daf00000(0000) knlGS:0000000000000000
[ 1203.526801] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1203.527626] CR2: 000000000000010b CR3: 0000000170e1e001 CR4: 00000000003706e0
[ 1203.528611] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 1203.529605] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400

Therefore, when ftrace_startup_enable fails, we need to rollback registration
process and remove ops from ftrace_ops_list.

Link: https://lkml.kernel.org/r/20220818032659.56209-1-yangjihong1@huawei.com

Suggested-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 kernel/trace/ftrace.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 601ccf1b2f09..4baa99363b16 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -2937,6 +2937,16 @@ int ftrace_startup(struct ftrace_ops *ops, int command)
 
 	ftrace_startup_enable(command);
 
+	/*
+	 * If ftrace is in an undefined state, we just remove ops from list
+	 * to prevent the NULL pointer, instead of totally rolling it back and
+	 * free trampoline, because those actions could cause further damage.
+	 */
+	if (unlikely(ftrace_disabled)) {
+		__unregister_ftrace_function(ops);
+		return -ENODEV;
+	}
+
 	ops->flags &= ~FTRACE_OPS_FL_ADDING;
 
 	return 0;
-- 
2.35.1

  parent reply	other threads:[~2022-08-21  0:09 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-21  0:07 [for-linus][PATCH 00/10] tracing: Fixes for 6.0 Steven Rostedt
2022-08-21  0:07 ` [for-linus][PATCH 01/10] tracing: Suppress sparse warnings triggered by is_signed_type() Steven Rostedt
2022-08-21 18:35   ` Linus Torvalds
2022-08-21 19:55     ` Steven Rostedt
2022-08-22 18:20     ` Bart Van Assche
2022-08-22 18:45       ` Linus Torvalds
2022-08-22 20:19         ` Bart Van Assche
2022-08-23  7:06         ` Rasmus Villemoes
2022-08-23 22:05           ` Bart Van Assche
2022-08-23 23:18             ` Linus Torvalds
2022-08-23 23:57               ` Linus Torvalds
2022-08-24  2:09                 ` Al Viro
2022-08-24  2:16                   ` Linus Torvalds
2022-08-24  3:10                   ` Al Viro
2022-08-24  3:20                     ` Al Viro
2022-08-24  5:56                     ` Linus Torvalds
2022-08-24  0:09               ` Bart Van Assche
2022-08-24  1:49                 ` Linus Torvalds
2022-08-24  2:11                   ` Linus Torvalds
2022-08-24  3:47                     ` Bart Van Assche
2022-08-24  3:46                   ` Bart Van Assche
2022-08-24 23:28                   ` Bart Van Assche
2022-08-25  0:30                     ` Linus Torvalds
2022-08-25  0:40                       ` Linus Torvalds
2022-08-25  7:57                         ` Rasmus Villemoes
2022-08-25  8:07                           ` Linus Torvalds
2022-08-25 17:39                         ` Bart Van Assche
2022-08-25 18:17                           ` Kees Cook
2022-08-21  0:07 ` [for-linus][PATCH 02/10] tracing: React to error return from traceprobe_parse_event_name() Steven Rostedt
2022-08-21  0:07 ` [for-linus][PATCH 03/10] tracing/perf: Fix double put of trace event when init fails Steven Rostedt
2022-08-21  0:07 ` Steven Rostedt [this message]
2022-08-21  0:07 ` [for-linus][PATCH 05/10] tracing/eprobes: Do not allow eprobes to use $stack, or % for regs Steven Rostedt
2022-08-21  0:07 ` [for-linus][PATCH 06/10] tracing/eprobes: Do not hardcode $comm as a string Steven Rostedt
2022-08-21  0:07 ` [for-linus][PATCH 07/10] tracing/eprobes: Fix reading of string fields Steven Rostedt
2022-08-21  0:07 ` [for-linus][PATCH 08/10] tracing/eprobes: Have event probes be consistent with kprobes and uprobes Steven Rostedt
2022-08-21  0:07 ` [for-linus][PATCH 09/10] tracing/probes: Have kprobes and uprobes use $COMM too Steven Rostedt
2022-08-21  0:07 ` [for-linus][PATCH 10/10] tracing: Have filter accept "common_cpu" to be consistent 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=20220821000845.047156494@goodmis.org \
    --to=rostedt@goodmis.org \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=yangjihong1@huawei.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.