[CRIU] [PATCH 3/3] x86: use breakpoints instead of tracing syscalls

Cyrill Gorcunov gorcunov at gmail.com
Tue Sep 16 08:31:30 PDT 2014


On Tue, Sep 16, 2014 at 07:00:46PM +0400, Andrey Vagin wrote:
> Currently CRIU traces syscalls to catch a moment, when sigreturn will be
> called.
> In this patch we set breakpoint on the first instruction after sigreturn
> and catch a process in the required moment. It should be much faster.
> 
> I was afraid that sigreturn() restarts syscalls and our breakpoints will
> not be triggered in this case. But actually sigreturn() always returns
> to userspace. Syscalls are restarted by adjusting eip. You can find more
> details in get_task_regs().
> 
> Signed-off-by: Andrey Vagin <avagin at openvz.org>
> ---
>  arch/aarch64/include/asm/parasite-syscall.h |  6 +++
>  arch/arm/include/asm/parasite-syscall.h     |  6 +++
>  arch/x86/crtools.c                          | 71 +++++++++++++++++++++++++++++
>  arch/x86/include/asm/parasite-syscall.h     |  2 +
>  parasite-syscall.c                          |  5 ++
>  5 files changed, 90 insertions(+)
> 
> diff --git a/arch/aarch64/include/asm/parasite-syscall.h b/arch/aarch64/include/asm/parasite-syscall.h
> index 0c07121..5ca586f 100644
> --- a/arch/aarch64/include/asm/parasite-syscall.h
> +++ b/arch/aarch64/include/asm/parasite-syscall.h
> @@ -1,6 +1,8 @@
>  #ifndef __CR_ASM_PARASITE_SYSCALL_H__
>  #define __CR_ASM_PARASITE_SYSCALL_H__
>  
> +#include <errno.h>
> +
>  struct parasite_ctl;
>  
>  #define ARCH_SI_TRAP TRAP_BRKPT
> @@ -15,4 +17,8 @@ void *mmap_seized(struct parasite_ctl *ctl,
>  		  void *addr, size_t length, int prot,
>  		  int flags, int fd, off_t offset);
>  
> +static inline int parasite_stop_after_sigreturn(pid_t pid, struct parasite_ctl *ctl)
> +{
> +	return -ENOSYS;
> +}
>  #endif
> diff --git a/arch/arm/include/asm/parasite-syscall.h b/arch/arm/include/asm/parasite-syscall.h
> index 0c66bf9..4802520 100644
> --- a/arch/arm/include/asm/parasite-syscall.h
> +++ b/arch/arm/include/asm/parasite-syscall.h
> @@ -1,6 +1,7 @@
>  #ifndef __CR_ASM_PARASITE_SYSCALL_H__
>  #define __CR_ASM_PARASITE_SYSCALL_H__
>  
> +#include <errno.h>
>  
>  #define ARCH_SI_TRAP TRAP_BRKPT
>  
> @@ -15,4 +16,9 @@ void *mmap_seized(struct parasite_ctl *ctl,
>  		  void *addr, size_t length, int prot,
>  		  int flags, int fd, off_t offset);
>  
> +static inline int parasite_stop_after_sigreturn(pid_t pid, struct parasite_ctl *ctl)
> +{
> +	return -ENOSYS;
> +}
> +
>  #endif
> diff --git a/arch/x86/crtools.c b/arch/x86/crtools.c
> index b127934..916be95 100644
> --- a/arch/x86/crtools.c
> +++ b/arch/x86/crtools.c
> @@ -1,6 +1,8 @@
>  #include <string.h>
>  #include <unistd.h>
>  #include <elf.h>
> +#include <sys/user.h>
> +#include <sys/wait.h>
>  
>  #include "asm/processor-flags.h"
>  #include "asm/restorer.h"
> @@ -494,3 +496,72 @@ int sigreturn_prep_fpu_frame(struct rt_sigframe *sigframe, fpu_state_t *fpu_stat
>  	return 0;
>  }
>  
> +/* Copied from the gdb header gdb/nat/x86-dregs.h */
> +
> +/* Debug registers' indices.  */
> +#define DR_FIRSTADDR 0
> +#define DR_LASTADDR  3
> +#define DR_NADDR     4  /* The number of debug address registers.  */
> +#define DR_STATUS    6  /* Index of debug status register (DR6).  */
> +#define DR_CONTROL   7  /* Index of debug control register (DR7).  */
> +
> +#define DR_LOCAL_ENABLE_SHIFT   0 /* Extra shift to the local enable bit.  */
> +#define DR_GLOBAL_ENABLE_SHIFT  1 /* Extra shift to the global enable bit.  */
> +#define DR_ENABLE_SIZE          2 /* Two enable bits per debug register.  */
> +
> +/* Locally enable the break/watchpoint in the I'th debug register.  */
> +#define X86_DR_LOCAL_ENABLE(i) (1 << (DR_LOCAL_ENABLE_SHIFT + DR_ENABLE_SIZE * (i)))

Please beautify it aligning values

> +
> +int parasite_stop_after_sigreturn(pid_t pid, struct parasite_ctl *ctl)
> +{
> +	int ret, status;
> +
> +	/* Set a breakpoint */
> +	if (ptrace(PTRACE_POKEUSER, pid,
> +			offsetof(struct user, u_debugreg[DR_FIRSTADDR]),
> +			RT_SIGFRAME_REGIP(ctl->sigframe))) {
> +		pr_err("Unable to setup a breakpoint\n");
> +		return 1;
> +	}
> +
> +	/* Clean up the DR_STATUS register */
> +	if (ptrace(PTRACE_POKEUSER, pid,
> +			offsetof(struct user, u_debugreg[DR_STATUS]), 0)) {
> +		pr_err("Unable to setup a breakpoint\n");
> +		return 1;
> +	}
> +
> +	/* Enable the breakpoint */
> +	if (ptrace(PTRACE_POKEUSER, pid,
> +			offsetof(struct user, u_debugreg[DR_CONTROL]),
> +			X86_DR_LOCAL_ENABLE(DR_FIRSTADDR))) {
> +		pr_err("Unable to setup a breakpoint\n");
> +		return 1;
> +	}

Why -1 | 1 return codes?


> +	ret = ptrace(PTRACE_CONT, pid, NULL, NULL);
> +	if (ret) {
> +		pr_perror("ptrace");
> +		return -1;
> +	}
> +
> +	pid = wait4(pid, &status, __WALL, NULL);
> +	if (pid == -1) {
> +		pr_perror("wait4 failed");
> +		return -1;
> +	}
> +
> +	if (!WIFSTOPPED(status) || WSTOPSIG(status) != SIGTRAP) {
> +		pr_err("Task is in unexpected state: %x\n", status);
> +		return -1;
> +	}
> +
> +	/* Disable all breakpoints */
> +	if (ptrace(PTRACE_POKEUSER, pid,
> +			&((struct user *) NULL)->u_debugreg[DR_CONTROL], 0)) {
> +		pr_err("Unable to setup a breakpoint\n");
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
> diff --git a/arch/x86/include/asm/parasite-syscall.h b/arch/x86/include/asm/parasite-syscall.h
> index 4d56cb0..80ca6f1 100644
> --- a/arch/x86/include/asm/parasite-syscall.h
> +++ b/arch/x86/include/asm/parasite-syscall.h
> @@ -17,4 +17,6 @@ void *mmap_seized(struct parasite_ctl *ctl,
>  		  void *addr, size_t length, int prot,
>  		  int flags, int fd, off_t offset);
>  
> +int parasite_stop_after_sigreturn(pid_t pid, struct parasite_ctl *ctl);
> +
>  #endif
> diff --git a/parasite-syscall.c b/parasite-syscall.c
> index 1c1a2cb..4f0b8e5 100644
> --- a/parasite-syscall.c
> +++ b/parasite-syscall.c
> @@ -868,6 +868,11 @@ static int parasite_fini_seized(struct parasite_ctl *ctl)
>  	if (ret)
>  		return -1;
>  
> +	/* CRIU doesn't know how to set breakpoints for this arch */
> +	ret = parasite_stop_after_sigreturn(pid, ctl);
> +	if (ret != ENOSYS)
> +		return ret;

-ENOSYS ?

> +
>  	ret = ptrace(PTRACE_SYSCALL, pid, NULL, NULL);
>  	if (ret) {
>  		pr_perror("ptrace");

other than that looks good.


More information about the CRIU mailing list