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

[Oleg Nesterov <oleg@redhat.com>]

On 02/17, Denys Vlasenko wrote:
>
> Hi Jan,
>
> Thanks for joining!

Yep ;)

> On Wednesday 16 February 2011 22:51, Jan Kratochvil wrote:
>
> > The current upstream GDB trick of
> >   PTRACE_ATTACH
> >   if /proc/PID/status->State: == `T (stopped)'
> >     tgkill(SIGSTOP)
> >     PTRACE_CONT(0)
> >   waitpid->SIGSTOP (or preceded by some other signal but 1x SIGSTOP will come)
>
> I don't fully understand the steps of the trick.

Please see my reply to Jan. In short: ->exit_code can be zero after
attach. This is unlikely case after 90bc8d8b, but still possible.

> I believe there is a proposal to add PTRACE_ATTACH_NOSTOP+PTRACE_INTERRUPT,

(This is more or less orthogonal to the discussed problem, I think
 we should discuss this separately).

> > That is after PTRACE_DETACH(0) the process should remain `T (stopped)'
> > iff the process was `T (stopped)' before PTRACE_ATTACH.
> >  - PTRACE_DETACH(0)       should preserve `T (stopped)'.
>
> I assume you are thinking about PTRACE_ATTACH + wait():SIGSTOP
> + PTRACE_DETACH(0) sequence.

plus it should be stopped before attach, I assume. Otherwise this
not true with the current code.

> It looks logical to use 0 in *this* sequence, but consider
> the following sequence:
>
> ....
> ptrace(PTRACE_CONT, 0)
> waitpid(): got SIGFOO
> ptrace(PTRACE_CONT/SYSCALL/DETACH, 0)

Agreed, and this matches (I hope) my reply.

> Regarding PTRACE_ATTACH + wait():SIGSTOP + PTRACE_DETACH(???)
> sequence. I think this SIGSTOP should be considered by kernel
> not to be a signal delivery ptrace stop - because this SIGSTOP
> is "artificial".

Well, this is another case when we have what we have. Unfortunately
it is not artificial. We can add the new PTRACE_ATTACH_* requests
(and we are going to do this), but I don't think we can change this.

> Actually, if you do waitpid():SIGTRAP + ptrace(PTRACE_DETACH, <anything>),
> <anything> will be ignored, because tracee is not in signal delivery ptrace stop,

Well, to clarify, this depends on SIGTRAP above. But in general yes,
<anything> can be ignored. In particular, it _is_ ignored when the
tracee is stopped (T (stopped)). I do not know was it supposed or not,
but this is how the code currently works.

Perhaps ptrace interface should give more respect to SIGXXX argument,
I have no idea. But let me repeat, imho this is another issue.

> So, the only special trick you seem to want is to make ptrace(PTRACE_DETACH, SIGCONT)
> to forcibly unpause stopped task, even if done from non-signal ptrace stop. Right?
>
> I guess this can be special-cased, but can't the same be trivially achieved by
> kill(SIGCONT) + ptrace(PTRACE_DETACH, SIGCONT)?

Hmm. Agreed. Contrary to what I said in the previous email.

> This will avoid the need to special case in the kernel...

And note that this currently doesn't work anyway.

Oleg.