[OE-core] [PATCH v2] glibc: add ld.so locks in _libc_fork

Tue Sep 5 04:40:26 UTC 2017

On Mon, Sep 4, 2017 at 9:32 PM, Zhixiong Chi <zhixiong.chi at windriver.com> wrote:
> Rebase it to the master branch for glibc 2.26.
>
>
> On 2017年09月05日 12:29, Zhixiong Chi wrote:
>>
>> The patch in this Bugzilla entry was requested by a customer:
>>    https://sourceware.org/bugzilla/show_bug.cgi?id=4578
>>    https://www.sourceware.org/bugzilla/show_bug.cgi?id=19282
>>
>> If a thread happens to hold dl_load_lock and have r_state set to RT_ADD or
>> RT_DELETE at the time another thread calls fork(), then the child exit
>> code
>> from fork (in nptl/sysdeps/unix/sysv/linux/fork.c in our case)
>> re-initializes
>> dl_load_lock but does not restore r_state to RT_CONSISTENT. If the child
>> subsequently requires ld.so functionality before calling exec(), then the
>> assertion will fire.
>>
>> The patch acquires dl_load_lock on entry to fork() and releases it on exit
>> from the parent path.  The child path is initialized as currently done.
>> This is essentially pthreads_atfork, but forced to be first because the
>> acquisition of dl_load_lock must happen before malloc_atfork is active
>> to avoid a deadlock.
>>
>> The __libc_fork() code reset dl_load_lock, but it also needed to reset
>> dl_load_write_lock.

can you send a ping to glibc bugzilla as well

>>
>> Signed-off-by: Zhixiong Chi <zhixiong.chi at windriver.com>
>> ---
>>   ...bc-reset-dl-load-write-lock-after-forking.patch | 37 ++++++++++++++
>>   .../0028-Bug-4578-add-ld.so-lock-while-fork.patch  | 57
>> ++++++++++++++++++++++
>>   meta/recipes-core/glibc/glibc_2.26.bb              |  2 +
>>   3 files changed, 96 insertions(+)
>>   create mode 100644
>> meta/recipes-core/glibc/glibc/0027-glibc-reset-dl-load-write-lock-after-forking.patch
>>   create mode 100644
>> meta/recipes-core/glibc/glibc/0028-Bug-4578-add-ld.so-lock-while-fork.patch
>>
>> diff --git
>> a/meta/recipes-core/glibc/glibc/0027-glibc-reset-dl-load-write-lock-after-forking.patch
>> b/meta/recipes-core/glibc/glibc/0027-glibc-reset-dl-load-write-lock-after-forking.patch
>> new file mode 100644
>> index 0000000..777b253
>> --- /dev/null
>> +++
>> b/meta/recipes-core/glibc/glibc/0027-glibc-reset-dl-load-write-lock-after-forking.patch
>> @@ -0,0 +1,37 @@
>> +From a6bb73d1cfd20a73fbbe6076008376fb87879d1b Mon Sep 17 00:00:00 2001
>> +From: Yuanjie Huang <yuanjie.huang at windriver.com>
>> +Date: Thu, 18 Aug 2016 17:59:13 +0800
>> +Subject: [PATCH] reset dl_load_write_lock after forking
>> +
>> +The patch in this Bugzilla entry was requested by a customer:
>> +
>> +  https://www.sourceware.org/bugzilla/show_bug.cgi?id=19282
>> +
>> +The __libc_fork() code reset dl_load_lock, but it also needed to reset
>> +dl_load_write_lock.  The patch has not yet been integrated upstream.
>> +
>> +Upstream-Status: Pending [ Not Author See bugzilla]
>> +
>> +Signed-off-by: Damodar Sonone <damodar.sonone at kpit.com>
>> +Signed-off-by: Yuanjie Huang <yuanjie.huang at windriver.com>
>> +---
>> + sysdeps/nptl/fork.c | 3 ++-
>> + 1 file changed, 2 insertions(+), 1 deletion(-)
>> +
>> +diff --git a/sysdeps/nptl/fork.c b/sysdeps/nptl/fork.c
>> +index 2b9ae4b..3d0b8da 100644
>> +--- a/sysdeps/nptl/fork.c
>> ++++ b/sysdeps/nptl/fork.c
>> +@@ -174,8 +174,9 @@ __libc_fork (void)
>> +       /* Reset locks in the I/O code.  */
>> +       _IO_list_resetlock ();
>> +
>> +-      /* Reset the lock the dynamic loader uses to protect its data.  */
>> ++      /* Reset the locks the dynamic loader uses to protect its data.
>> */
>> +       __rtld_lock_initialize (GL(dl_load_lock));
>> ++      __rtld_lock_initialize (GL(dl_load_write_lock));
>> +
>> +       /* Run the handlers registered for the child.  */
>> +       while (allp != NULL)
>> +--
>> +1.9.1
>> diff --git
>> a/meta/recipes-core/glibc/glibc/0028-Bug-4578-add-ld.so-lock-while-fork.patch
>> b/meta/recipes-core/glibc/glibc/0028-Bug-4578-add-ld.so-lock-while-fork.patch
>> new file mode 100644
>> index 0000000..f76237a
>> --- /dev/null
>> +++
>> b/meta/recipes-core/glibc/glibc/0028-Bug-4578-add-ld.so-lock-while-fork.patch
>> @@ -0,0 +1,57 @@
>> +The patch in this Bugzilla entry was requested by a customer:
>> +  https://sourceware.org/bugzilla/show_bug.cgi?id=4578
>> +
>> +If a thread happens to hold dl_load_lock and have r_state set to RT_ADD
>> or
>> +RT_DELETE at the time another thread calls fork(), then the child exit
>> code
>> +from fork (in nptl/sysdeps/unix/sysv/linux/fork.c in our case)
>> re-initializes
>> +dl_load_lock but does not restore r_state to RT_CONSISTENT. If the child
>> +subsequently requires ld.so functionality before calling exec(), then the
>> +assertion will fire.
>> +
>> +The patch acquires dl_load_lock on entry to fork() and releases it on
>> exit
>> +from the parent path.  The child path is initialized as currently done.
>> +This is essentially pthreads_atfork, but forced to be first because the
>> +acquisition of dl_load_lock must happen before malloc_atfork is active
>> +to avoid a deadlock.
>> +The patch has not yet been integrated upstream.
>> +
>> +Upstream-Status: Pending [ Not Author See bugzilla]
>> +
>> +Signed-off-by: Raghunath Lolur <Raghunath.Lolur at kpit.com>
>> +Signed-off-by: Yuanjie Huang <yuanjie.huang at windriver.com>
>> +Signed-off-by: Zhixiong Chi <zhixiong.chi at windriver.com>
>> +
>> +Index: git/sysdeps/nptl/fork.c
>> +===================================================================
>> +--- git.orig/sysdeps/nptl/fork.c       2017-08-03 16:02:15.674704080
>> +0800
>> ++++ git/sysdeps/nptl/fork.c    2017-08-04 18:15:02.463362015 +0800
>> +@@ -25,6 +25,7 @@
>> + #include <tls.h>
>> + #include <hp-timing.h>
>> + #include <ldsodefs.h>
>> ++#include <libc-lock.h>
>> + #include <stdio-lock.h>
>> + #include <atomic.h>
>> + #include <nptl/pthreadP.h>
>> +@@ -60,6 +61,10 @@
>> +      but our current fork implementation is not.  */
>> +   bool multiple_threads = THREAD_GETMEM (THREAD_SELF,
>> header.multiple_threads);
>> +
>> ++  /* grab ld.so lock BEFORE switching to malloc_atfork */
>> ++  __rtld_lock_lock_recursive (GL(dl_load_lock));
>> ++  __rtld_lock_lock_recursive (GL(dl_load_write_lock));
>> ++
>> +   /* Run all the registered preparation handlers.  In reverse order.
>> +      While doing this we build up a list of all the entries.  */
>> +   struct fork_handler *runp;
>> +@@ -247,6 +252,10 @@
>> +
>> +         allp = allp->next;
>> +       }
>> ++
>> ++      /* unlock ld.so last, because we locked it first */
>> ++      __rtld_lock_unlock_recursive (GL(dl_load_write_lock));
>> ++      __rtld_lock_unlock_recursive (GL(dl_load_lock));
>> +     }
>> +
>> +   return pid;
>> diff --git a/meta/recipes-core/glibc/glibc_2.26.bb
>> b/meta/recipes-core/glibc/glibc_2.26.bb
>> index d453d8f..135ec4f 100644
>> --- a/meta/recipes-core/glibc/glibc_2.26.bb
>> +++ b/meta/recipes-core/glibc/glibc_2.26.bb
>> @@ -41,6 +41,8 @@ SRC_URI =
>> "${GLIBC_GIT_URI};branch=${SRCBRANCH};name=glibc \
>>
>> file://0024-elf-dl-deps.c-Make-_dl_build_local_scope-breadth-fir.patch \
>>              file://0025-locale-fix-hard-coded-reference-to-gcc-E.patch \
>>
>> file://0026-assert-Suppress-pedantic-warning-caused-by-statement.patch \
>> +           file://0027-glibc-reset-dl-load-write-lock-after-forking.patch
>> \
>> +           file://0028-Bug-4578-add-ld.so-lock-while-fork.patch \
>>   "
>>     NATIVESDKFIXES ?= ""
>
>
> --
> ---------------------
> Thanks,
> Zhixiong Chi
> Tel: +86-10-8477-7036
>
>
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core at lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core