diff mbox series

[RFC,3/4] kgdb: Add NOKPROBE labels on the trap handler functions

Message ID 20200605132130.1411255-4-daniel.thompson@linaro.org
State Superseded
Headers show
Series kgdb: Honour the kprobe blacklist when setting breakpoints | expand

Commit Message

Daniel Thompson June 5, 2020, 1:21 p.m. UTC
Currently kgdb honours the kprobe blacklist but doesn't place its own
trap handling code on the list. Add macros to discourage attempting to
use kgdb to debug itself.

These changes do not make it impossible to provoke recursive trapping
since they do not cover all the calls that can be made on kgdb's entry
logic. However going much further whilst we are sharing the kprobe
blacklist risks reducing the capabilities of kprobe and this is a bad
trade off (especially so given kgdb's users are currently conditioned to
avoid recursive traps).

Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>

---
 kernel/debug/debug_core.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

-- 
2.25.4

Comments

Masami Hiramatsu (Google) June 11, 2020, 12:43 p.m. UTC | #1
On Fri,  5 Jun 2020 14:21:29 +0100
Daniel Thompson <daniel.thompson@linaro.org> wrote:

> Currently kgdb honours the kprobe blacklist but doesn't place its own

> trap handling code on the list. Add macros to discourage attempting to

> use kgdb to debug itself.

> 

> These changes do not make it impossible to provoke recursive trapping

> since they do not cover all the calls that can be made on kgdb's entry

> logic. However going much further whilst we are sharing the kprobe

> blacklist risks reducing the capabilities of kprobe and this is a bad

> trade off (especially so given kgdb's users are currently conditioned to

> avoid recursive traps).

> 

> Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>

> ---

>  kernel/debug/debug_core.c | 9 ++++++++-

>  1 file changed, 8 insertions(+), 1 deletion(-)

> 

> diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c

> index 4a2df4509fe1..21d1d91da4bb 100644

> --- a/kernel/debug/debug_core.c

> +++ b/kernel/debug/debug_core.c

> @@ -184,6 +184,7 @@ int __weak kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt)

>  	return probe_kernel_write((char *)bpt->bpt_addr,

>  				  (char *)bpt->saved_instr, BREAK_INSTR_SIZE);

>  }

> +NOKPROBE_SYMBOL(kgdb_arch_remove_breakpoint);

>  

>  int __weak kgdb_validate_break_address(unsigned long addr)

>  {

> @@ -321,6 +322,7 @@ static void kgdb_flush_swbreak_addr(unsigned long addr)

>  	/* Force flush instruction cache if it was outside the mm */

>  	flush_icache_range(addr, addr + BREAK_INSTR_SIZE);

>  }

> +NOKPROBE_SYMBOL(kgdb_flush_swbreak_addr);

>  

>  /*

>   * SW breakpoint management:

> @@ -411,6 +413,7 @@ int dbg_deactivate_sw_breakpoints(void)

>  	}

>  	return ret;

>  }

> +NOKPROBE_SYMBOL(dbg_deactivate_sw_breakpoints);

>  

>  int dbg_remove_sw_break(unsigned long addr)

>  {

> @@ -567,6 +570,7 @@ static int kgdb_reenter_check(struct kgdb_state *ks)

>  

>  	return 1;

>  }

> +NOKPROBE_SYMBOL(kgdb_reenter_check);

>  

>  static void dbg_touch_watchdogs(void)

>  {

> @@ -801,6 +805,7 @@ static int kgdb_cpu_enter(struct kgdb_state *ks, struct pt_regs *regs,

>  

>  	return kgdb_info[cpu].ret_state;

>  }

> +NOKPROBE_SYMBOL(kgdb_cpu_enter);

>  

>  /*

>   * kgdb_handle_exception() - main entry point from a kernel exception

> @@ -845,6 +850,7 @@ kgdb_handle_exception(int evector, int signo, int ecode, struct pt_regs *regs)

>  		arch_kgdb_ops.enable_nmi(1);

>  	return ret;

>  }

> +NOKPROBE_SYMBOL(kgdb_handle_exception);

>  

>  /*

>   * GDB places a breakpoint at this function to know dynamically loaded objects.

> @@ -879,6 +885,7 @@ int kgdb_nmicallback(int cpu, void *regs)

>  #endif

>  	return 1;

>  }

> +NOKPROBE_SYMBOL(kgdb_nmicallback);

>  

>  int kgdb_nmicallin(int cpu, int trapnr, void *regs, int err_code,

>  							atomic_t *send_ready)

> @@ -904,6 +911,7 @@ int kgdb_nmicallin(int cpu, int trapnr, void *regs, int err_code,

>  #endif

>  	return 1;

>  }

> +NOKPROBE_SYMBOL(kgdb_nmicallin);

>  

>  static void kgdb_console_write(struct console *co, const char *s,

>     unsigned count)

> @@ -1204,7 +1212,6 @@ noinline void kgdb_breakpoint(void)

>  	atomic_dec(&kgdb_setting_breakpoint);

>  }

>  EXPORT_SYMBOL_GPL(kgdb_breakpoint);

> -NOKPROBE_SYMBOL(kgdb_breakpoint);


BTW, why did you mark this NOKPROBE in 2/4 and remove it 3/4 again?

Thank you,


-- 
Masami Hiramatsu <mhiramat@kernel.org>
diff mbox series

Patch

diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c
index 4a2df4509fe1..21d1d91da4bb 100644
--- a/kernel/debug/debug_core.c
+++ b/kernel/debug/debug_core.c
@@ -184,6 +184,7 @@  int __weak kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt)
 	return probe_kernel_write((char *)bpt->bpt_addr,
 				  (char *)bpt->saved_instr, BREAK_INSTR_SIZE);
 }
+NOKPROBE_SYMBOL(kgdb_arch_remove_breakpoint);
 
 int __weak kgdb_validate_break_address(unsigned long addr)
 {
@@ -321,6 +322,7 @@  static void kgdb_flush_swbreak_addr(unsigned long addr)
 	/* Force flush instruction cache if it was outside the mm */
 	flush_icache_range(addr, addr + BREAK_INSTR_SIZE);
 }
+NOKPROBE_SYMBOL(kgdb_flush_swbreak_addr);
 
 /*
  * SW breakpoint management:
@@ -411,6 +413,7 @@  int dbg_deactivate_sw_breakpoints(void)
 	}
 	return ret;
 }
+NOKPROBE_SYMBOL(dbg_deactivate_sw_breakpoints);
 
 int dbg_remove_sw_break(unsigned long addr)
 {
@@ -567,6 +570,7 @@  static int kgdb_reenter_check(struct kgdb_state *ks)
 
 	return 1;
 }
+NOKPROBE_SYMBOL(kgdb_reenter_check);
 
 static void dbg_touch_watchdogs(void)
 {
@@ -801,6 +805,7 @@  static int kgdb_cpu_enter(struct kgdb_state *ks, struct pt_regs *regs,
 
 	return kgdb_info[cpu].ret_state;
 }
+NOKPROBE_SYMBOL(kgdb_cpu_enter);
 
 /*
  * kgdb_handle_exception() - main entry point from a kernel exception
@@ -845,6 +850,7 @@  kgdb_handle_exception(int evector, int signo, int ecode, struct pt_regs *regs)
 		arch_kgdb_ops.enable_nmi(1);
 	return ret;
 }
+NOKPROBE_SYMBOL(kgdb_handle_exception);
 
 /*
  * GDB places a breakpoint at this function to know dynamically loaded objects.
@@ -879,6 +885,7 @@  int kgdb_nmicallback(int cpu, void *regs)
 #endif
 	return 1;
 }
+NOKPROBE_SYMBOL(kgdb_nmicallback);
 
 int kgdb_nmicallin(int cpu, int trapnr, void *regs, int err_code,
 							atomic_t *send_ready)
@@ -904,6 +911,7 @@  int kgdb_nmicallin(int cpu, int trapnr, void *regs, int err_code,
 #endif
 	return 1;
 }
+NOKPROBE_SYMBOL(kgdb_nmicallin);
 
 static void kgdb_console_write(struct console *co, const char *s,
    unsigned count)
@@ -1204,7 +1212,6 @@  noinline void kgdb_breakpoint(void)
 	atomic_dec(&kgdb_setting_breakpoint);
 }
 EXPORT_SYMBOL_GPL(kgdb_breakpoint);
-NOKPROBE_SYMBOL(kgdb_breakpoint);
 
 static int __init opt_kgdb_wait(char *str)
 {