Messages in this thread | | | Date | Sat, 15 Feb 2014 00:44:46 +0400 | Subject | Re: [CRIU] [PATCH 1/3] prctl: reduce permissions to change boundaries of data, brk and stack | From | Andrey Wagin <> |
| |
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
| |