This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
[BZ#17090/17620/17621]: fix DTV race, assert, and DTV_SURPLUS Static TLS limit
- From: Alexandre Oliva <aoliva at redhat dot com>
- To: libc-alpha at sourceware dot org
- Date: Tue, 18 Nov 2014 17:57:36 -0200
- Subject: [BZ#17090/17620/17621]: fix DTV race, assert, and DTV_SURPLUS Static TLS limit
- Authentication-results: sourceware.org; auth=none
This patch fixes 3 bugs, 2 of which I've just filed to have all of the
issues covered in the bug database.
The original goal was to fix #17620, the problem with Static TLS
overflowing the DTV_SURPLUS range of the DTV, causing unnecessary dlopen
failures, more visible on AArch64 due to TLS Descriptor use by default.
In the process of fixing it, I noticed the race condition reported in
#17621.
The fix for the latter is to move Static TLS DTV entry initialization to
DTV updates by the thread itself, since the DTV entry is never used
except in GD/LD access models.
This enables us to get rid of the limit imposed in dlopen, that arises
precisely from the former need to ensure that all threads' DTVs are big
enough. If we leave it for each thread to update its own DTV, the check
becomes unnecessary and it can be safely dropped.
The assert in bug 17090 was pretty obvious, and since I found it while
looking for dupes of the bugs I filed, and I was touching neighbor code,
I figured I'd just bring it into this patch.
Regression-tested on x86_64-linux-gnu. Ok to install?
for ChangeLog
[BZ #17090]
[BZ #17620]
[BZ #17621]
* NEWS: Update.
* elf/dl-tls.c (_dl_update_slotinfo): Clean up outdated DTV
entries with Static TLS too.
(tls_get_addr_tail): Move Static TLS DTV entry set up from...
(_dl_allocate_tls_init): ... here (fix modid assertion), ...
* elf/dl-reloc.c (_dl_nothread_init_static_tls): ... here...
* nptl/allocatestack.c (init_one_static_tls): ... and here...
* elf/dlopen.c (dl_open_worker): Drop l_tls_modid upper bound
for Static TLS.
---
NEWS | 8 ++++----
elf/dl-open.c | 12 +----------
elf/dl-reloc.c | 6 ------
elf/dl-tls.c | 53 ++++++++++++++++++++++++--------------------------
nptl/allocatestack.c | 9 +++-----
5 files changed, 33 insertions(+), 55 deletions(-)
diff --git a/NEWS b/NEWS
index b152488..4f208a2 100644
--- a/NEWS
+++ b/NEWS
@@ -9,10 +9,10 @@ Version 2.21
* The following bugs are resolved with this release:
- 6652, 12926, 14132, 14138, 14171, 15215, 15884, 17266, 17344, 17363,
- 17370, 17371, 17411, 17460, 17475, 17485, 17501, 17506, 17508, 17522,
- 17555, 17570, 17571, 17572, 17573, 17574, 17582, 17583, 17584, 17585,
- 17589, 17594, 17616.
+ 6652, 12926, 14132, 14138, 14171, 15215, 15884, 17090, 17266, 17344,
+ 17363, 17370, 17371, 17411, 17460, 17475, 17485, 17501, 17506, 17508,
+ 17522, 17555, 17570, 17571, 17572, 17573, 17574, 17582, 17583, 17584,
+ 17585, 17589, 17594, 17616, 17620, 17621.
* The minimum GCC version that can be used to build this version of the GNU
C Library is GCC 4.6. Older GCC versions, and non-GNU compilers, can
diff --git a/elf/dl-open.c b/elf/dl-open.c
index 7cc4cc1..9d6006b 100644
--- a/elf/dl-open.c
+++ b/elf/dl-open.c
@@ -531,17 +531,7 @@ TLS generation counter wrapped! Please report this."));
&& imap->l_tls_blocksize > 0)
{
/* For static TLS we have to allocate the memory here and
- now. This includes allocating memory in the DTV. But we
- cannot change any DTV other than our own. So, if we
- cannot guarantee that there is room in the DTV we don't
- even try it and fail the load.
-
- XXX We could track the minimum DTV slots allocated in
- all threads. */
- if (! RTLD_SINGLE_THREAD_P && imap->l_tls_modid > DTV_SURPLUS)
- _dl_signal_error (0, "dlopen", NULL, N_("\
-cannot load any more object with static TLS"));
-
+ now, but we can delay updating the DTV. */
imap->l_need_tls_init = 0;
#ifdef SHARED
/* Update the slot information data for at least the
diff --git a/elf/dl-reloc.c b/elf/dl-reloc.c
index 97a7119..1d66f79 100644
--- a/elf/dl-reloc.c
+++ b/elf/dl-reloc.c
@@ -136,12 +136,6 @@ _dl_nothread_init_static_tls (struct link_map *map)
# error "Either TLS_TCB_AT_TP or TLS_DTV_AT_TP must be defined"
#endif
- /* Fill in the DTV slot so that a later LD/GD access will find it. */
- dtv_t *dtv = THREAD_DTV ();
- assert (map->l_tls_modid <= dtv[-1].counter);
- dtv[map->l_tls_modid].pointer.val = dest;
- dtv[map->l_tls_modid].pointer.is_static = true;
-
/* Initialize the memory. */
memset (__mempcpy (dest, map->l_tls_initimage, map->l_tls_initimage_size),
'\0', map->l_tls_blocksize - map->l_tls_initimage_size);
diff --git a/elf/dl-tls.c b/elf/dl-tls.c
index 5204fda..2408384 100644
--- a/elf/dl-tls.c
+++ b/elf/dl-tls.c
@@ -437,17 +437,14 @@ _dl_allocate_tls_init (void *result)
assert (listp->slotinfo[cnt].gen <= GL(dl_tls_generation));
maxgen = MAX (maxgen, listp->slotinfo[cnt].gen);
+ dtv[map->l_tls_modid].pointer.val = TLS_DTV_UNALLOCATED;
+ dtv[map->l_tls_modid].pointer.is_static = false;
+
if (map->l_tls_offset == NO_TLS_OFFSET
|| map->l_tls_offset == FORCED_DYNAMIC_TLS_OFFSET)
- {
- /* For dynamically loaded modules we simply store
- the value indicating deferred allocation. */
- dtv[map->l_tls_modid].pointer.val = TLS_DTV_UNALLOCATED;
- dtv[map->l_tls_modid].pointer.is_static = false;
- continue;
- }
+ continue;
- assert (map->l_tls_modid == cnt);
+ assert (map->l_tls_modid == total + cnt);
assert (map->l_tls_blocksize >= map->l_tls_initimage_size);
#if TLS_TCB_AT_TP
assert ((size_t) map->l_tls_offset >= map->l_tls_blocksize);
@@ -459,8 +456,6 @@ _dl_allocate_tls_init (void *result)
#endif
/* Copy the initialization image and clear the BSS part. */
- dtv[map->l_tls_modid].pointer.val = dest;
- dtv[map->l_tls_modid].pointer.is_static = true;
memset (__mempcpy (dest, map->l_tls_initimage,
map->l_tls_initimage_size), '\0',
map->l_tls_blocksize - map->l_tls_initimage_size);
@@ -632,10 +627,9 @@ _dl_update_slotinfo (unsigned long int req_modid)
might still be allocated. */
if (! dtv[total + cnt].pointer.is_static
&& dtv[total + cnt].pointer.val != TLS_DTV_UNALLOCATED)
- {
- free (dtv[total + cnt].pointer.val);
- dtv[total + cnt].pointer.val = TLS_DTV_UNALLOCATED;
- }
+ free (dtv[total + cnt].pointer.val);
+ dtv[total + cnt].pointer.val = TLS_DTV_UNALLOCATED;
+ dtv[total + cnt].pointer.is_static = false;
continue;
}
@@ -698,10 +692,8 @@ _dl_update_slotinfo (unsigned long int req_modid)
memalign and not malloc. */
free (dtv[modid].pointer.val);
- /* This module is loaded dynamically- We defer memory
- allocation. */
- dtv[modid].pointer.is_static = false;
dtv[modid].pointer.val = TLS_DTV_UNALLOCATED;
+ dtv[modid].pointer.is_static = false;
if (modid == req_modid)
the_map = map;
@@ -739,7 +731,6 @@ tls_get_addr_tail (GET_ADDR_ARGS, dtv_t *dtv, struct link_map *the_map)
the_map = listp->slotinfo[idx].map;
}
- again:
/* Make sure that, if a dlopen running in parallel forces the
variable into static storage, we'll wait until the address in the
static TLS block is set up, and use that. If we're undecided
@@ -753,22 +744,28 @@ tls_get_addr_tail (GET_ADDR_ARGS, dtv_t *dtv, struct link_map *the_map)
the_map->l_tls_offset = FORCED_DYNAMIC_TLS_OFFSET;
__rtld_lock_unlock_recursive (GL(dl_load_lock));
}
- else
+ else if (__builtin_expect (the_map->l_tls_offset
+ != FORCED_DYNAMIC_TLS_OFFSET, 1))
{
+#if TLS_TCB_AT_TP
+ void *p = (char *) THREAD_SELF - the_map->l_tls_offset;
+#elif TLS_DTV_AT_TP
+ void *p = (char *) THREAD_SELF + the_map->l_tls_offset + TLS_PRE_TCB_SIZE;
+#else
+# error "Either TLS_TCB_AT_TP or TLS_DTV_AT_TP must be defined"
+#endif
__rtld_lock_unlock_recursive (GL(dl_load_lock));
- if (__builtin_expect (the_map->l_tls_offset
- != FORCED_DYNAMIC_TLS_OFFSET, 1))
- {
- void *p = dtv[GET_ADDR_MODULE].pointer.val;
- if (__glibc_unlikely (p == TLS_DTV_UNALLOCATED))
- goto again;
- return (char *) p + GET_ADDR_OFFSET;
- }
+ dtv[GET_ADDR_MODULE].pointer.is_static = true;
+ dtv[GET_ADDR_MODULE].pointer.val = p;
+
+ return (char *) p + GET_ADDR_OFFSET;
}
+ else
+ __rtld_lock_unlock_recursive (GL(dl_load_lock));
}
void *p = dtv[GET_ADDR_MODULE].pointer.val = allocate_and_init (the_map);
- dtv[GET_ADDR_MODULE].pointer.is_static = false;
+ assert (!dtv[GET_ADDR_MODULE].pointer.is_static);
return (char *) p + GET_ADDR_OFFSET;
}
diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c
index 8cf0274..8b053c1 100644
--- a/nptl/allocatestack.c
+++ b/nptl/allocatestack.c
@@ -1190,7 +1190,6 @@ __nptl_setxid (struct xid_command *cmdp)
static inline void __attribute__((always_inline))
init_one_static_tls (struct pthread *curp, struct link_map *map)
{
- dtv_t *dtv = GET_DTV (TLS_TPADJ (curp));
# if TLS_TCB_AT_TP
void *dest = (char *) curp - map->l_tls_offset;
# elif TLS_DTV_AT_TP
@@ -1199,11 +1198,9 @@ init_one_static_tls (struct pthread *curp, struct link_map *map)
# error "Either TLS_TCB_AT_TP or TLS_DTV_AT_TP must be defined"
# endif
- /* Fill in the DTV slot so that a later LD/GD access will find it. */
- dtv[map->l_tls_modid].pointer.val = dest;
- dtv[map->l_tls_modid].pointer.is_static = true;
-
- /* Initialize the memory. */
+ /* We cannot delay the initialization of the Static TLS area, since
+ it can be accessed with LE or IE, but since the DTV is only used
+ by GD and LD, we can delay its update to avoid a race. */
memset (__mempcpy (dest, map->l_tls_initimage, map->l_tls_initimage_size),
'\0', map->l_tls_blocksize - map->l_tls_initimage_size);
}
--
Alexandre Oliva, freedom fighter http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/ FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer