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

[Masami Hiramatsu <mhiramat@kernel.org>]

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>