lkml.org 
[lkml]   [2012]   [May]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 1/3] uprobes: install_breakpoint() should fail if is_swbp_insn() == T
From
Date
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.


\
 
 \ /
  Last update: 2012-05-30 20:01    [W:0.390 / U:0.216 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site