Currently kgdb has absolutely no safety rails in place to discourage or prevent a user from placing a breakpoint in dangerous places such as the debugger's own trap entry/exit and other places where it is not safe to take synchronous traps.
Modify the default implementation of kgdb_validate_break_address() so that we honour the kprobe blacklist (if there is one). The resulting blacklist will include code that kgdb could, in fact, debug but I think we can assume that anyone with sufficient knowledge to meaningfully debug that code would trivially be able to find and remove the safety rail if they need to. Suggested-by: Peter Zijlstra <pet...@infradead.org> Signed-off-by: Daniel Thompson <daniel.thomp...@linaro.org> --- kernel/debug/debug_core.c | 11 +++++++++++ kernel/debug/kdb/kdb_bp.c | 9 +++++++++ 2 files changed, 20 insertions(+) diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c index ef94e906f05a..81f56d616e04 100644 --- a/kernel/debug/debug_core.c +++ b/kernel/debug/debug_core.c @@ -56,6 +56,7 @@ #include <linux/vmacache.h> #include <linux/rcupdate.h> #include <linux/irq.h> +#include <linux/kprobes.h> #include <asm/cacheflush.h> #include <asm/byteorder.h> @@ -188,6 +189,16 @@ int __weak kgdb_validate_break_address(unsigned long addr) { struct kgdb_bkpt tmp; int err; + + /* + * Disallow breakpoints that are marked as unsuitable for kprobing. + * This check is a little over-zealous because it does include + * code that kgdb is entirely capable of debugging but in exchange + * we can avoid recursive trapping (and all the problems that brings). + */ + if (within_kprobe_blacklist(addr)) + return -EINVAL; + /* Validate setting the breakpoint and then removing it. If the * remove fails, the kernel needs to emit a bad message because we * are deep trouble not being able to put things back the way we diff --git a/kernel/debug/kdb/kdb_bp.c b/kernel/debug/kdb/kdb_bp.c index d7ebb2c79cb8..ec4940146612 100644 --- a/kernel/debug/kdb/kdb_bp.c +++ b/kernel/debug/kdb/kdb_bp.c @@ -306,6 +306,15 @@ static int kdb_bp(int argc, const char **argv) if (!template.bp_addr) return KDB_BADINT; + /* + * This check is redundant (since the breakpoint machinery should + * be doing the same check during kdb_bp_install) but gives the + * user immediate feedback. + */ + diag = kgdb_validate_break_address(template.bp_addr); + if (diag) + return diag; + /* * Find an empty bp structure to allocate */ -- 2.25.4