[CRIU] RPC support for --shell-job missing on restore

[Andrew Vagin]

On Fri, May 16, 2014 at 05:16:02PM +0300, Ruslan Kuprieiev wrote:
> On 16.05.2014 17:08, Andrew Vagin wrote:
> >On Fri, May 16, 2014 at 04:49:16PM +0300, Ruslan Kuprieiev wrote:
> >>On 16.05.2014 16:28, Andrew Vagin wrote:
> >>>On Fri, May 16, 2014 at 08:33:41AM -0400, Christopher Covington wrote:
> >>>>Hi Allan,
> >>>>
> >>>>On 05/16/2014 03:44 AM, Allan Cecil wrote:
> >>>>>On 2014-05-15 06:59, Pavel Emelyanov wrote:
> >>>>>>I'd like to notice here, that right now this is only possible if you
> >>>>>>terminate the whole existing process and
> >>>>>>restore one back from scratch using criu. However, we have a feature
> >>>>>>called "applying images" (http://criu.org/Applying_images)
> >>>>>>With it you will not have to kill the original process and the
> >>>>>>revert-back should become MUCH faster.
> >>>>>Applying images sounds very interesting - how do I do that?  I don't see it in
> >>>>>the man page.  I've built from source so I can update or switch branches as
> >>>>>needed.
> >>>>>
> >>>>>>You might b einterested in the --exec-cmd option for criu. It causes
> >>>>>>criu to call execv() on whatever you want after
> >>>>>>restore thus making _your_ code control the restored processes.
> >>>>>I haven't figured out how to make this work, yet.  I'll keep experimenting.  I
> >>>>>thought it was for saying "restore this into this screen session" but that did
> >>>>>not work.
> >>>>>
> >>>>>>>that would still work.  Unfortunately, even if I set the suid bit and
> >>>>>>>attempt to restore from the command line I still get denied
> >>>>>>>bgecause the UID / GID doesn't match.
> >>>>>>Can you shed more light on this? We tried to make it work like -- if
> >>>>>>we have images for user X, then if we restore from
> >>>>>>them from suid-ed criu and the user that does so is X as well, then we
> >>>>>>allow for that. Has that get broken?
> >>>>>>
> >>>>>Here are the exact steps I'm taking.  First, I'm running as the unprivileged
> >>>>>user named tas:
> >>>>>$ id
> >>>>>uid=1001(tas) gid=1001(tas) groups=1001(tas),110(kvm),119(nopasswdlogin)
> >>>>>
> >>>>>I start screen and inside of screen I start nethack.  This produces the
> >>>>>following ps axf -o pid,sid,pgid,uid,gid,comm output:
> >>>>>
> >>>>>  9002  9002  9002  1001    43 screen
> >>>>>  9003  9003  9003  1001  1001  \_ bash
> >>>>>  9165  9003  9165  1001  1001      \_ nethack
> >>>>>
> >>>>>If outside of screen I attempt to issue the command criu dump -v4 -t 9002 as
> >>>>>the same tas user I get this:
> >>>>>
> >>>>>(00.014062) Obtaining task stat ... (00.014126) Error (security.c:34): UID/GID
> >>>>>mismatch 1001 != (1001,43,43)
> >>>>It looks like the session ID isn't matching.
> >>>I have reproduce this bug on my host.
> >>>
> >>>(00.012437) Obtaining task stat ... (00.012592) Error (security.c:34): UID/GID mismatch 1000 != (1000,20,20)
> >>>
> >>>[avagin at localhost criu]$ ps -C nethack
> >>>   PID TTY          TIME CMD
> >>>10962 pts/2    00:00:00 nethack
> >>>[avagin at localhost criu]$ cat /proc/10962/status | grep Gid
> >>>Gid:	1000	20	20	20
> >>>
> >>>[avagin at localhost zzz]$ cat /etc/group | grep 20
> >>>games:x:20:
> >>>
> >>>Add Ruslan Kuprieiev in CC. He is the main security specialist in CRIU;)
> >>Thanks Andrew. =)
> >>
> >>If i get it right, there are some typos in check_ids() from security.c:
> >>static bool check_ids(unsigned int crid, unsigned int rid, unsigned int eid,
> >>unsigned int sid)
> >>{
> >>     if (crid == 0)
> >>         return true;
> >>     if (crid == rid && crid == eid && crid == sid) // Here probably should
> >>be "||"s instead of "&&"s
> >>         return true;
> >>
> >>     pr_err("UID/GID mismatch %u != (%u,%u,%u)\n", crid, rid, eid, sid);
> >>     return false;
> >>}
> >>
> >>Does it look right to you? If so, will send patch.
> >Yes, it does. Is it correct that if we can kill a task, we should be
> >able to dump it?
> Looks correct to me.
> 
> >static int kill_ok_by_cred(struct task_struct *t)
> >{
> >         const struct cred *cred = current_cred();
> >         const struct cred *tcred = __task_cred(t);
> >
> >         if (uid_eq(cred->euid, tcred->suid) ||
> >             uid_eq(cred->euid, tcred->uid)  ||
> >             uid_eq(cred->uid,  tcred->suid) ||
> >             uid_eq(cred->uid,  tcred->uid))
> >                 return 1;
> >
> >         if (ns_capable(tcred->user_ns, CAP_KILL))
> >                 return 1;
> >
> >         return 0;
> >}
> >
> 
> Sorry if I didn't get smth right, but where and why do you want to put this
> function(kill_ok_by_cred)?

This function from the linux kernel is used to check permission to kill
a "t" task

> 
> >>
> >>>>>(00.014137) Error (cr-dump.c:1438): Check uid (pid: 9002) failed
> >>>>>
> >>>>>For the record, the criu process has the stuid bit set:
> >>>>>
> >>>>>-rwsr-xr-x 1 root root 779899 Apr 25 14:39 /usr/local/sbin/criu
> >>>>>
> >>>>>I'm not sure what to make of this.  Thanks for your thoughts,
> >>>>Can you try `criu dump` of `setsid nethack` instead of `criu dump -j` of
> >>>>`nethack`.
> >>>>
> >>>>Regards,
> >>>>Christopher
> >>>>
> >>>>-- 
> >>>>Employee of Qualcomm Innovation Center, Inc.
> >>>>Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> >>>>hosted by the Linux Foundation.
> >>
> >>-- 
> >>Signed-off-by: Ruslan Kuprieiev kupruser at gmail.com
> >>
> 
> 
> -- 
> Signed-off-by: Ruslan Kuprieiev kupruser at gmail.com
>