[PATCH 1/1] x86, kgdb: correct kgdb_arch_remove_breakpoint

From: zhe.he
Date: Tue Dec 30 2014 - 02:52:19 EST

From: He Zhe <zhe.he@xxxxxxxxxxxxx>

On 3.19-rc2, kgdbts boot time test fails with default parameter V1F100
"KGDB: BP remove failed: ffffffff81049070"
Then system is hanged.

When CONFIG_DEBUG_RODATA is on, kgdb_arch_set_breakpoint firstly tries
probe_kernel_write to set breakpoints and mark their type as BP_BREAKPOINT. If
fails it would use text_poke and mark their type as BP_POKE_BREAKPOINT.

On the other hand, kgdb_arch_remove_breakpoint uses probe_kernel_write to delete
breakpoints if they are BP_BREAKPOINT, or uses text_poke if they are

The kgdbts' boot time test case loops for do_fork and/or sys_open may run
through initialization. During this procedure, the read only area is created. If
a breakpoint is marked as BP_BREAKPOINT before creating read only area and then
its address is put into that area, it would fail to be deleted due to
kgdb_arch_remove_breakpoint would use wrong function.

This patch:
- Make kgdb_arch_remove_breakpoint work like kgdb_arch_set_breakpoint, trying
probe_kernel_write first then trying text_poke if fails.
- Remove BP_POKE_BREAKPOINT as it is only used in these two functions.

Signed-off-by: He Zhe <zhe.he@xxxxxxxxxxxxx>
arch/x86/kernel/kgdb.c | 25 +++++++++++++------------
include/linux/kgdb.h | 1 -
2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kernel/kgdb.c b/arch/x86/kernel/kgdb.c
index 7ec1d5f..f5f7772 100644
--- a/arch/x86/kernel/kgdb.c
+++ b/arch/x86/kernel/kgdb.c
@@ -749,7 +749,6 @@ int kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt)

- bpt->type = BP_BREAKPOINT;
err = probe_kernel_read(bpt->saved_instr, (char *)bpt->bpt_addr,
if (err)
@@ -772,34 +771,36 @@ int kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt)
return err;
if (memcmp(opc, arch_kgdb_ops.gdb_bpt_instr, BREAK_INSTR_SIZE))
return -EINVAL;
- bpt->type = BP_POKE_BREAKPOINT;
return err;

int kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt)
int err;

- if (bpt->type != BP_POKE_BREAKPOINT)
- goto knl_write;
+ err = probe_kernel_write((char *)bpt->bpt_addr,
+ (char *)bpt->saved_instr, BREAK_INSTR_SIZE);
+ if (!err)
+ return err;
* It is safe to call text_poke() because normal kernel execution
* is stopped on all cores, so long as the text_mutex is not locked.
if (mutex_is_locked(&text_mutex))
- goto knl_write;
+ return -EBUSY;
text_poke((void *)bpt->bpt_addr, bpt->saved_instr, BREAK_INSTR_SIZE);
err = probe_kernel_read(opc, (char *)bpt->bpt_addr, BREAK_INSTR_SIZE);
- if (err || memcmp(opc, bpt->saved_instr, BREAK_INSTR_SIZE))
- goto knl_write;
- return err;
+ if (err)
+ return err;
+ if (memcmp(opc, bpt->saved_instr, BREAK_INSTR_SIZE))
+ return -EINVAL;
- return probe_kernel_write((char *)bpt->bpt_addr,
- (char *)bpt->saved_instr, BREAK_INSTR_SIZE);
+ return err;

struct kgdb_arch arch_kgdb_ops = {
diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
index fc513db..cded3c75 100644
--- a/include/linux/kgdb.h
+++ b/include/linux/kgdb.h
@@ -63,7 +63,6 @@ enum kgdb_bptype {

enum kgdb_bpstate {

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/