lkml.org 
[lkml]   [2014]   [Feb]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [CRIU] [PATCH 1/3] prctl: reduce permissions to change boundaries of data, brk and stack
    From
    2014-02-14 23:16 GMT+04:00 Eric W. Biederman <ebiederm@xmission.com>:
    > Cyrill Gorcunov <gorcunov@gmail.com> writes:
    >
    >> On Fri, Feb 14, 2014 at 09:43:14PM +0400, Andrew Vagin wrote:
    >>> > My brain hurts just looking at this patch and how you are justifying it.
    >>> >
    >>> > For the resources you are mucking with below all you have to do is to
    >>> > verify that you are below the appropriate rlimit at all times and no
    >>> > CAP_SYS_RESOURCE check is needed. You only need CAP_SYS_RESOURCE
    >>> > to exceed your per process limits.
    >>> >
    >>> > All you have to do is to fix the current code to properly enforce the
    >>> > limits.
    >>>
    >>> I'm afraid what you are suggesting doesn't work.
    >>>
    >>> The first reason is that we can not change both boundaries in one call.
    >>> But when we are restoring these attributes, we may need to move their
    >>> too far.
    >>
    >> When this code was introduced, there were no user-namespace implementation,
    >> if I remember correctly, so CAP_SYS_RESOURCE was enough barrier point
    >> to prevent modifying this values by anyone. Now user-ns brings a limit --
    >> we need somehow to provide a way to modify these mm fields having no
    >> CAP_SYS_RESOURCE set. "Verifying rlimit" not an option here because
    >> we're modifying members one by one (looking back I think this was not
    >> a good idea to modify the fields in this manner).
    >>
    >> 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.
    > */
    >
    > 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.

    start_brk and start_stack are randomized each time. I don't understand
    how execve() can restore the origin values of attributes.

    >
    > I am hoping we can move this conversation what makes sense from oh ick
    > checkpoint/restort does not work with user namespaces.
    >
    > Eric


    \
     
     \ /
      Last update: 2014-02-15 00:41    [W:2.237 / U:0.584 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site