Re: [PATCH] [BZ #15392] Remove fork child pid assertion

["Carlos O'Donell" <carlos at redhat dot com>]

On 11/18/2014 09:32 AM, Torvald Riegel wrote:
> On Mon, 2014-11-17 at 14:20 -0800, Ricky Zhou wrote:
>> On Mon, Nov 17, 2014 at 2:38 AM, Torvald Riegel <triegel@redhat.com> wrote:
>>> On Fri, 2014-11-14 at 17:29 -0800, Ricky Zhou wrote:
>>>> This assertion is no longer always true, since a forked child may be in
>>>> a different PID namespace than its parent, and the two namespace may
>>>> have PID collisions.
>>>
>>> If that's true, process-shared synchronization facilities based on the
>>> thread ID (e.g., mutexes) might not work anymore.
>>>
>>> Do we at least need a note in the documentation that this is the case?
>>> Or do we still need to implement it?
>>>
>>> Is there any glibc-internal synchronization that this could break?
>>>
>>> I believe we deal with these issues before dropping the assert.
>> Within a PID namespace, all thread IDs should remain unique, and a
>> process cannot have threads in different PID namespaces
>> (http://lxr.free-electrons.com/source/kernel/fork.c#L1217), so I'm
>> having trouble coming up with a situation where removing this assert
>> would allow badness to happen down the line.
> 
> Okay, so private (intra-process) mutexes and such are safe.  However,
> what about process-shared mutexes?

I agree with Torvald, and I have further comments.

All of pthreads expects POSIX requirements, which are that the PIDs
are stable for the life of the process. If setns is called, then the
fork'd child violates that invariant and enters undefined behaviour.

The only tenable solution I see is documenting that you must use clone
to enter the new namespace. The documentation should note that fork
is forbidden after setns and unshare.

The patch I would like to see is:

* Add minimal stub entry for `setns` and `unshare` to 
  manual/threads.texi and mention that after calling this
  function that fork must not be used, and clone is the
  only safe function to call.

If you have problems using clone, then we need to talk about that
interface. The feedback from lxc has been that they would like not
to have to worry about the stack requirement in clone(), but we
haven't done anything to make that easier.

Cheers,
Carlos.