This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]