[CRIU] [PATCH 5/8] parasite: restore registers after each parasite command

[Andrew Vagin]

On Tue, Jul 16, 2013 at 01:47:53PM +0400, Pavel Emelyanov wrote:
> On 07/11/2013 01:46 PM, Andrey Vagin wrote:
> > Currenly we can restore blocking signals with help ptrace,
> > so if registers are restored after command, a process state
> > will not be corrupted if criu crashes.
> 
> This doesn't look symmetrical to sigblock restore in parasite_daemonize --
> you restore the sigblock mask _after_ daemonize command (on err path),
> but not the registers.

Sorry I don't understand what do you mean. Pls, look at this code and
point a place. If a process can't be executed, sigmask and registers
will be restored. Here are another order of setting and restoring regs
and sigblock and maybe it's a reason why this looks wrong for you. I can
fix that.

static int parasite_init_daemon(struct parasite_ctl *ctl)
...
        if (ptrace(PTRACE_CONT, pid, NULL, NULL)) {
                pr_perror("Can't continue (pid: %d)\n", pid);
                goto err_mask;
        }

....

err_mask:
        ptrace(PTRACE_SETSIGMASK, pid, sizeof(k_rtsigset_t),
                                &RT_SIGFRAME_UC(ctl->sigframe).uc_sigmask);
err_regs:
        ptrace(PTRACE_SETREGS, pid, NULL, ctl->regs_orig);
err:
        return -1;

> 
> > Signed-off-by: Andrey Vagin <avagin at openvz.org>
> > ---
> >  parasite-syscall.c | 19 +++++++------------
> >  1 file changed, 7 insertions(+), 12 deletions(-)
> > 
> > diff --git a/parasite-syscall.c b/parasite-syscall.c
> > index 2b9877c..07b45bc 100644
> > --- a/parasite-syscall.c
> > +++ b/parasite-syscall.c
> > @@ -84,7 +84,7 @@ int __parasite_execute_trap(struct parasite_ctl *ctl, pid_t pid,
> >  
> >  	if (ptrace(PTRACE_SETREGS, pid, NULL, regs)) {
> >  		pr_perror("Can't set registers (pid: %d)", pid);
> > -		goto err;
> > +		goto err_sigmask;
> >  	}
> >  
> >  	/*
> > @@ -130,7 +130,13 @@ int __parasite_execute_trap(struct parasite_ctl *ctl, pid_t pid,
> >  	 * parasite code. So we're done.
> >  	 */
> >  	ret = 0;
> > +
> >  err:
> > +	if (ptrace(PTRACE_SETREGS, pid, NULL, regs_orig)) {
> > +		pr_perror("Can't restore registers (pid: %d)", pid);
> > +		ret = -1;
> > +	}
> > +err_sigmask:
> >  	if (ptrace(PTRACE_SETSIGMASK, pid, sizeof(k_rtsigset_t), &sigmask)) {
> >  		pr_perror("Can't block signals");
> >  		ret = -1;
> > @@ -164,12 +170,6 @@ static int parasite_execute_trap_by_pid(unsigned int cmd,
> >  	if (ret)
> >  		pr_err("Parasite exited with %d\n", ret);
> >  
> > -	if (ctl->pid.real != pid)
> > -		if (ptrace(PTRACE_SETREGS, pid, NULL, regs_orig)) {
> > -			pr_perror("Can't restore registers (pid: %d)", pid);
> > -			return -1;
> > -		}
> > -
> >  	return ret;
> >  }
> >  
> > @@ -864,11 +864,6 @@ int parasite_cure_remote(struct parasite_ctl *ctl)
> >  		ret = -1;
> >  	}
> >  
> > -	if (ptrace(PTRACE_SETREGS, ctl->pid.real, NULL, &ctl->regs_orig)) {
> > -		pr_err("Can't restore registers (pid: %d)\n", ctl->pid.real);
> > -		ret = -1;
> > -	}
> > -
> >  	return ret;
> >  }
> >  
> > 
> 
>