Messages in this thread | | | Subject | Re: [PATCH 1/3] uprobes: install_breakpoint() should fail if is_swbp_insn() == T | From | Peter Zijlstra <> | Date | Wed, 30 May 2012 19:49:02 +0200 |
| |
On Wed, 2012-05-30 at 23:07 +0530, Srikar Dronamraju wrote: > > > > > > Signed-off-by: Oleg Nesterov <oleg@redhat.com> > > > --- > > > 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. > > > > Peter, > > is_swbp_insn() is looking at the copy of the instruction thats read from > the file. This path is only taken even before any mm's are inserted with > the breakpoint instruction. > > We still check and return -EEXIST if the memory while inserting the breakpoint > instruction already has a breakpoint. > > Hence this change is correct.
Oh, indeed. I overlooked copy_insn() is taking the page from the page-cache instead of the page-tables and will thus get the original.
OK, no worries then.
| |