linux-kernel - Re: [PATCH 1/7] uprobes: kill uprobes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20120813131850.GF31805@linux.vnet.ibm.com>
Date:	Mon, 13 Aug 2012 18:48:51 +0530
From:	Srikar Dronamraju <srikar@...ux.vnet.ibm.com>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	Ingo Molnar <mingo@...e.hu>, Peter Zijlstra <peterz@...radead.org>,
	Ananth N Mavinakayanahalli <ananth@...ibm.com>,
	Anton Arapov <anton@...hat.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/7] uprobes: kill uprobes_state->count

* Oleg Nesterov <oleg@...hat.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@...hat.com>

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/