Re: [PATCH 6/6] ptrace: PTRACE_DETACH should doflush_ptrace_hw_breakpoint(child)

[Oleg Nesterov]

On 04/29, Frederic Weisbecker wrote:
>
> On Thu, Apr 18, 2013 at 08:44:25PM +0200, Oleg Nesterov wrote:
> > 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?

Yes.

This is only called by PTRACE_DETACH which requires the stopped tracee,
like all ptrace requests except PTRACE_KILL/INTERRUPT. And only one
thread (the tracer) can do this.

> But it can't be concurrently killed given the patch you did that prevented that?

No, it can't. To clarify, the tracee can't run even if killed.

And just in case... If the tracer exits and does the implicit detach,
ptrace_detach() (and thus flush_ptrace_hw_breakpoint()) is not called,
that would be wrong exactly because we can race with the tracee.

> Also it seems to be a regression since we brought the breakpoint/perf
> infrastructure.

No, I think this (minor) problem is very old... At least, when I look
at 2.6.26 code I do not see anything which coould clear db regs on
detach.

> backporting this patch prior to "ptrace: ensure arch_ptrace/ptrace_request can never race with SIGKILL"
> might be racy.

Yes, unlikely this is possible or even makes sense, the problem is
minor.



Btw. perhaps flush_ptrace_hw_breakpoint() should also clear the
virtual registers like thread.debugreg7 ? Even without this patch,
flush_ is also called exec.

Oleg.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/