linux-kernel - Re: [PATCH 0/1] uprobes: Kill __replace_page(), change uprobe_write

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]

Date:	Tue, 10 Dec 2013 12:16:18 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	"H. Peter Anvin" <hpa@...or.com>,
	Ananth N Mavinakayanahalli <ananth@...ibm.com>,
	Andi Kleen <andi@...stfloor.org>,
	Borislav Petkov <bp@...en8.de>,
	Hugh Dickins <hughd@...gle.com>,
	Ingo Molnar <mingo@...nel.org>, Jiri Kosina <jkosina@...e.cz>,
	Peter Zijlstra <peterz@...radead.org>,
	Srikar Dronamraju <srikar@...ux.vnet.ibm.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 0/1] uprobes: Kill __replace_page(), change
 uprobe_write_opcode() to rely on gup(WRITE)

On Tue, Dec 10, 2013 at 12:04 PM, Oleg Nesterov <oleg@...hat.com> wrote:
>> We'd be *much* better off using get_user_pages_fast() if possible -
>> and I bet _is_ possible in 99% of all cases.
>
> We can't. get_user_pages_fast() takes mmap_sem.

Yeah, and we need to look up the page table entry anyway, so what we
actually want here is just the page table walker, none of the "get
page" crap at all.

So the core function should (I think) just do something like:

 - new_page = NULL
 - get page table lock
 - look up page tables
 - if it's dirty and private, or we have a new page for it, replace
the instruction
 - drop page table lock
 - we're done, free the new page if we didn't use it.

with a retry for the "uhhuh, it's not dirty and private, and we don't
have a new page", so we do
 - drop the lock
 - use a "get_user()" or something to page it in.
 - allocate a new page if necessary
 - goto retry

which gets us a single page table walk for the common case, and a
retry for the "uhhuh, we needed to page it in or allocate a new page"
condition.

Put another way: I actually think the existing "__replace_page()" code
is closer to being good than that disgusting uprobe_write_opcode()
function. I think you may be getting rid of the wrong ugly function.

                Linus
--
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/