Re: [PATCH 1/7] uprobes: kill uprobes_state->count

[Srikar Dronamraju <srikar@linux.vnet.ibm.com>]

* Oleg Nesterov <oleg@redhat.com> [2012-08-08 19:37:37]:

> uprobes_state->count is only needed to avoid the slow path in
> uprobe_pre_sstep_notifier(). It is also checked in uprobe_munmap()
> but ironically its only goal to decrement this counter. However,
> it is very broken. Just some examples:
> 
> - uprobe_mmap() can race with uprobe_unregister() and wrongly
>   increment the counter if it hits the non-uprobe "int3". Note
>   that install_breakpoint() checks ->consumers first and returns
>   -EEXIST if it is NULL.
> 
>   "atomic_sub() if error" in uprobe_mmap() looks obviously wrong
>   too.
> 
> - uprobe_munmap() can race with uprobe_register() and wrongly
>   decrement the counter by the same reason.
> 
> - Suppose an appication tries to increase the mmapped area via
>   sys_mremap(). vma_adjust() does uprobe_munmap(whole_vma) first,
>   this can nullify the counter temporarily and race with another
>   thread which can hit the bp, the application will be killed by
>   SIGTRAP.
> 
> - Suppose an application mmaps 2 consecutive areas in the same file
>   and one (or both) of these areas has uprobes. In the likely case
>   mmap_region()->vma_merge() suceeds. Like above, this leads to
>   uprobe_munmap/uprobe_mmap from vma_merge()->vma_adjust() but then
>   mmap_region() does another uprobe_mmap(resulting_vma) and doubles
>   the counter.
> 
> This patch only removes this counter and fixes the compile errors,
> then we will try to cleanup the changed code and add something else
> instead.
> 
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>

Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>