Re: [PATCH v2 7/11] Uprobes Implementation

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

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