On Wed, 2012-05-30 at 18:58 +0200, Oleg Nesterov wrote: > install_breakpoint() returns -EEXIST if is_swbp_insn(orig_insn) == T, > the caller treats this code as success. > > This is doubly wrong. The successful return should set UPROBE_COPY_INSN, > but the real problem is that it shouldn't succeed. If the probed insn is > int3 the application should get SIGTRAP, this won't happen with uprobe. > > Probably we can fix this, we can add the UPROBE_SHARED_BP flag and teach > handle_swbp/set_orig_insn to handle this case correctly. But this needs > some complications and we have other insns which can't be probed, lets > make a simple fix for now. > > I think this needs a cleanup. UPROBE_COPY_INSN should die, copy_insn() > should be called by alloc_uprobe(). arch_uprobe_analyze_insn() depends > on ->mm (ia32_compat) but it is called only once. > > Signed-off-by: Oleg Nesterov <oleg@xxxxxxxxxx> > --- > kernel/events/uprobes.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c > index 8c5e043..1593b43 100644 > --- a/kernel/events/uprobes.c > +++ b/kernel/events/uprobes.c > @@ -704,7 +704,7 @@ install_breakpoint(struct uprobe *uprobe, struct mm_struct *mm, > return ret; > > if (is_swbp_insn((uprobe_opcode_t *)uprobe->arch.insn)) > - return -EEXIST; > + return -ENOTSUPP; > > ret = arch_uprobe_analyze_insn(&uprobe->arch, mm); > if (ret) IIRC this -EEXIST existed because the vma iteration it does is racy and one can encounter the same vma twice or so. See the special -EEXIST handling in register_for_each_vma(). Changing it like this would break stuff. -- 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/