@@ -213,6 +213,7 @@ extern void kgdb_arch_set_pc(struct pt_regs *regs, unsigned long pc);
/* Optional functions. */
extern int kgdb_validate_break_address(unsigned long addr);
+extern int kgdb_validate_single_step_address(unsigned long addr);
extern int kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt);
extern int kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt);
@@ -215,6 +215,18 @@ int __weak kgdb_validate_break_address(unsigned long addr)
return err;
}
+int __weak kgdb_validate_single_step_address(unsigned long addr)
+{
+ /*
+ * Disallow stepping when we are executing code that is marked
+ * as unsuitable for kprobing.
+ */
+ if (within_kprobe_blacklist(addr))
+ return -EINVAL;
+
+ return 0;
+}
+
unsigned long __weak kgdb_arch_pc(int exception, struct pt_regs *regs)
{
return instruction_pointer(regs);
@@ -1192,6 +1204,7 @@ 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)
{
@@ -1041,8 +1041,16 @@ int gdb_serial_stub(struct kgdb_state *ks)
if (tmp == 0)
break;
/* Fall through - on tmp < 0 */
- case 'c': /* Continue packet */
case 's': /* Single step packet */
+ error = kgdb_validate_single_step_address(
+ kgdb_arch_pc(ks->ex_vector,
+ ks->linux_regs));
+ if (error != 0) {
+ error_packet(remcom_out_buffer, error);
+ break;
+ }
+ fallthrough;
+ case 'c': /* Continue packet */
if (kgdb_contthread && kgdb_contthread != current) {
/* Can't switch threads in kgdb */
error_packet(remcom_out_buffer, -EINVAL);
@@ -507,18 +507,14 @@ static int kdb_bc(int argc, const char **argv)
* None.
* Remarks:
*
- * Set the arch specific option to trigger a debug trap after the next
- * instruction.
+ * KDB_CMD_SS is a command that our caller acts on to effect the step.
*/
static int kdb_ss(int argc, const char **argv)
{
if (argc != 0)
return KDB_ARGCOUNT;
- /*
- * Set trace flag and go.
- */
- KDB_STATE_SET(DOING_SS);
+
return KDB_CMD_SS;
}
@@ -1189,7 +1189,7 @@ static int kdb_local(kdb_reason_t reason, int error, struct pt_regs *regs,
kdb_dbtrap_t db_result)
{
char *cmdbuf;
- int diag;
+ int diag, res;
struct task_struct *kdb_current =
kdb_curr_task(raw_smp_processor_id());
@@ -1346,10 +1346,16 @@ static int kdb_local(kdb_reason_t reason, int error, struct pt_regs *regs,
}
if (diag == KDB_CMD_GO
|| diag == KDB_CMD_CPU
- || diag == KDB_CMD_SS
|| diag == KDB_CMD_KGDB)
break;
+ if (diag == KDB_CMD_SS) {
+ res = kgdb_validate_single_step_address(instruction_pointer(regs));
+ if (res == 0)
+ break;
+ diag = res;
+ }
+
if (diag)
kdb_cmderror(diag);
}
If we are running in a part of the kernel that dislikes breakpoint debugging then it is very unlikely to be safe to single step. Add some safety rails to prevent stepping through anything on the kprobe blacklist. As part of this kdb_ss() will no longer set the DOING_SS flags when it requests a step. This is safe because this flag is already redundant, returning KDB_CMD_SS is all that is needed to request a step (and this saves us from having to unset the flag if the safety check fails). Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org> --- include/linux/kgdb.h | 1 + kernel/debug/debug_core.c | 13 +++++++++++++ kernel/debug/gdbstub.c | 10 +++++++++- kernel/debug/kdb/kdb_bp.c | 8 ++------ kernel/debug/kdb/kdb_main.c | 10 ++++++++-- 5 files changed, 33 insertions(+), 9 deletions(-) -- 2.25.4