Re: [CRIU] [PATCH 1/3] prctl: reduce permissions to change boundaries of data, brk and stack

[ebiederm@xmission.com (Eric W. Biederman)]

Cyrill Gorcunov <gorcunov@gmail.com> writes:

> On Fri, Feb 14, 2014 at 11:47:13PM +0400, Pavel Emelyanov wrote:
>> >> Maybe we could improve this api and provide argument as a pointer
>> >> to a structure, which would have all the fields we're going to
>> >> modify, which in turn would allow us to verify that all new values
>> >> are sane and fit rlimits, then we could (probably) deprecate old
>> >> api if noone except c/r camp is using it (I actually can't imagine
>> >> who else might need this api). Then CAP_SYS_RESOURCE requirement
>> >> could be ripped off. Hm? (sure touching api is always "no-no"
>> >> case, but maybe...)
>> > 
>> > Hmm.  Let me rewind this a little bit.
>> > 
>> > I want to be very stupid and ask the following.
>> > 
>> > Why can't you have the process of interest do:
>> > 	ptrace(PTRACE_ATTACHME);
>> > 	execve(executable, args, ...);
>> >         
>> >         /* Have the ptracer inject the recovery/fixup code */
>> > 	    /* Fix up the mostly correct process to look like it has been
>> >          * executing for a while.
>> >          */
>
> Erik, it seems I don't understand how it will help us to restore
> the mm fields mentioned above?

Because exec is how those mm fields are set when you don't use
prctl_set_mm.  So execpt for the stack and the brk limits that
will simply result in the values being set to what the usually
would be set to.

>> Let's imagine we do that.
>> 
>> This means, that the whole memory contents should be restored _after_
>> the execve() call, since the execve() flushes old mappings. In
>> that case we lose the ability to preserve any shared memory regions
>> between any two processes. This "shared" can be either regular
>> MAP_SHARED mappings or MAP_ANONYMOUS but still not COW-ed ones.
>> 
>> > That should work, set all of the interesting fields, and works as
>> > non-root today.  My gut feel says do that and we can just
>> > deprecate/remove prctl_set_mm.
>> > 
>> > I am hoping we can move this conversation what makes sense from oh ick
>> > checkpoint/restort does not work with user namespaces.
>
> I fear you've got a wrong impression that we're "ick'ing" about user-ns ;)
> Actually it's "must have" feature for containers thus we would _really_
> love to be able to c/r them.

What I meant is that the analysis of how to deal with prctl_set_mm seems
to be knee jerk shallow analysis based upon the fact that things are not
working, and not asking the question what really makes sense here? and
Why are people concerned with these changes and these values?

Eric