[uclibc-ng-devel] libpthread/nptl: bugfix malloc segfault in race conditions.

Message ID 1515650252-2936-1-git-send-email-ren_guo@c-sky.com
State Accepted
Headers show
Series
  • [uclibc-ng-devel] libpthread/nptl: bugfix malloc segfault in race conditions.
Related show

Commit Message

Guo Ren Jan. 11, 2018, 5:57 a.m.
In libc/sysdeps/linux/common/bits/uClibc_pthread.h:
extern void weak_function _pthread_cleanup_push_defer(...)

This *weak_function* declaration will cause nptl/cleanup_defer_compat.c:
strong_alias (...) !!!FAIL!!!, because it include pthreadP.h->pthread.h
->uClibc_pthread.h

That means:
Readelf -s libpthread/nptl/cleanup_defer_compat.o you will get:
18: 00000000   198 FUNC    WEAK   DEFAULT    1 _pthread_cleanup_push_def
Readelf -s libc/misc/internals/__uClibc_main.o you will also get:
32: 00000038    58 FUNC    WEAK   DEFAULT    1 _pthread_cleanup_push_def

Final: gcc malloc_pthread_test.c -lpthread
The libc/stdlib/malloc-standard/malloc.c:839 (__MALLOC_LOCK->
_pthread_cleanup_push_def) will use the one in __uClibc_main.o
!!!not in cleanup_defer_compat.o!!!, becasue two cleanup_defer_compat in
libc.a with the same weak declarations and the __uClibc_main.o is close
to front.

Comments

Waldemar Brodkorb Jan. 15, 2018, 8:03 p.m. | #1
Hi Guo,
Guo Ren wrote,

> In libc/sysdeps/linux/common/bits/uClibc_pthread.h:
> extern void weak_function _pthread_cleanup_push_defer(...)
> 
> This *weak_function* declaration will cause nptl/cleanup_defer_compat.c:
> strong_alias (...) !!!FAIL!!!, because it include pthreadP.h->pthread.h
> ->uClibc_pthread.h
> 
> That means:
> Readelf -s libpthread/nptl/cleanup_defer_compat.o you will get:
> 18: 00000000   198 FUNC    WEAK   DEFAULT    1 _pthread_cleanup_push_def
> Readelf -s libc/misc/internals/__uClibc_main.o you will also get:
> 32: 00000038    58 FUNC    WEAK   DEFAULT    1 _pthread_cleanup_push_def
> 
> Final: gcc malloc_pthread_test.c -lpthread
> The libc/stdlib/malloc-standard/malloc.c:839 (__MALLOC_LOCK->
> _pthread_cleanup_push_def) will use the one in __uClibc_main.o
> !!!not in cleanup_defer_compat.o!!!, becasue two cleanup_defer_compat in
> libc.a with the same weak declarations and the __uClibc_main.o is close
> to front.
> 
> ====
> 
> All of malloc/free will failed in muti-threads' race conditions
> probabilistic.
> 
> As it happens, uClibc-0.9.33.2 is OK, Becasue:
> It's seperated in libpthread.a and libc.a, and the libc.a is the
> last lib for ld internal-cmd, and malloc will get right cleanup_defer_compat
> from libpthread.a.
> 
> This BUG is from 2016-09-24 to now:
> >>>
> commit 29ff9055c80efe77a7130767a9fcb3ab8c67e8ce
> Author: Waldemar Brodkorb <wbx@uclibc-ng.org>
> Date:   Sat Sep 24 02:55:31 2016 +0200
> 
>     use a single libc and deduplicate threading code
> 
>     Similar to musl libc a single libc has many benefits and solves
>     some open issues with uClibc-ng.
> 
>     - no pthread_mutex_* weak symbols exported anymore
>     - applications no longer failing to link when either
>       -lrt or -lpthread are missing for dynamic and static linking mode
>     - smaller C library
>     - slightly better runtime performance
> <<<
> 
> Perharps we need carefully check all of the impact about that commit.

Yes, you are right. There might be more sharp edges.

Thanks for the fix, applied and pushed,
 Waldemar

Patch

====

All of malloc/free will failed in muti-threads' race conditions
probabilistic.

As it happens, uClibc-0.9.33.2 is OK, Becasue:
It's seperated in libpthread.a and libc.a, and the libc.a is the
last lib for ld internal-cmd, and malloc will get right cleanup_defer_compat
from libpthread.a.

This BUG is from 2016-09-24 to now:
>>>
commit 29ff9055c80efe77a7130767a9fcb3ab8c67e8ce
Author: Waldemar Brodkorb <wbx@uclibc-ng.org>
Date:   Sat Sep 24 02:55:31 2016 +0200

    use a single libc and deduplicate threading code

    Similar to musl libc a single libc has many benefits and solves
    some open issues with uClibc-ng.

    - no pthread_mutex_* weak symbols exported anymore
    - applications no longer failing to link when either
      -lrt or -lpthread are missing for dynamic and static linking mode
    - smaller C library
    - slightly better runtime performance
<<<

Perharps we need carefully check all of the impact about that commit.

Signed-off-by: Guo Ren <ren_guo@c-sky.com>
---
 libpthread/nptl/sysdeps/pthread/pthread.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libpthread/nptl/sysdeps/pthread/pthread.h b/libpthread/nptl/sysdeps/pthread/pthread.h
index b2897b3..1fba7fc 100644
--- a/libpthread/nptl/sysdeps/pthread/pthread.h
+++ b/libpthread/nptl/sysdeps/pthread/pthread.h
@@ -29,7 +29,7 @@ 
 #include <bits/pthreadtypes.h>
 #include <bits/setjmp.h>
 #include <bits/wordsize.h>
-#if defined _LIBC && ( defined IS_IN_libc || defined NOT_IN_libc )
+#if defined _LIBC && ( defined IS_IN_libc || !defined NOT_IN_libc )
 #include <bits/uClibc_pthread.h>
 #endif