On Thu, Apr 18, 2013 at 08:44:25PM +0200, Oleg Nesterov wrote:
> Change ptrace_detach() to call flush_ptrace_hw_breakpoint(child).
> This frees the slots for non-ptrace PERF_TYPE_BREAKPOINT users, and
> this ensures that the tracee won't be killed by SIGTRAP triggered by
> the active breakpoints.
> 
> Test-case:
> 
>       unsigned long encode_dr7(int drnum, int enable, unsigned int type, 
> unsigned int len)
>       {
>               unsigned long dr7;
> 
>               dr7 = ((len | type) & 0xf)
>                       << (DR_CONTROL_SHIFT + drnum * DR_CONTROL_SIZE);
>               if (enable)
>                       dr7 |= (DR_GLOBAL_ENABLE << (drnum * DR_ENABLE_SIZE));
> 
>               return dr7;
>       }
> 
>       int write_dr(int pid, int dr, unsigned long val)
>       {
>               return ptrace(PTRACE_POKEUSER, pid,
>                               offsetof (struct user, u_debugreg[dr]),
>                               val);
>       }
> 
>       void func(void)
>       {
>       }
> 
>       int main(void)
>       {
>               int pid, stat;
>               unsigned long dr7;
> 
>               pid = fork();
>               if (!pid) {
>                       assert(ptrace(PTRACE_TRACEME, 0,0,0) == 0);
>                       kill(getpid(), SIGHUP);
> 
>                       func();
>                       return 0x13;
>               }
> 
>               assert(pid == waitpid(-1, &stat, 0));
>               assert(WSTOPSIG(stat) == SIGHUP);
> 
>               assert(write_dr(pid, 0, (long)func) == 0);
>               dr7 = encode_dr7(0, 1, DR_RW_EXECUTE, DR_LEN_1);
>               assert(write_dr(pid, 7, dr7) == 0);
> 
>               assert(ptrace(PTRACE_DETACH, pid, 0,0) == 0);
>               assert(pid == waitpid(-1, &stat, 0));
>               assert(stat == 0x1300);
> 
>               return 0;
>       }
> 
> Before this patch the child is killed after PTRACE_DETACH.
> 
> Signed-off-by: Oleg Nesterov <o...@redhat.com>
> ---
>  kernel/ptrace.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/kernel/ptrace.c b/kernel/ptrace.c
> index 776ab3b..33752d9 100644
> --- a/kernel/ptrace.c
> +++ b/kernel/ptrace.c
> @@ -467,6 +467,7 @@ static int ptrace_detach(struct task_struct *child, 
> unsigned int data)
>       /* Architecture-specific hardware disable .. */
>       ptrace_disable(child);
>       clear_tsk_thread_flag(child, TIF_SYSCALL_TRACE);
> +     flush_ptrace_hw_breakpoint(child);

So I assume the tracee is still guaranteed to be stopped at that time, right? 
Or already
killed? But it can't be concurrently killed given the patch you did that 
prevented that?

I'm just asking to make sure we can't have two concurrent calls to 
flush_ptrace_hw_breakpoint()
at the same time.

Also it seems to be a regression since we brought the breakpoint/perf 
infrastructure. But
backporting this patch prior to "ptrace: ensure arch_ptrace/ptrace_request can 
never race with SIGKILL"
might be racy. Hmm...

Thanks.

>  
>       write_lock_irq(&tasklist_lock);
>       /*
> -- 
> 1.5.5.1
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to