linux-kernel - Re: [PATCH v2 7/11] Uprobes Implementation

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20100514054243.GQ7426@linux.vnet.ibm.com>
Date:	Fri, 14 May 2010 11:12:43 +0530
From:	Srikar Dronamraju <srikar@...ux.vnet.ibm.com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Oleg Nesterov <oleg@...hat.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...e.hu>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Masami Hiramatsu <mhiramat@...hat.com>,
	Randy Dunlap <rdunlap@...otime.net>,
	Ananth N Mavinakayanahalli <ananth@...ibm.com>,
	Jim Keniston <jkenisto@...ux.vnet.ibm.com>,
	Frederic Weisbecker <fweisbec@...il.com>,
	"Frank Ch. Eigler" <fche@...hat.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Roland McGrath <roland@...hat.com>, Mel Gorman <mel@....ul.ie>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	Andrea Arcangeli <aarcange@...hat.com>,
	Hugh Dickins <hugh.dickins@...cali.co.uk>,
	Rik van Riel <riel@...hat.com>
Subject: Re: [PATCH v2 7/11] Uprobes Implementation

> 
> On Thu, 13 May 2010, Oleg Nesterov wrote:
> > 
> > But I still think this doesn't actually matter, replace_page() shouldn't
> > preserve the mapping, it should always install the anonymous page. I can
> > be wrong, of course.
> 
> Well, if I reasd the patches right, uprobes will use "copy_to_user()" for 
> the self-probing case. So that would definitely just modify a shared 
> mapping.


Uprobes uses copy_to_user() to write data/stack and never to write to
instruction addresses.

To write an instruction uprobes either used access_process_vm or the
replace_page() based background page replacement method. This is true
even if the process was probing itself.

Soon to be posted v4 will revert to background page replacement method
on the lines illustrated by Peter in one of his mails.

> 
> Of course, arguably, who really cares? As long as it's not a security 
> issue (and it isn't - since the person could just have written to the 
> thing directly instead), I guess it doesn't much matter. But it's a bit 
> sad when a probing feature either
> 
>  - changes a global mapping that may be executed by other non-related 
>    processes that the prober isn't even _aware_ of.
> 
>  - changes semantics by creating a non-coherent private page

Do you see these concerns even when uprobes uses background page replacement?

> 
> so arguably it would be good to just make the rule be that you cannot 
> probe a shared mapping. Because whatever you do, it's always the wrong 
> thing.
> 

Yes, I will be adding a check to discard probing if the vma has
VM_SHARED flag set. I have already committed to Oleg on this issue.
I didnt include this check in v3 patchset, because uprobes was using
access_process_vm in v3 patchset and I thought access_process_vm would
do the right thing even if VM_SHARED is set.

--
Thanks and Regards
Srikar
--
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/