The rules that I am using are:
mmap_uprobe() increments the count if
- it successfully adds a breakpoint.
- it not add a breakpoint, but sees that there is a underlying
breakpoint (via a read_opcode call).
munmap_uprobe() decrements the count if
- it sees a underlying breakpoint, (via a read_opcode call)
- Subsequent unregister_uprobe wouldnt find the breakpoint
unless a mmap_uprobe kicks in, since the old vma would be
dropped just after munmap_uprobe.
register_uprobe increments the count if:
- it successfully adds a breakpoint.
unregister_uprobe decrements the count if:
- it sees a underlying breakpoint and removes successfully.
(via a read_opcode call)
- Subsequent munmap_uprobe wouldnt find the breakpoint
since there is no underlying breakpoint after the
breakpoint removal.
> >
> > if consumers is NULL, unregister_uprobes() has kicked already in, so
> > there is no point in inserting the probe, Hence we return EEXIST. The
> > following unregister_uprobe() (or the munmap_uprobe() which might race
> > before unregister_uprobe) is also going to decrement the count. So we
> > have a case where the same breakpoint is accounted as removed twice. To
> > offset this, we pretend as if the breakpoint is around by incrementing
> > the count.
>
> There's 2 main cases,
> A) vma_adjust() vs unregister_uprobe() and
> B) mmap() vs unregister_uprobe().
>
> The result of A should be -1 reference in total, since we're removing
> the one probe.
If the breakpoint was never there, then a value of 0 should also be
correct. See case A3a and A3b.
> The result of B should be 0 since we're removing the
> probe and we shouldn't be installing new ones.
>
> A1)
> vma_adjust()
> munmap_uprobe()
> unregister_uprobe()
> mmap_uprobe()
> delete_uprobe()
>
>
> munmap will to -1, mmap will do +1, __unregister_uprobe() which is
> serialized against vma_adjust() will do -1 on either the old or new vma,
> resulting in a grand total of: -1+1-1=-1, OK
Right.
>
> A2) breakpoint is in old, not in new, again two cases:
>
> A2a) __unregister_uprobe() sees old
So unregister_uprobe is called on the vma before vma_adjust.
>
> munmap -1, __unregister_uprobe -1, mmap 0: -2 FAIL
>
So munmap wouldnt decrement because, munmap_uprobe checks to see if the
breakpoint is still around before it increments.
unregister unlike munmap removes the breakpoint too.
> A2b) __unregister_uprobe() sees new
>
So the order would be munmap(), mmap() and unregister_uprobe()
> munmap -1, __unregister_uprobe 0, mmap 0: -1 OK
Right, Since the old vma is gone, the new vma doesnt have the
breakpoint.
>
> A3) breakpoint is in new, not in old, again two cases:
>
> A3a) __unregister_uprobe() sees old
>
So unregister_uprobe is called on the vma before vma_adjust.
> munmap 0, __unregister_uprobe 0, mmap: 1: 1 FAIL
If mmap_uprobe() increments it would mean that breakpoint was already
there. (-EEXIST + read_opcode); since there was no breakpoint, it will
not increment..
0 is the correct value here, Not -1. because there was no probe inserted
or removed.
>
> A3b) __unregister_uprobe() seed new
So the order would be munmap(), mmap() and unregister_uprobe()
>
> munmap 0, __unregister_uprobe -1, mmap: 1: 0 FAIL
>
If mmap_uprobe() increments it would mean that breakpoint was already
there. __unregister_uprobe will decrement. Since we added a new probe
and deleted it, the value 0 is correct here.
> B1)
> unregister_uprobe()
> mmap()
> mmap_uprobe()
> __unregister_uprobe()
> delete_uprobe()
>
> mmap +1, __unregister_uprobe() -1: 0 OK
>
> B2)
> unregister_uprobe()
> mmap()
> __unregister_uprobe()
> mmap_uprobe()
> delete_uprobe()
>
> mmap +1, __unregister_uprobe() 0: +1 FAIL
I think you meant __unregister_uprobe happened before mmap_uprobe.
If mmap_uprobe() increments it would mean that breakpoint was already
there. (-EEXIST + read_opcode); since there was no breakpoint, it will
not increment..
>
>
> > Would it help if I add an extra check in mmap_uprobe?
> >
> > int mmap_uprobe(...) {
> > ....
> > ret = install_breakpoint(vma->vm_mm, uprobe);
> > if (ret == -EEXIST) {
> > if (!read_opcode(vma->vm_mm, vaddr, &opcode) &&
> > (opcode == UPROBES_BKPT_INSN))
> > atomic_inc(&vma->vm_mm->mm_uprobes_count);
> > ret = 0;
> > }
> > ....
> > }
>
> > The extra read_opcode check will tell us if the breakpoint is still
> > around and then only increment the count. (As in it will distinguish if
> > the mmap_uprobe is from vm_adjust).
>
> No, I don't see that fixing A2a for example.
This check should help A3a and B2 cases.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>