1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
|
From f4f8f4d4e0f92488431b268c8cd9555730b9afe9 Mon Sep 17 00:00:00 2001
From: Szabolcs Nagy <szabolcs.nagy@arm.com>
Date: Wed, 30 Dec 2020 19:19:37 +0000
Subject: [PATCH] elf: Use relaxed atomics for racy accesses [BZ #19329]
This is a follow up patch to the fix for bug 19329. This adds relaxed
MO atomics to accesses that were previously data races but are now
race conditions, and where relaxed MO is sufficient.
The race conditions all follow the pattern that the write is behind the
dlopen lock, but a read can happen concurrently (e.g. during tls access)
without holding the lock. For slotinfo entries the read value only
matters if it reads from a synchronized write in dlopen or dlclose,
otherwise the related dtv entry is not valid to access so it is fine
to leave it in an inconsistent state. The same applies for
GL(dl_tls_max_dtv_idx) and GL(dl_tls_generation), but there the
algorithm relies on the fact that the read of the last synchronized
write is an increasing value.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
elf/dl-close.c | 20 +++++++++++++-------
elf/dl-open.c | 5 ++++-
elf/dl-tls.c | 31 +++++++++++++++++++++++--------
sysdeps/x86_64/dl-tls.c | 3 ++-
4 files changed, 42 insertions(+), 17 deletions(-)
---
Upstream-Status: Backport [https://sourceware.org/git/?p=glibc.git;a=patch;h=f4f8f4d4e0f92488431b268c8cd9555730b9afe9]
Comment: Hunks from elf/dl-open.c and elf/dl-tls.c are refreshed due to offset change.
Signed-off-by: Akash Hadke <akash.hadke@kpit.com>
Signed-off-by: Akash Hadke <hadkeakash4@gmail.com>
---
diff --git a/elf/dl-close.c b/elf/dl-close.c
index c51becd06b..3720e47dd1 100644
--- a/elf/dl-close.c
+++ b/elf/dl-close.c
@@ -79,9 +79,10 @@ remove_slotinfo (size_t idx, struct dtv_slotinfo_list *listp, size_t disp,
{
assert (old_map->l_tls_modid == idx);
- /* Mark the entry as unused. */
- listp->slotinfo[idx - disp].gen = GL(dl_tls_generation) + 1;
- listp->slotinfo[idx - disp].map = NULL;
+ /* Mark the entry as unused. These can be read concurrently. */
+ atomic_store_relaxed (&listp->slotinfo[idx - disp].gen,
+ GL(dl_tls_generation) + 1);
+ atomic_store_relaxed (&listp->slotinfo[idx - disp].map, NULL);
}
/* If this is not the last currently used entry no need to look
@@ -96,8 +97,8 @@ remove_slotinfo (size_t idx, struct dtv_slotinfo_list *listp, size_t disp,
if (listp->slotinfo[idx - disp].map != NULL)
{
- /* Found a new last used index. */
- GL(dl_tls_max_dtv_idx) = idx;
+ /* Found a new last used index. This can be read concurrently. */
+ atomic_store_relaxed (&GL(dl_tls_max_dtv_idx), idx);
return true;
}
}
@@ -571,7 +572,9 @@ _dl_close_worker (struct link_map *map, bool force)
GL(dl_tls_dtv_slotinfo_list), 0,
imap->l_init_called))
/* All dynamically loaded modules with TLS are unloaded. */
- GL(dl_tls_max_dtv_idx) = GL(dl_tls_static_nelem);
+ /* Can be read concurrently. */
+ atomic_store_relaxed (&GL(dl_tls_max_dtv_idx),
+ GL(dl_tls_static_nelem));
if (imap->l_tls_offset != NO_TLS_OFFSET
&& imap->l_tls_offset != FORCED_DYNAMIC_TLS_OFFSET)
@@ -769,8 +772,11 @@ _dl_close_worker (struct link_map *map, bool force)
/* If we removed any object which uses TLS bump the generation counter. */
if (any_tls)
{
- if (__glibc_unlikely (++GL(dl_tls_generation) == 0))
+ size_t newgen = GL(dl_tls_generation) + 1;
+ if (__glibc_unlikely (newgen == 0))
_dl_fatal_printf ("TLS generation counter wrapped! Please report as described in "REPORT_BUGS_TO".\n");
+ /* Can be read concurrently. */
+ atomic_store_relaxed (&GL(dl_tls_generation), newgen);
if (tls_free_end == GL(dl_tls_static_used))
GL(dl_tls_static_used) = tls_free_start;
diff --git a/elf/dl-open.c b/elf/dl-open.c
index 09f0df7d38..bb79ef00f1 100644
--- a/elf/dl-open.c
+++ b/elf/dl-open.c
@@ -387,9 +387,12 @@
}
}
- if (__builtin_expect (++GL(dl_tls_generation) == 0, 0))
+ size_t newgen = GL(dl_tls_generation) + 1;
+ if (__glibc_unlikely (newgen == 0))
_dl_fatal_printf (N_("\
TLS generation counter wrapped! Please report this."));
+ /* Can be read concurrently. */
+ atomic_store_relaxed (&GL(dl_tls_generation), newgen);
/* We need a second pass for static tls data, because
_dl_update_slotinfo must not be run while calls to
diff --git a/elf/dl-tls.c b/elf/dl-tls.c
index 94f3cdbae0..dc69cd984e 100644
--- a/elf/dl-tls.c
+++ b/elf/dl-tls.c
@@ -96,7 +96,9 @@
/* No gaps, allocate a new entry. */
nogaps:
- result = ++GL(dl_tls_max_dtv_idx);
+ result = GL(dl_tls_max_dtv_idx) + 1;
+ /* Can be read concurrently. */
+ atomic_store_relaxed (&GL(dl_tls_max_dtv_idx), result);
}
return result;
@@ -279,10 +281,12 @@
dtv_t *dtv;
size_t dtv_length;
+ /* Relaxed MO, because the dtv size is later rechecked, not relied on. */
+ size_t max_modid = atomic_load_relaxed (&GL(dl_tls_max_dtv_idx));
/* We allocate a few more elements in the dtv than are needed for the
initial set of modules. This should avoid in most cases expansions
of the dtv. */
- dtv_length = GL(dl_tls_max_dtv_idx) + DTV_SURPLUS;
+ dtv_length = max_modid + DTV_SURPLUS;
dtv = calloc (dtv_length + 2, sizeof (dtv_t));
if (dtv != NULL)
{
@@ -687,7 +691,7 @@
if (modid > max_modid)
break;
- size_t gen = listp->slotinfo[cnt].gen;
+ size_t gen = atomic_load_relaxed (&listp->slotinfo[cnt].gen);
if (gen > new_gen)
/* Not relevant. */
@@ -699,7 +703,8 @@
continue;
/* If there is no map this means the entry is empty. */
- struct link_map *map = listp->slotinfo[cnt].map;
+ struct link_map *map
+ = atomic_load_relaxed (&listp->slotinfo[cnt].map);
/* Check whether the current dtv array is large enough. */
if (dtv[-1].counter < modid)
{
@@ -843,7 +848,12 @@
{
dtv_t *dtv = THREAD_DTV ();
- if (__glibc_unlikely (dtv[0].counter != GL(dl_tls_generation)))
+ /* Update is needed if dtv[0].counter < the generation of the accessed
+ module. The global generation counter is used here as it is easier
+ to check. Synchronization for the relaxed MO access is guaranteed
+ by user code, see CONCURRENCY NOTES in _dl_update_slotinfo. */
+ size_t gen = atomic_load_relaxed (&GL(dl_tls_generation));
+ if (__glibc_unlikely (dtv[0].counter != gen))
return update_get_addr (GET_ADDR_PARAM);
void *p = dtv[GET_ADDR_MODULE].pointer.val;
@@ -866,7 +876,10 @@
return NULL;
dtv_t *dtv = THREAD_DTV ();
- if (__glibc_unlikely (dtv[0].counter != GL(dl_tls_generation)))
+ /* This may be called without holding the GL(dl_load_lock). Reading
+ arbitrary gen value is fine since this is best effort code. */
+ size_t gen = atomic_load_relaxed (&GL(dl_tls_generation));
+ if (__glibc_unlikely (dtv[0].counter != gen))
{
/* This thread's DTV is not completely current,
but it might already cover this module. */
@@ -961,7 +974,9 @@
/* Add the information into the slotinfo data structure. */
if (do_add)
{
- listp->slotinfo[idx].map = l;
- listp->slotinfo[idx].gen = GL(dl_tls_generation) + 1;
+ /* Can be read concurrently. See _dl_update_slotinfo. */
+ atomic_store_relaxed (&listp->slotinfo[idx].map, l);
+ atomic_store_relaxed (&listp->slotinfo[idx].gen,
+ GL(dl_tls_generation) + 1);
}
}
diff --git a/sysdeps/x86_64/dl-tls.c b/sysdeps/x86_64/dl-tls.c
index 6595f6615b..24ef560b71 100644
--- a/sysdeps/x86_64/dl-tls.c
+++ b/sysdeps/x86_64/dl-tls.c
@@ -40,7 +40,8 @@ __tls_get_addr_slow (GET_ADDR_ARGS)
{
dtv_t *dtv = THREAD_DTV ();
- if (__glibc_unlikely (dtv[0].counter != GL(dl_tls_generation)))
+ size_t gen = atomic_load_relaxed (&GL(dl_tls_generation));
+ if (__glibc_unlikely (dtv[0].counter != gen))
return update_get_addr (GET_ADDR_PARAM);
return tls_get_addr_tail (GET_ADDR_PARAM, dtv, NULL);
--
2.27.0
|
