This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] [BZ #19402] Clear list of acquired robust mutexes in the child process after forking.
- From: Florian Weimer <fweimer at redhat dot com>
- To: Torvald Riegel <triegel at redhat dot com>, GLIBC Devel <libc-alpha at sourceware dot org>
- Cc: "Carlos O'Donell" <codonell at redhat dot com>
- Date: Thu, 22 Dec 2016 13:22:03 +0100
- Subject: Re: [PATCH] [BZ #19402] Clear list of acquired robust mutexes in the child process after forking.
- Authentication-results: sourceware.org; auth=none
- References: <1482401752.14990.777.camel@redhat.com>
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