Re: [PATCH 1/1] ptrace: make sure do_wait() won't hang after PTRACE_ATTACH

[Tejun Heo <tj@kernel.org>]

Hello, Oleg.

Still trying to follow the new discussion.

On Tue, Feb 15, 2011 at 09:27:47PM +0100, Oleg Nesterov wrote:
> > The reason for the transition to TASK_TRACED is to prevent a race with
> > SIGCONT waking the task.  There is always a race with SIGKILL waking it,
> > but the circumstances where that can really matter are far fewer.
> > You need to make sure that the work PTRACE_GETSIGINFO does to access
> > last_siginfo cannot race with that pointer disappearing or the stack
> > space it points to becoming invalid.  I think the use of siglock ensures
> > that, but Oleg should verify it.
> 
> Yes, I think this is safe.
> 
> I do not really like this idea because it looks a bit strange to treat
> PTRACE_GETSIGINFO specially, and this doesn't solve all problems. And,
> once again, I still hope we can change ptrace_resume() so that it doesn't
> wakeup the stopped (I mean, SIGNAL_STOP_STOPPED) tracee, in this case this
> hack is not needed.
> 
> And. We are going to add the new requests which doesn't need the stopped
> tracee anyway. So we can just add PTRACE_HAS_SIGINFO which returns
> child->last_siginfo != NULL. This looks simpler, and this is compatible.
> Of course this check is racy, but this doesn't matter. PTRACE_GETSIGINFO
> is equally racy if it doesn't change the state to TASK_TRACED.

This is probably where we disagree the most but I think the weird part
isn't making PTRACE_GETSIGINFO exempt from TASK_TRACE transition.  The
weirdness starts when the tracee is put into TASK_STOPPED while being
ptraced.  I think such dual modes of operation inherently lead to
strange problems.

Instead of having simple "a ptracer stops in TASK_TRACED and its
execution is under the control of ptrace", we end up with the tracee
stopping here or there depending on why it stops and the involved
behavioral subtleties like consumption of wait state and the mentioned
GETSIGINFO problem.  The patch which puts the tracee into TASK_TRACED
on ATTACH already fix two problems discussed in this thread without
doing anything wonky.  I think it says a lot.

As it currently stands, SIGSTOP/CONT while ptraced doesn't work and
even if we bend the rules subtly and provide sneaky ways like the
above, userland needs to be modified to make use of it anyway.  I
think it would be far cleaner to simply make ptracee always stop in
TASK_TRACED and give the ptracer a way to notice what's happening to
the tracee w.r.t. group stop, so that it can comply if it wants to.
What do you think?

Thank you.

-- 
tejun