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]

[ping][PATCH][BZ #13862] Reuse of cached stack can cause bounds overrun of thread DTV


Previosuly reported and patch fomulated by Paul Archard
https://sourceware.org/bugzilla/show_bug.cgi?id=13862#c0

GLIBC versions: tested with 2.7, 2.9, 2.11, 2.14 built from official
source (by Paul Archard)
GLIBC versions: tested with 2.18 built from official source
(by MyungJoo Ham)
EGLIBC versions: tested with 2.13 built for Tizen/armv7l
(by Beomho Seo and MyungJoo Ham)

Quoting Paul Archard's message:

In a program satisfying the conditions listed below, reusing a cached
stack causes a bounds overrun of the thread's DTV structure, leading to
a probable crash.  The _dl_allocate_tls_init function re-initializes the
dtv based on the current slotinfo_list, however it is possible for the
dtv to be smaller than the highest module id loaded.  When this happens
the function will write over memory outside of the dtv, leading to
unpredictable behavior and an eventual crash.

See proposed fix below and attached test-case for repro.

Conditions needed:

The use of a relatively large number of dynamic libraries, loaded at
runtime using dlopen

The use of thread-local-storage within those libraries

A thread exiting prior to the number of loaded libraries increasing a
significant amount, followed by a new thread being created after the
number of libraries has increased.

Example Valgrind output:

==27966== Invalid write of size 8
==27966==    at 0x4010A7A: _dl_allocate_tls_init (dl-tls.c:418)
==27966==    by 0x4E35294: pthread_create@@GLIBC_2.2.5
(allocatestack.c:252)

followed by:
==27966==  Address 0x5b4e6d0 is 0 bytes after a block of size 304
alloc'd
==27966==    at 0x4C26D85: calloc (vg_replace_malloc.c:566)
==27966==    by 0x4010439: allocate_dtv (dl-tls.c:297)
==27966==    by 0x4010B4D: _dl_allocate_tls (dl-tls.c:461)
==27966==    by 0x4E357E9: pthread_create@@GLIBC_2.2.5
(allocatestack.c:575)

Tested-by: Beomho Seo <beomho.seo@samsung.com>
Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
---
 ChangeLog    |  5 +++++
 elf/dl-tls.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index b9201fc..39f9ce7 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2013-11-22  MyungJoo Ham  <myungjoo.ham@samsung.com>
+
+	* elf/dl_tls.c: Prevent bound overrun and allocate another chunk of
+	memory when needed for dtv.
+
 2013-11-21  Roland McGrath  <roland@hack.frob.com>
 
 	* malloc/malloc.c: Move #include <sys/param.h> to the top; comment why
diff --git a/elf/dl-tls.c b/elf/dl-tls.c
index 576d9a1..d19c9f5 100644
--- a/elf/dl-tls.c
+++ b/elf/dl-tls.c
@@ -34,14 +34,12 @@
 
 
 /* Out-of-memory handler.  */
-#ifdef SHARED
 static void
 __attribute__ ((__noreturn__))
 oom (void)
 {
   _dl_fatal_printf ("cannot allocate memory for thread-local data: ABORT\n");
 }
-#endif
 
 
 size_t
@@ -387,6 +385,52 @@ _dl_allocate_tls_init (void *result)
      TLS.  For those which are dynamically loaded we add the values
      indicating deferred allocation.  */
   listp = GL(dl_tls_dtv_slotinfo_list);
+
+  /* check if current dtv is big enough */
+  if (dtv[-1].counter < GL(dl_tls_max_dtv_idx))
+  {
+    dtv_t *newp;
+    size_t newsize = GL(dl_tls_max_dtv_idx) + DTV_SURPLUS;
+    size_t oldsize = dtv[-1].counter;
+
+    if (
+#ifdef SHARED
+      dtv == GL(dl_initial_dtv)
+#else
+      0
+#endif
+       )
+    {
+      /* This is the initial dtv that was allocated
+	 during rtld startup using the dl-minimal.c
+	 malloc instead of the real malloc.  We can't
+	 free it, we have to abandon the old storage.  */
+      newp = malloc ((2 + newsize) * sizeof (dtv_t));
+      if (newp == NULL)
+        oom ();
+      memcpy (newp, &dtv[-1], (2 + oldsize) * sizeof (dtv_t));
+    }
+    else
+    {
+      newp = realloc(&dtv[-1], (2 + newsize) * sizeof (dtv_t));
+      if (newp == NULL)
+        oom();
+    }
+
+    newp[0].counter = newsize;
+
+    /* Clear the newly allocated part.  */
+    memset (newp + 2 + oldsize, '\0', (newsize - oldsize) * sizeof (dtv_t));
+
+    /* Point dtv to the generation counter.  */
+    dtv = &newp[1];
+
+    /* Install this new dtv in the given thread */
+    INSTALL_DTV (result, newp);
+
+    assert(dtv[-1].counter >= GL(dl_tls_max_dtv_idx));
+  }
+
   while (1)
     {
       size_t cnt;
-- 
1.8.3.2


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