lkml.org 
[lkml]   [2012]   [Mar]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/2] kgdb,debug_core,kgdbts: End DEBUG_RODATA limitation using kprobe breakpoints
(2012/03/22 2:55), Jason Wessel wrote:
> There has long been a limitation using software breakpoints with a
> kernel compiled with CONFIG_DEBUG_RODATA. The kprobe breakpoint code
> has its own text_poke() function which accommodates writing a
> breakpoint into a read-only page. The debug_core can make use of the
> text_poke() capabilities by using the kprobes API, specifically
> arch_arm_kprobe() and arch_disarm_kprobe(). For now it is safe to use
> a single statically allocated kprobe structure to call the kprobes API
> because the debug_core breakpoint API is only used when the kernel is
> in the debug state.

You might misunderstand it. arch_*_kprobe() are not open APIs.
Those are kprobes internal APIs (which means that those functions
should be used only by kprobes).


> The debug_core will first attempt to use the traditional
> probe_kernel_write(), and next try using a kprobe breakpoint. The
> kgdb test suite was updated to run all the software breakpoint tests
> when using a kernel with built with CONFIG_DEBUG_RODATA.
>
> Signed-off-by: Jason Wessel <jason.wessel@windriver.com>

Nak.

[...]
> @@ -165,17 +173,48 @@ int __weak kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt)
> {
> int err;
>
> + bpt->type = BP_BREAKPOINT;
> err = probe_kernel_read(bpt->saved_instr, (char *)bpt->bpt_addr,
> BREAK_INSTR_SIZE);
> if (err)
> return err;
> err = probe_kernel_write((char *)bpt->bpt_addr,
> arch_kgdb_ops.gdb_bpt_instr, BREAK_INSTR_SIZE);
> +#if defined(CONFIG_KPROBES) && defined(CONFIG_DEBUG_RODATA)
> + if (!err)
> + return err;
> + probe_write_tmp.addr = (kprobe_opcode_t *)bpt->bpt_addr;
> + arch_arm_kprobe(&probe_write_tmp);

No, please don't use kprobes internal function this way, because
you can't ensure that the arch_arm_kprobe() has no side-effect.

Why don't you use text_poke()? I see that the text_poke()
is only for x86, but you already have arch/x86/kernel/kgdb.c for
making your own wrapper function.

> + err = probe_kernel_read(&probe_write_tmp.opcode, (char *)bpt->bpt_addr,
> + BREAK_INSTR_SIZE);
> + if (err)
> + return err;
> + if (memcmp(&probe_write_tmp.opcode, arch_kgdb_ops.gdb_bpt_instr,
> + BREAK_INSTR_SIZE))
> + return -EINVAL;
> + bpt->type = BP_KPROBE_BREAKPOINT;
> +#endif /* CONFIG_KPROBES && CONFIG_DEBUG_RODATA */
> return err;
> }
>
> int __weak kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt)
> {
> +#if defined(CONFIG_KPROBES) && defined(CONFIG_DEBUG_RODATA)
> + int err;
> +
> + if (bpt->type != BP_KPROBE_BREAKPOINT)
> + goto knl_write;
> + probe_write_tmp.addr = (kprobe_opcode_t *)bpt->bpt_addr;
> + memcpy(&probe_write_tmp.opcode, bpt->saved_instr, BREAK_INSTR_SIZE);
> + arch_disarm_kprobe(&probe_write_tmp);

Ditto.

> + err = probe_kernel_read(&probe_write_tmp.opcode, (char *)bpt->bpt_addr,
> + BREAK_INSTR_SIZE);
> + if (err ||
> + memcmp(&probe_write_tmp.opcode, bpt->saved_instr, BREAK_INSTR_SIZE))
> + goto knl_write;
> + return err;
> +knl_write:
> +#endif /* CONFIG_KPROBES && CONFIG_DEBUG_RODATA */
> return probe_kernel_write((char *)bpt->bpt_addr,
> (char *)bpt->saved_instr, BREAK_INSTR_SIZE);
> }
> @@ -294,7 +333,6 @@ int dbg_set_sw_break(unsigned long addr)
> return -E2BIG;
>
> kgdb_break[breakno].state = BP_SET;
> - kgdb_break[breakno].type = BP_BREAKPOINT;
> kgdb_break[breakno].bpt_addr = addr;
>
> return 0;

Thank you,


--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com


\
 
 \ /
  Last update: 2012-03-22 03:57    [W:0.086 / U:0.116 seconds]
©2003-2014 Jasper Spaans. Advertise on this site