Skip to content

Commit 6c216ec

Browse files
torvaldstorvalds
committed
Merge tag 'for_linus-3.4-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb
Pull KGDB/KDB regression fixes from Jason Wessel: - Fix a Smatch warning that appeared in the 3.4 merge window - Fix kgdb test suite with SMP for all archs without HW single stepping - Fix kgdb sw breakpoints with CONFIG_DEBUG_RODATA=y limitations on x86 - Fix oops on kgdb test suite with CONFIG_DEBUG_RODATA - Fix kgdb test suite with SMP for all archs with HW single stepping * tag 'for_linus-3.4-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb: x86,kgdb: Fix DEBUG_RODATA limitation using text_poke() kgdb,debug_core: pass the breakpoint struct instead of address and memory kgdbts: (2 of 2) fix single step awareness to work correctly with SMP kgdbts: (1 of 2) fix single step awareness to work correctly with SMP kgdbts: Fix kernel oops with CONFIG_DEBUG_RODATA kdb: Fix smatch warning on dbg_io_ops->is_console
2 parents 58bca4a + 3751d3e commit 6c216ec

5 files changed

Lines changed: 204 additions & 78 deletions

File tree

‎arch/x86/kernel/kgdb.c‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@
4343
#include <linux/smp.h>
4444
#include <linux/nmi.h>
4545
#include <linux/hw_breakpoint.h>
46+
#include <linux/uaccess.h>
47+
#include <linux/memory.h>
4648

4749
#include <asm/debugreg.h>
4850
#include <asm/apicdef.h>
@@ -741,6 +743,64 @@ void kgdb_arch_set_pc(struct pt_regs *regs, unsigned long ip)
741743
regs->ip = ip;
742744
}
743745

746+
int kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt)
747+
{
748+
int err;
749+
char opc[BREAK_INSTR_SIZE];
750+
751+
bpt->type = BP_BREAKPOINT;
752+
err = probe_kernel_read(bpt->saved_instr, (char *)bpt->bpt_addr,
753+
BREAK_INSTR_SIZE);
754+
if (err)
755+
return err;
756+
err = probe_kernel_write((char *)bpt->bpt_addr,
757+
arch_kgdb_ops.gdb_bpt_instr, BREAK_INSTR_SIZE);
758+
#ifdef CONFIG_DEBUG_RODATA
759+
if (!err)
760+
return err;
761+
/*
762+
* It is safe to call text_poke() because normal kernel execution
763+
* is stopped on all cores, so long as the text_mutex is not locked.
764+
*/
765+
if (mutex_is_locked(&text_mutex))
766+
return -EBUSY;
767+
text_poke((void *)bpt->bpt_addr, arch_kgdb_ops.gdb_bpt_instr,
768+
BREAK_INSTR_SIZE);
769+
err = probe_kernel_read(opc, (char *)bpt->bpt_addr, BREAK_INSTR_SIZE);
770+
if (err)
771+
return err;
772+
if (memcmp(opc, arch_kgdb_ops.gdb_bpt_instr, BREAK_INSTR_SIZE))
773+
return -EINVAL;
774+
bpt->type = BP_POKE_BREAKPOINT;
775+
#endif /* CONFIG_DEBUG_RODATA */
776+
return err;
777+
}
778+
779+
int kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt)
780+
{
781+
#ifdef CONFIG_DEBUG_RODATA
782+
int err;
783+
char opc[BREAK_INSTR_SIZE];
784+
785+
if (bpt->type != BP_POKE_BREAKPOINT)
786+
goto knl_write;
787+
/*
788+
* It is safe to call text_poke() because normal kernel execution
789+
* is stopped on all cores, so long as the text_mutex is not locked.
790+
*/
791+
if (mutex_is_locked(&text_mutex))
792+
goto knl_write;
793+
text_poke((void *)bpt->bpt_addr, bpt->saved_instr, BREAK_INSTR_SIZE);
794+
err = probe_kernel_read(opc, (char *)bpt->bpt_addr, BREAK_INSTR_SIZE);
795+
if (err || memcmp(opc, bpt->saved_instr, BREAK_INSTR_SIZE))
796+
goto knl_write;
797+
return err;
798+
knl_write:
799+
#endif /* CONFIG_DEBUG_RODATA */
800+
return probe_kernel_write((char *)bpt->bpt_addr,
801+
(char *)bpt->saved_instr, BREAK_INSTR_SIZE);
802+
}
803+
744804
struct kgdb_arch arch_kgdb_ops = {
745805
/* Breakpoint instruction: */
746806
.gdb_bpt_instr = { 0xcc },

‎drivers/misc/kgdbts.c‎

Lines changed: 115 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -134,12 +134,17 @@ static int force_hwbrks;
134134
static int hwbreaks_ok;
135135
static int hw_break_val;
136136
static int hw_break_val2;
137+
static int cont_instead_of_sstep;
138+
static unsigned long cont_thread_id;
139+
static unsigned long sstep_thread_id;
137140
#if defined(CONFIG_ARM) || defined(CONFIG_MIPS) || defined(CONFIG_SPARC)
138141
static int arch_needs_sstep_emulation = 1;
139142
#else
140143
static int arch_needs_sstep_emulation;
141144
#endif
145+
static unsigned long cont_addr;
142146
static unsigned long sstep_addr;
147+
static int restart_from_top_after_write;
143148
static int sstep_state;
144149

145150
/* Storage for the registers, in GDB format. */
@@ -187,7 +192,8 @@ static int kgdbts_unreg_thread(void *ptr)
187192
*/
188193
while (!final_ack)
189194
msleep_interruptible(1500);
190-
195+
/* Pause for any other threads to exit after final ack. */
196+
msleep_interruptible(1000);
191197
if (configured)
192198
kgdb_unregister_io_module(&kgdbts_io_ops);
193199
configured = 0;
@@ -211,7 +217,7 @@ static unsigned long lookup_addr(char *arg)
211217
if (!strcmp(arg, "kgdbts_break_test"))
212218
addr = (unsigned long)kgdbts_break_test;
213219
else if (!strcmp(arg, "sys_open"))
214-
addr = (unsigned long)sys_open;
220+
addr = (unsigned long)do_sys_open;
215221
else if (!strcmp(arg, "do_fork"))
216222
addr = (unsigned long)do_fork;
217223
else if (!strcmp(arg, "hw_break_val"))
@@ -283,6 +289,16 @@ static void hw_break_val_write(void)
283289
hw_break_val++;
284290
}
285291

292+
static int get_thread_id_continue(char *put_str, char *arg)
293+
{
294+
char *ptr = &put_str[11];
295+
296+
if (put_str[1] != 'T' || put_str[2] != '0')
297+
return 1;
298+
kgdb_hex2long(&ptr, &cont_thread_id);
299+
return 0;
300+
}
301+
286302
static int check_and_rewind_pc(char *put_str, char *arg)
287303
{
288304
unsigned long addr = lookup_addr(arg);
@@ -299,13 +315,21 @@ static int check_and_rewind_pc(char *put_str, char *arg)
299315
if (addr + BREAK_INSTR_SIZE == ip)
300316
offset = -BREAK_INSTR_SIZE;
301317
#endif
302-
if (strcmp(arg, "silent") && ip + offset != addr) {
318+
319+
if (arch_needs_sstep_emulation && sstep_addr &&
320+
ip + offset == sstep_addr &&
321+
((!strcmp(arg, "sys_open") || !strcmp(arg, "do_fork")))) {
322+
/* This is special case for emulated single step */
323+
v2printk("Emul: rewind hit single step bp\n");
324+
restart_from_top_after_write = 1;
325+
} else if (strcmp(arg, "silent") && ip + offset != addr) {
303326
eprintk("kgdbts: BP mismatch %lx expected %lx\n",
304327
ip + offset, addr);
305328
return 1;
306329
}
307330
/* Readjust the instruction pointer if needed */
308331
ip += offset;
332+
cont_addr = ip;
309333
#ifdef GDB_ADJUSTS_BREAK_OFFSET
310334
instruction_pointer_set(&kgdbts_regs, ip);
311335
#endif
@@ -315,6 +339,8 @@ static int check_and_rewind_pc(char *put_str, char *arg)
315339
static int check_single_step(char *put_str, char *arg)
316340
{
317341
unsigned long addr = lookup_addr(arg);
342+
static int matched_id;
343+
318344
/*
319345
* From an arch indepent point of view the instruction pointer
320346
* should be on a different instruction
@@ -324,6 +350,29 @@ static int check_single_step(char *put_str, char *arg)
324350
gdb_regs_to_pt_regs(kgdbts_gdb_regs, &kgdbts_regs);
325351
v2printk("Singlestep stopped at IP: %lx\n",
326352
instruction_pointer(&kgdbts_regs));
353+
354+
if (sstep_thread_id != cont_thread_id) {
355+
/*
356+
* Ensure we stopped in the same thread id as before, else the
357+
* debugger should continue until the original thread that was
358+
* single stepped is scheduled again, emulating gdb's behavior.
359+
*/
360+
v2printk("ThrID does not match: %lx\n", cont_thread_id);
361+
if (arch_needs_sstep_emulation) {
362+
if (matched_id &&
363+
instruction_pointer(&kgdbts_regs) != addr)
364+
goto continue_test;
365+
matched_id++;
366+
ts.idx -= 2;
367+
sstep_state = 0;
368+
return 0;
369+
}
370+
cont_instead_of_sstep = 1;
371+
ts.idx -= 4;
372+
return 0;
373+
}
374+
continue_test:
375+
matched_id = 0;
327376
if (instruction_pointer(&kgdbts_regs) == addr) {
328377
eprintk("kgdbts: SingleStep failed at %lx\n",
329378
instruction_pointer(&kgdbts_regs));
@@ -365,10 +414,40 @@ static int got_break(char *put_str, char *arg)
365414
return 1;
366415
}
367416

417+
static void get_cont_catch(char *arg)
418+
{
419+
/* Always send detach because the test is completed at this point */
420+
fill_get_buf("D");
421+
}
422+
423+
static int put_cont_catch(char *put_str, char *arg)
424+
{
425+
/* This is at the end of the test and we catch any and all input */
426+
v2printk("kgdbts: cleanup task: %lx\n", sstep_thread_id);
427+
ts.idx--;
428+
return 0;
429+
}
430+
431+
static int emul_reset(char *put_str, char *arg)
432+
{
433+
if (strncmp(put_str, "$OK", 3))
434+
return 1;
435+
if (restart_from_top_after_write) {
436+
restart_from_top_after_write = 0;
437+
ts.idx = -1;
438+
}
439+
return 0;
440+
}
441+
368442
static void emul_sstep_get(char *arg)
369443
{
370444
if (!arch_needs_sstep_emulation) {
371-
fill_get_buf(arg);
445+
if (cont_instead_of_sstep) {
446+
cont_instead_of_sstep = 0;
447+
fill_get_buf("c");
448+
} else {
449+
fill_get_buf(arg);
450+
}
372451
return;
373452
}
374453
switch (sstep_state) {
@@ -398,9 +477,11 @@ static void emul_sstep_get(char *arg)
398477
static int emul_sstep_put(char *put_str, char *arg)
399478
{
400479
if (!arch_needs_sstep_emulation) {
401-
if (!strncmp(put_str+1, arg, 2))
402-
return 0;
403-
return 1;
480+
char *ptr = &put_str[11];
481+
if (put_str[1] != 'T' || put_str[2] != '0')
482+
return 1;
483+
kgdb_hex2long(&ptr, &sstep_thread_id);
484+
return 0;
404485
}
405486
switch (sstep_state) {
406487
case 1:
@@ -411,8 +492,7 @@ static int emul_sstep_put(char *put_str, char *arg)
411492
v2printk("Stopped at IP: %lx\n",
412493
instruction_pointer(&kgdbts_regs));
413494
/* Want to stop at IP + break instruction size by default */
414-
sstep_addr = instruction_pointer(&kgdbts_regs) +
415-
BREAK_INSTR_SIZE;
495+
sstep_addr = cont_addr + BREAK_INSTR_SIZE;
416496
break;
417497
case 2:
418498
if (strncmp(put_str, "$OK", 3)) {
@@ -424,6 +504,9 @@ static int emul_sstep_put(char *put_str, char *arg)
424504
if (strncmp(put_str, "$T0", 3)) {
425505
eprintk("kgdbts: failed continue sstep\n");
426506
return 1;
507+
} else {
508+
char *ptr = &put_str[11];
509+
kgdb_hex2long(&ptr, &sstep_thread_id);
427510
}
428511
break;
429512
case 4:
@@ -502,10 +585,10 @@ static struct test_struct bad_read_test[] = {
502585
static struct test_struct singlestep_break_test[] = {
503586
{ "?", "S0*" }, /* Clear break points */
504587
{ "kgdbts_break_test", "OK", sw_break, }, /* set sw breakpoint */
505-
{ "c", "T0*", }, /* Continue */
588+
{ "c", "T0*", NULL, get_thread_id_continue }, /* Continue */
589+
{ "kgdbts_break_test", "OK", sw_rem_break }, /*remove breakpoint */
506590
{ "g", "kgdbts_break_test", NULL, check_and_rewind_pc },
507591
{ "write", "OK", write_regs }, /* Write registers */
508-
{ "kgdbts_break_test", "OK", sw_rem_break }, /*remove breakpoint */
509592
{ "s", "T0*", emul_sstep_get, emul_sstep_put }, /* Single step */
510593
{ "g", "kgdbts_break_test", NULL, check_single_step },
511594
{ "kgdbts_break_test", "OK", sw_break, }, /* set sw breakpoint */
@@ -523,16 +606,16 @@ static struct test_struct singlestep_break_test[] = {
523606
static struct test_struct do_fork_test[] = {
524607
{ "?", "S0*" }, /* Clear break points */
525608
{ "do_fork", "OK", sw_break, }, /* set sw breakpoint */
526-
{ "c", "T0*", }, /* Continue */
527-
{ "g", "do_fork", NULL, check_and_rewind_pc }, /* check location */
528-
{ "write", "OK", write_regs }, /* Write registers */
609+
{ "c", "T0*", NULL, get_thread_id_continue }, /* Continue */
529610
{ "do_fork", "OK", sw_rem_break }, /*remove breakpoint */
611+
{ "g", "do_fork", NULL, check_and_rewind_pc }, /* check location */
612+
{ "write", "OK", write_regs, emul_reset }, /* Write registers */
530613
{ "s", "T0*", emul_sstep_get, emul_sstep_put }, /* Single step */
531614
{ "g", "do_fork", NULL, check_single_step },
532615
{ "do_fork", "OK", sw_break, }, /* set sw breakpoint */
533616
{ "7", "T0*", skip_back_repeat_test }, /* Loop based on repeat_test */
534617
{ "D", "OK", NULL, final_ack_set }, /* detach and unregister I/O */
535-
{ "", "" },
618+
{ "", "", get_cont_catch, put_cont_catch },
536619
};
537620

538621
/* Test for hitting a breakpoint at sys_open for what ever the number
@@ -541,16 +624,16 @@ static struct test_struct do_fork_test[] = {
541624
static struct test_struct sys_open_test[] = {
542625
{ "?", "S0*" }, /* Clear break points */
543626
{ "sys_open", "OK", sw_break, }, /* set sw breakpoint */
544-
{ "c", "T0*", }, /* Continue */
545-
{ "g", "sys_open", NULL, check_and_rewind_pc }, /* check location */
546-
{ "write", "OK", write_regs }, /* Write registers */
627+
{ "c", "T0*", NULL, get_thread_id_continue }, /* Continue */
547628
{ "sys_open", "OK", sw_rem_break }, /*remove breakpoint */
629+
{ "g", "sys_open", NULL, check_and_rewind_pc }, /* check location */
630+
{ "write", "OK", write_regs, emul_reset }, /* Write registers */
548631
{ "s", "T0*", emul_sstep_get, emul_sstep_put }, /* Single step */
549632
{ "g", "sys_open", NULL, check_single_step },
550633
{ "sys_open", "OK", sw_break, }, /* set sw breakpoint */
551634
{ "7", "T0*", skip_back_repeat_test }, /* Loop based on repeat_test */
552635
{ "D", "OK", NULL, final_ack_set }, /* detach and unregister I/O */
553-
{ "", "" },
636+
{ "", "", get_cont_catch, put_cont_catch },
554637
};
555638

556639
/*
@@ -693,8 +776,8 @@ static int run_simple_test(int is_get_char, int chr)
693776
/* This callback is a put char which is when kgdb sends data to
694777
* this I/O module.
695778
*/
696-
if (ts.tst[ts.idx].get[0] == '\0' &&
697-
ts.tst[ts.idx].put[0] == '\0') {
779+
if (ts.tst[ts.idx].get[0] == '\0' && ts.tst[ts.idx].put[0] == '\0' &&
780+
!ts.tst[ts.idx].get_handler) {
698781
eprintk("kgdbts: ERROR: beyond end of test on"
699782
" '%s' line %i\n", ts.name, ts.idx);
700783
return 0;
@@ -907,6 +990,17 @@ static void kgdbts_run_tests(void)
907990
if (ptr)
908991
sstep_test = simple_strtol(ptr+1, NULL, 10);
909992

993+
/* All HW break point tests */
994+
if (arch_kgdb_ops.flags & KGDB_HW_BREAKPOINT) {
995+
hwbreaks_ok = 1;
996+
v1printk("kgdbts:RUN hw breakpoint test\n");
997+
run_breakpoint_test(1);
998+
v1printk("kgdbts:RUN hw write breakpoint test\n");
999+
run_hw_break_test(1);
1000+
v1printk("kgdbts:RUN access write breakpoint test\n");
1001+
run_hw_break_test(0);
1002+
}
1003+
9101004
/* required internal KGDB tests */
9111005
v1printk("kgdbts:RUN plant and detach test\n");
9121006
run_plant_and_detach_test(0);
@@ -924,35 +1018,11 @@ static void kgdbts_run_tests(void)
9241018

9251019
/* ===Optional tests=== */
9261020

927-
/* All HW break point tests */
928-
if (arch_kgdb_ops.flags & KGDB_HW_BREAKPOINT) {
929-
hwbreaks_ok = 1;
930-
v1printk("kgdbts:RUN hw breakpoint test\n");
931-
run_breakpoint_test(1);
932-
v1printk("kgdbts:RUN hw write breakpoint test\n");
933-
run_hw_break_test(1);
934-
v1printk("kgdbts:RUN access write breakpoint test\n");
935-
run_hw_break_test(0);
936-
}
937-
9381021
if (nmi_sleep) {
9391022
v1printk("kgdbts:RUN NMI sleep %i seconds test\n", nmi_sleep);
9401023
run_nmi_sleep_test(nmi_sleep);
9411024
}
9421025

943-
#ifdef CONFIG_DEBUG_RODATA
944-
/* Until there is an api to write to read-only text segments, use
945-
* HW breakpoints for the remainder of any tests, else print a
946-
* failure message if hw breakpoints do not work.
947-
*/
948-
if (!(arch_kgdb_ops.flags & KGDB_HW_BREAKPOINT && hwbreaks_ok)) {
949-
eprintk("kgdbts: HW breakpoints do not work,"
950-
"skipping remaining tests\n");
951-
return;
952-
}
953-
force_hwbrks = 1;
954-
#endif /* CONFIG_DEBUG_RODATA */
955-
9561026
/* If the do_fork test is run it will be the last test that is
9571027
* executed because a kernel thread will be spawned at the very
9581028
* end to unregister the debug hooks.

0 commit comments

Comments
 (0)