[PATCH] [BZ #19402] Clear list of acquired robust mutexes in the child process after forking.

[Florian Weimer]

On 12/22/2016 11:15 AM, Torvald Riegel wrote:
> I've asked for comments on whether mutexes acquired before fork() remain
> to be acquired by just the parent process after fork():
> https://sourceware.org/ml/libc-alpha/2016-12/msg00772.html

The conceptual problem here is that we need to effectively 
process-shared mutexes differently from those which are process-private. 
  With effectively process-shared, I mean that the mutex object resides 
on a mapping which is not copied by fork, but remains shared with the 
parent.

I'm going to comment on the other thread as well.

> The fix is:
>
> Robust mutexes acquired at the time of a call to fork() do not remain
> acquired by the forked child process.  We have to clear the list of
> acquired robust mutexes before registering this list with the kernel;
> otherwise, if some of the robust mutexes are process-shared, the parent
> process can alter the child's robust mutex list, which can lead to
> deadlocks or even modification of memory that may not be occupied by a
> mutex anymore.
>
> Tested on x86_64-linux with glibc's tests and the reproducer from 19402.

Can we add a test case for this?

> +      /* Initialize the robust mutex list setting in the kernel which has
> +	 been reset during the fork.  We do not check for errors because if
> +	 it fails here, it must have failed at process startup as well and
> +	 nobody could have used robust mutexes.
> +	 Before we do that, we have to clear the list of robust mutexes
> +	 because we do not inherit ownership of mutexes from the parent.
> +	 We do not have to set self->robust_head.futex_offset since we do
> +	 inherit the correct value from the parent.  We do not need to clear
> +	 the pending operation because it must have been zero when fork was
> +	 called.  */

fork is supposed to be async-signal-safe, so I'm not sure the comment 
about the pending operation is completely correct.

(Our fork implementation is currently not async-signal-safe, though.)

> +# ifdef __PTHREAD_MUTEX_HAVE_PREV
> +      self->robust_prev = &self->robust_head;
> +# endif
> +      self->robust_head.list = &self->robust_head;
>  # ifdef SHARED
>        if (__builtin_expect (__libc_pthread_functions_init, 0))
>  	PTHFCT_CALL (ptr_set_robust, (self));

The change as such looks good to me, assuming this is the semantics we 
want.  I checked it matches the initialization code, and it comes before 
the set_robust_list call, as required.

Thanks,
Florian