]> git.dujemihanovic.xyz Git - linux.git/commitdiff
kprobe/ftrace: bail out if ftrace was killed
authorStephen Brennan <stephen.s.brennan@oracle.com>
Wed, 1 May 2024 16:29:56 +0000 (09:29 -0700)
committerMasami Hiramatsu (Google) <mhiramat@kernel.org>
Wed, 15 May 2024 22:23:30 +0000 (07:23 +0900)
If an error happens in ftrace, ftrace_kill() will prevent disarming
kprobes. Eventually, the ftrace_ops associated with the kprobes will be
freed, yet the kprobes will still be active, and when triggered, they
will use the freed memory, likely resulting in a page fault and panic.

This behavior can be reproduced quite easily, by creating a kprobe and
then triggering a ftrace_kill(). For simplicity, we can simulate an
ftrace error with a kernel module like [1]:

[1]: https://github.com/brenns10/kernel_stuff/tree/master/ftrace_killer

  sudo perf probe --add commit_creds
  sudo perf trace -e probe:commit_creds
  # In another terminal
  make
  sudo insmod ftrace_killer.ko  # calls ftrace_kill(), simulating bug
  # Back to perf terminal
  # ctrl-c
  sudo perf probe --del commit_creds

After a short period, a page fault and panic would occur as the kprobe
continues to execute and uses the freed ftrace_ops. While ftrace_kill()
is supposed to be used only in extreme circumstances, it is invoked in
FTRACE_WARN_ON() and so there are many places where an unexpected bug
could be triggered, yet the system may continue operating, possibly
without the administrator noticing. If ftrace_kill() does not panic the
system, then we should do everything we can to continue operating,
rather than leave a ticking time bomb.

Link: https://lore.kernel.org/all/20240501162956.229427-1-stephen.s.brennan@oracle.com/
Signed-off-by: Stephen Brennan <stephen.s.brennan@oracle.com>
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Acked-by: Guo Ren <guoren@kernel.org>
Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
arch/csky/kernel/probes/ftrace.c
arch/loongarch/kernel/ftrace_dyn.c
arch/parisc/kernel/ftrace.c
arch/powerpc/kernel/kprobes-ftrace.c
arch/riscv/kernel/probes/ftrace.c
arch/s390/kernel/ftrace.c
arch/x86/kernel/kprobes/ftrace.c
include/linux/kprobes.h
kernel/kprobes.c
kernel/trace/ftrace.c

index 834cffcfbce32039a18301de4424c58812b814ae..7ba4b98076de1e83669f13b08b765cba31dcd5ae 100644 (file)
@@ -12,6 +12,9 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
        struct kprobe_ctlblk *kcb;
        struct pt_regs *regs;
 
+       if (unlikely(kprobe_ftrace_disabled))
+               return;
+
        bit = ftrace_test_recursion_trylock(ip, parent_ip);
        if (bit < 0)
                return;
index 73858c9029cc9e65d34728a40df2a96fe5f56271..bff058317062e367941604f97f758b11986002cf 100644 (file)
@@ -287,6 +287,9 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
        struct kprobe *p;
        struct kprobe_ctlblk *kcb;
 
+       if (unlikely(kprobe_ftrace_disabled))
+               return;
+
        bit = ftrace_test_recursion_trylock(ip, parent_ip);
        if (bit < 0)
                return;
index 621a4b386ae4fcc90fa5e2ad9b7ac6b947fd903d..c91f9c2e61ed25e605dacd34a6f270282c022e34 100644 (file)
@@ -206,6 +206,9 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
        struct kprobe *p;
        int bit;
 
+       if (unlikely(kprobe_ftrace_disabled))
+               return;
+
        bit = ftrace_test_recursion_trylock(ip, parent_ip);
        if (bit < 0)
                return;
index 072ebe7f290ba77b5e40a2303e0f856edfa5660f..f8208c027148fdf0332ab9819f66b1f6c10acdf4 100644 (file)
@@ -21,6 +21,9 @@ void kprobe_ftrace_handler(unsigned long nip, unsigned long parent_nip,
        struct pt_regs *regs;
        int bit;
 
+       if (unlikely(kprobe_ftrace_disabled))
+               return;
+
        bit = ftrace_test_recursion_trylock(nip, parent_nip);
        if (bit < 0)
                return;
index 7142ec42e889f996fd2d0b0eb32a54fba1a89128..a69dfa610aa85724e01858cc320a55bd46298bce 100644 (file)
@@ -11,6 +11,9 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
        struct kprobe_ctlblk *kcb;
        int bit;
 
+       if (unlikely(kprobe_ftrace_disabled))
+               return;
+
        bit = ftrace_test_recursion_trylock(ip, parent_ip);
        if (bit < 0)
                return;
index c46381ea04ecb1328e9f06d186ca1aa65ce07a34..7f6f8c438c2654c4eb4fe68867aca5c5c4bd39e4 100644 (file)
@@ -296,6 +296,9 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
        struct kprobe *p;
        int bit;
 
+       if (unlikely(kprobe_ftrace_disabled))
+               return;
+
        bit = ftrace_test_recursion_trylock(ip, parent_ip);
        if (bit < 0)
                return;
index dd2ec14adb77ba97b22e8c849681f4f1ae133c59..15af7e98e161a406f7f48229de59f62a2f9a9538 100644 (file)
@@ -21,6 +21,9 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
        struct kprobe_ctlblk *kcb;
        int bit;
 
+       if (unlikely(kprobe_ftrace_disabled))
+               return;
+
        bit = ftrace_test_recursion_trylock(ip, parent_ip);
        if (bit < 0)
                return;
index 0ff44d6633e33956c2828da6e341da5136eb3ad7..5fcbc254d1864720de61bd568506375bd49b4eaa 100644 (file)
@@ -378,11 +378,15 @@ static inline void wait_for_kprobe_optimizer(void) { }
 extern void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
                                  struct ftrace_ops *ops, struct ftrace_regs *fregs);
 extern int arch_prepare_kprobe_ftrace(struct kprobe *p);
+/* Set when ftrace has been killed: kprobes on ftrace must be disabled for safety */
+extern bool kprobe_ftrace_disabled __read_mostly;
+extern void kprobe_ftrace_kill(void);
 #else
 static inline int arch_prepare_kprobe_ftrace(struct kprobe *p)
 {
        return -EINVAL;
 }
+static inline void kprobe_ftrace_kill(void) {}
 #endif /* CONFIG_KPROBES_ON_FTRACE */
 
 /* Get the kprobe at this addr (if any) - called with preemption disabled */
@@ -495,6 +499,9 @@ static inline void kprobe_flush_task(struct task_struct *tk)
 static inline void kprobe_free_init_mem(void)
 {
 }
+static inline void kprobe_ftrace_kill(void)
+{
+}
 static inline int disable_kprobe(struct kprobe *kp)
 {
        return -EOPNOTSUPP;
index 65adc815fc6e63027e1b7f0b23c597475a3fea1e..166ebf81dc45025176d6ca4faed94cf7657b51dd 100644 (file)
@@ -1068,6 +1068,7 @@ static struct ftrace_ops kprobe_ipmodify_ops __read_mostly = {
 
 static int kprobe_ipmodify_enabled;
 static int kprobe_ftrace_enabled;
+bool kprobe_ftrace_disabled;
 
 static int __arm_kprobe_ftrace(struct kprobe *p, struct ftrace_ops *ops,
                               int *cnt)
@@ -1136,6 +1137,11 @@ static int disarm_kprobe_ftrace(struct kprobe *p)
                ipmodify ? &kprobe_ipmodify_ops : &kprobe_ftrace_ops,
                ipmodify ? &kprobe_ipmodify_enabled : &kprobe_ftrace_enabled);
 }
+
+void kprobe_ftrace_kill()
+{
+       kprobe_ftrace_disabled = true;
+}
 #else  /* !CONFIG_KPROBES_ON_FTRACE */
 static inline int arm_kprobe_ftrace(struct kprobe *p)
 {
index da1710499698b0ed56ea89183d18befda56c0243..96db99c347b3b01bf10ded57ef342d9640d69070 100644 (file)
@@ -7895,6 +7895,7 @@ void ftrace_kill(void)
        ftrace_disabled = 1;
        ftrace_enabled = 0;
        ftrace_trace_function = ftrace_stub;
+       kprobe_ftrace_kill();
 }
 
 /**