Re: [PATCH v18] exec: Fix dead-lock in de_thread with ptrace_attach

["Eric W. Biederman" <ebiederm@xmission.com>]

"Eric W. Biederman" <ebiederm@xmission.com> writes:

> Bernd Edlinger <bernd.edlinger@hotmail.de> writes:
>
>> This introduces signal->exec_bprm, which is used to
>> fix the case when at least one of the sibling threads
>> is traced, and therefore the trace process may dead-lock
>> in ptrace_attach, but de_thread will need to wait for the
>> tracer to continue execution.
>
> A small quibble it isn't a dead lock.  It isn't even really a live lock,
> as it is possible to SIGKILL our way out.
>
> Thinking about this there is a really silly and simple way we can deal
> with this situation for PTRACE_ATTACH.  We can send SIGSTOP and wait for
> the thread to stop before doing anything with cred_guard_mutex.
>
> PTRACE_ATTACH already implies sending SIGSTOP so as long as we have
> enough permissions to send SIGSTOP I don't see that being a problem.
>
> The worst case I can see is that we get a case where we stop the
> process, the permission check fails under cred_guard_mutex and
> and ptrace attach has fails and has to send SIGCONT to undo it's
> premature SIGSTOP.  That might almost be visible, but it would still
> be legitimate because we can still check that we have permission to
> send SIGSTOP.

Bah no I am full of it.

The challenging behavior is in the semantics of the kernel operations.
We need to describe it as such please.

It is the same class of problem as a single threaded process calls exec
with a pipe attached to both stdin and stdout of the new process.

For the stdin and stdout we can say just use pull and nonblocking I/O.

The problem is that both PTRACE_ATTACH and PTRACE_SEIZE block over
the duration of exec, and if exec is waiting for a thread to exit,
and that thread is blocked in PTRACE_EVENT_EXIT waiting for that very
same tracer those processes will hang. Not deadlock.


I haven't seen anyone clearly describe the problem lately so I am
repeating it.


Just looking at the code I don't think there is any fundamental reason
to call commit_creds after de_thread.  If we can change that we can sort
this out without any change in userspace semantics.

If we can't move commit_creds we have to either give
PTRACE_ATTACH/PTRACE_SEIZE a non-block mode, or break out of
PTRACE_EVENT_EXIT in de_thread.

I will post a proof of concept of moving commit_creds in just a minute.

Eric