Commit 1a7d0890 authored by Stephen Brennan's avatar Stephen Brennan Committed by Masami Hiramatsu (Google)

kprobe/ftrace: bail out if ftrace was killed

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: default avatarStephen Brennan <stephen.s.brennan@oracle.com>
Acked-by: default avatarMasami Hiramatsu (Google) <mhiramat@kernel.org>
Acked-by: default avatarGuo Ren <guoren@kernel.org>
Reviewed-by: default avatarSteven Rostedt (Google) <rostedt@goodmis.org>
Signed-off-by: default avatarMasami Hiramatsu (Google) <mhiramat@kernel.org>
parent b7bd96ec
...@@ -12,6 +12,9 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip, ...@@ -12,6 +12,9 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
struct kprobe_ctlblk *kcb; struct kprobe_ctlblk *kcb;
struct pt_regs *regs; struct pt_regs *regs;
if (unlikely(kprobe_ftrace_disabled))
return;
bit = ftrace_test_recursion_trylock(ip, parent_ip); bit = ftrace_test_recursion_trylock(ip, parent_ip);
if (bit < 0) if (bit < 0)
return; return;
......
...@@ -287,6 +287,9 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip, ...@@ -287,6 +287,9 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
struct kprobe *p; struct kprobe *p;
struct kprobe_ctlblk *kcb; struct kprobe_ctlblk *kcb;
if (unlikely(kprobe_ftrace_disabled))
return;
bit = ftrace_test_recursion_trylock(ip, parent_ip); bit = ftrace_test_recursion_trylock(ip, parent_ip);
if (bit < 0) if (bit < 0)
return; return;
......
...@@ -206,6 +206,9 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip, ...@@ -206,6 +206,9 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
struct kprobe *p; struct kprobe *p;
int bit; int bit;
if (unlikely(kprobe_ftrace_disabled))
return;
bit = ftrace_test_recursion_trylock(ip, parent_ip); bit = ftrace_test_recursion_trylock(ip, parent_ip);
if (bit < 0) if (bit < 0)
return; return;
......
...@@ -21,6 +21,9 @@ void kprobe_ftrace_handler(unsigned long nip, unsigned long parent_nip, ...@@ -21,6 +21,9 @@ void kprobe_ftrace_handler(unsigned long nip, unsigned long parent_nip,
struct pt_regs *regs; struct pt_regs *regs;
int bit; int bit;
if (unlikely(kprobe_ftrace_disabled))
return;
bit = ftrace_test_recursion_trylock(nip, parent_nip); bit = ftrace_test_recursion_trylock(nip, parent_nip);
if (bit < 0) if (bit < 0)
return; return;
......
...@@ -11,6 +11,9 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip, ...@@ -11,6 +11,9 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
struct kprobe_ctlblk *kcb; struct kprobe_ctlblk *kcb;
int bit; int bit;
if (unlikely(kprobe_ftrace_disabled))
return;
bit = ftrace_test_recursion_trylock(ip, parent_ip); bit = ftrace_test_recursion_trylock(ip, parent_ip);
if (bit < 0) if (bit < 0)
return; return;
......
...@@ -296,6 +296,9 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip, ...@@ -296,6 +296,9 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
struct kprobe *p; struct kprobe *p;
int bit; int bit;
if (unlikely(kprobe_ftrace_disabled))
return;
bit = ftrace_test_recursion_trylock(ip, parent_ip); bit = ftrace_test_recursion_trylock(ip, parent_ip);
if (bit < 0) if (bit < 0)
return; return;
......
...@@ -21,6 +21,9 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip, ...@@ -21,6 +21,9 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
struct kprobe_ctlblk *kcb; struct kprobe_ctlblk *kcb;
int bit; int bit;
if (unlikely(kprobe_ftrace_disabled))
return;
bit = ftrace_test_recursion_trylock(ip, parent_ip); bit = ftrace_test_recursion_trylock(ip, parent_ip);
if (bit < 0) if (bit < 0)
return; return;
......
...@@ -378,11 +378,15 @@ static inline void wait_for_kprobe_optimizer(void) { } ...@@ -378,11 +378,15 @@ static inline void wait_for_kprobe_optimizer(void) { }
extern void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip, extern void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
struct ftrace_ops *ops, struct ftrace_regs *fregs); struct ftrace_ops *ops, struct ftrace_regs *fregs);
extern int arch_prepare_kprobe_ftrace(struct kprobe *p); 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 #else
static inline int arch_prepare_kprobe_ftrace(struct kprobe *p) static inline int arch_prepare_kprobe_ftrace(struct kprobe *p)
{ {
return -EINVAL; return -EINVAL;
} }
static inline void kprobe_ftrace_kill(void) {}
#endif /* CONFIG_KPROBES_ON_FTRACE */ #endif /* CONFIG_KPROBES_ON_FTRACE */
/* Get the kprobe at this addr (if any) - called with preemption disabled */ /* 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) ...@@ -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_free_init_mem(void)
{ {
} }
static inline void kprobe_ftrace_kill(void)
{
}
static inline int disable_kprobe(struct kprobe *kp) static inline int disable_kprobe(struct kprobe *kp)
{ {
return -EOPNOTSUPP; return -EOPNOTSUPP;
......
...@@ -1068,6 +1068,7 @@ static struct ftrace_ops kprobe_ipmodify_ops __read_mostly = { ...@@ -1068,6 +1068,7 @@ static struct ftrace_ops kprobe_ipmodify_ops __read_mostly = {
static int kprobe_ipmodify_enabled; static int kprobe_ipmodify_enabled;
static int kprobe_ftrace_enabled; static int kprobe_ftrace_enabled;
bool kprobe_ftrace_disabled;
static int __arm_kprobe_ftrace(struct kprobe *p, struct ftrace_ops *ops, static int __arm_kprobe_ftrace(struct kprobe *p, struct ftrace_ops *ops,
int *cnt) int *cnt)
...@@ -1136,6 +1137,11 @@ static int disarm_kprobe_ftrace(struct kprobe *p) ...@@ -1136,6 +1137,11 @@ static int disarm_kprobe_ftrace(struct kprobe *p)
ipmodify ? &kprobe_ipmodify_ops : &kprobe_ftrace_ops, ipmodify ? &kprobe_ipmodify_ops : &kprobe_ftrace_ops,
ipmodify ? &kprobe_ipmodify_enabled : &kprobe_ftrace_enabled); ipmodify ? &kprobe_ipmodify_enabled : &kprobe_ftrace_enabled);
} }
void kprobe_ftrace_kill()
{
kprobe_ftrace_disabled = true;
}
#else /* !CONFIG_KPROBES_ON_FTRACE */ #else /* !CONFIG_KPROBES_ON_FTRACE */
static inline int arm_kprobe_ftrace(struct kprobe *p) static inline int arm_kprobe_ftrace(struct kprobe *p)
{ {
......
...@@ -7895,6 +7895,7 @@ void ftrace_kill(void) ...@@ -7895,6 +7895,7 @@ void ftrace_kill(void)
ftrace_disabled = 1; ftrace_disabled = 1;
ftrace_enabled = 0; ftrace_enabled = 0;
ftrace_trace_function = ftrace_stub; ftrace_trace_function = ftrace_stub;
kprobe_ftrace_kill();
} }
/** /**
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment