[CRIU] [PATCH] ptrace: flush breakpoints

[Andrew Vagin]

On Mon, Sep 22, 2014 at 12:04:35PM +0400, Pavel Emelyanov wrote:
> On 09/19/2014 11:48 PM, Andrey Vagin wrote:
> > Unfortunately the kernel doesn't flush hw breakpoints on
> > detaching ptrace. If a breakpoint is triggered without ptrace, it
> > will be killed by SIGTRAP.
> > 
> > Reported-by: Mr Jenkins
> > Signed-off-by: Andrey Vagin <avagin at openvz.org>
> > ---
> >  arch/aarch64/include/asm/restorer.h |  5 +++++
> >  arch/arm/include/asm/restorer.h     |  5 +++++
> >  arch/x86/crtools.c                  | 13 +++++++++++++
> >  arch/x86/include/asm/restorer.h     |  1 +
> >  cr-restore.c                        | 15 +++++++++++++++
> >  parasite-syscall.c                  |  3 +++
> >  6 files changed, 42 insertions(+)
> > 
> > diff --git a/arch/aarch64/include/asm/restorer.h b/arch/aarch64/include/asm/restorer.h
> > index 53c3847..61e8576 100644
> > --- a/arch/aarch64/include/asm/restorer.h
> > +++ b/arch/aarch64/include/asm/restorer.h
> > @@ -113,4 +113,9 @@ static inline int ptrace_set_breakpoint(pid_t pid, void *addr)
> >  	return 0;
> >  }
> >  
> > +static inline int ptrace_flush_breakpoints(pid_t pid)
> > +{
> > +	return 0;
> > +}
> > +
> >  #endif
> > diff --git a/arch/arm/include/asm/restorer.h b/arch/arm/include/asm/restorer.h
> > index 36ce4e9..8acb2d3 100644
> > --- a/arch/arm/include/asm/restorer.h
> > +++ b/arch/arm/include/asm/restorer.h
> > @@ -155,4 +155,9 @@ static inline int ptrace_set_breakpoint(pid_t pid, void *addr)
> >  	return 0;
> >  }
> >  
> > +static inline int ptrace_flush_breakpoints(pid_t pid)
> > +{
> > +	return 0;
> > +}
> > +
> >  #endif
> > diff --git a/arch/x86/crtools.c b/arch/x86/crtools.c
> > index 1248975..cbbcb9d 100644
> > --- a/arch/x86/crtools.c
> > +++ b/arch/x86/crtools.c
> > @@ -540,3 +540,16 @@ int ptrace_set_breakpoint(pid_t pid, void *addr)
> >  	return 1;
> >  }
> >  
> > +int ptrace_flush_breakpoints(pid_t pid)
> > +{
> > +	/* Disable the breakpoint */
> > +	if (ptrace(PTRACE_POKEUSER, pid,
> > +			offsetof(struct user, u_debugreg[DR_CONTROL]),
> > +			0)) {
> > +		pr_err("Unable to disable the breakpoint\n");
> > +		return -1;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > diff --git a/arch/x86/include/asm/restorer.h b/arch/x86/include/asm/restorer.h
> > index 00d4977..70199fb 100644
> > --- a/arch/x86/include/asm/restorer.h
> > +++ b/arch/x86/include/asm/restorer.h
> > @@ -146,6 +146,7 @@ int sigreturn_prep_fpu_frame(struct rt_sigframe *sigframe, fpu_state_t *fpu_stat
> >  static inline void restore_tls(tls_t *ptls) { (void)ptls; }
> >  
> >  int ptrace_set_breakpoint(pid_t pid, void *addr);
> > +int ptrace_flush_breakpoints(pid_t pid);
> >  
> >  
> >  #endif
> > diff --git a/cr-restore.c b/cr-restore.c
> > index 809564b..7141c6c 100644
> > --- a/cr-restore.c
> > +++ b/cr-restore.c
> > @@ -1599,6 +1599,18 @@ static int attach_to_tasks(bool root_seized, enum trace_flags *flag)
> >  	return 0;
> >  }
> >  
> > +static int clear_breakpoints()
> > +{
> > +	struct pstree_item *item;
> > +	int ret = 0, i;
> > +
> > +	for_each_pstree_item(item)
> > +		for (i = 0; i < item->nr_threads; i++)
> > +			ret |= ptrace_flush_breakpoints(item->threads[i].real);
> 
> Can we optimize this loop out on arches that don't support
> breakpoints?

ARM and ARM64 have support of breakpoints. And lack of this optimization
should motivate arm users to add suppport of breakpoints into CRIU;).

Seriously, I don't want to do this optimization to remove it in a
future.

Thanks,
Andrew

> 
> > +
> > +	return ret;
> > +}
> > +
> >  static void finalize_restore(int status)
> >  {
> >  	struct pstree_item *item;
> > @@ -1775,6 +1787,9 @@ static int restore_root_task(struct pstree_item *init)
> >  		ret = parasite_stop_on_syscall(task_entries->nr_threads,
> >  						__NR_rt_sigreturn, flag);
> >  
> > +	if (clear_breakpoints())
> > +		pr_err("Unable to flush breakpoints\n");
> > +
> >  	/*
> >  	 * finalize_restore() always detaches from processes and
> >  	 * they continue run through sigreturn.
> > diff --git a/parasite-syscall.c b/parasite-syscall.c
> > index 37e9db0..9102b23 100644
> > --- a/parasite-syscall.c
> > +++ b/parasite-syscall.c
> > @@ -890,6 +890,9 @@ static int parasite_fini_seized(struct parasite_ctl *ctl)
> >  	if (parasite_stop_on_syscall(1, __NR_rt_sigreturn, flag))
> >  		return -1;
> >  
> > +	if (ptrace_flush_breakpoints(pid))
> > +		return -1;
> > +
> >  	/*
> >  	 * All signals are unblocked now. The kernel notifies about leaving
> >  	 * syscall before starting to deliver signals. All parasite code are
> > 
>