Skip to content

Commit b0478df

Browse files
committed
tls: make local-exec tls aligned
The patch 70547a6 added support of local-exec TLS used by pies and position-dependant executables. Unfortunately it missed to make sure that static TLS in this case is properly aligned per alignment in TLS segment. This patch fixes it by making sure TLS init is placed at aligned offset and similarly the TLS variables are accessed using the aligned offset. Signed-off-by: Waldemar Kozaczuk <[email protected]>
1 parent cccc7d4 commit b0478df

File tree

5 files changed

+27
-16
lines changed

5 files changed

+27
-16
lines changed

arch/x64/arch-elf.cc

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,11 +128,11 @@ bool object::arch_relocate_rela(u32 type, u32 sym, void *addr,
128128
auto sm = symbol(sym);
129129
ulong tls_offset;
130130
if (sm.obj->is_executable()) {
131-
tls_offset = sm.obj->get_tls_size();
132131
// If this is an executable (pie or position-dependant one)
133132
// then the variable is located in the reserved slot of the TLS
134133
// right where the kernel TLS lives
135-
// So the offset is negative size of this ELF TLS block
134+
// So the offset is negative aligned size of this ELF TLS block
135+
tls_offset = sm.obj->get_aligned_tls_size();
136136
} else {
137137
// If shared library, the variable is located in one of TLS
138138
// blocks that are part of the static TLS before kernel part
@@ -190,7 +190,7 @@ void object::prepare_local_tls(std::vector<ptrdiff_t>& offsets)
190190
}
191191

192192
offsets.resize(std::max(_module_index + 1, offsets.size()));
193-
auto offset = - get_tls_size();
193+
auto offset = - get_aligned_tls_size();
194194
offsets[_module_index] = offset;
195195
}
196196

arch/x64/arch-switch.hh

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,13 +187,15 @@ void thread::setup_tcb()
187187
void* user_tls_data;
188188
size_t user_tls_size = 0;
189189
size_t executable_tls_size = 0;
190+
size_t aligned_executable_tls_size = 0;
190191
if (_app_runtime) {
191192
auto obj = _app_runtime->app.lib();
192193
assert(obj);
193194
user_tls_size = obj->initial_tls_size();
194195
user_tls_data = obj->initial_tls();
195196
if (obj->is_executable()) {
196197
executable_tls_size = obj->get_tls_size();
198+
aligned_executable_tls_size = obj->get_aligned_tls_size();
197199
}
198200
}
199201

@@ -222,7 +224,7 @@ void thread::setup_tcb()
222224
// If executable copy its TLS block data at the designated offset
223225
// at the end of area as described in the ascii art for executables
224226
// TLS layout
225-
auto executable_tls_offset = total_tls_size - executable_tls_size;
227+
auto executable_tls_offset = total_tls_size - aligned_executable_tls_size;
226228
_app_runtime->app.lib()->copy_local_tls(p + executable_tls_offset);
227229
}
228230
_tcb = static_cast<thread_control_block*>(p + total_tls_size);

core/elf.cc

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,7 @@ void object::load_segments()
461461
_tls_segment = _base + phdr.p_vaddr;
462462
_tls_init_size = phdr.p_filesz;
463463
_tls_uninit_size = phdr.p_memsz - phdr.p_filesz;
464+
_tls_alignment = phdr.p_align;
464465
break;
465466
default:
466467
abort("Unknown p_type in executable %s: %d\n", pathname(), phdr.p_type);
@@ -469,13 +470,11 @@ void object::load_segments()
469470
if (!is_core() && _ehdr.e_type == ET_EXEC && !_is_executable) {
470471
abort("Statically linked executables are not supported!\n");
471472
}
472-
// As explained in issue #352, we currently don't correctly support TLS
473-
// used in PIEs.
474473
if (_is_executable && _tls_segment) {
475-
auto tls_size = _tls_init_size + _tls_uninit_size;
474+
auto needed_tls_size = get_aligned_tls_size();
476475
ulong pie_static_tls_maximum_size = &_pie_static_tls_end - &_pie_static_tls_start;
477-
if (tls_size > pie_static_tls_maximum_size) {
478-
std::cout << "WARNING: " << pathname() << " is a PIE using TLS of size " << tls_size
476+
if (needed_tls_size > pie_static_tls_maximum_size) {
477+
std::cout << "WARNING: " << pathname() << " is a PIE using TLS of size " << needed_tls_size
479478
<< " which is greater than " << pie_static_tls_maximum_size << " bytes limit. "
480479
<< "Either increase the size of TLS reserve in arch/x64/loader.ld or "
481480
<< "link with '-shared' instead of '-pie'.\n";
@@ -1003,6 +1002,11 @@ ulong object::get_tls_size()
10031002
return _tls_init_size + _tls_uninit_size;
10041003
}
10051004

1005+
ulong object::get_aligned_tls_size()
1006+
{
1007+
return align_up(_tls_init_size + _tls_uninit_size, _tls_alignment);
1008+
}
1009+
10061010
void object::collect_dependencies(std::unordered_set<elf::object*>& ds)
10071011
{
10081012
ds.insert(this);

include/osv/elf.hh

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,7 @@ public:
371371
std::vector<ptrdiff_t>& initial_tls_offsets() { return _initial_tls_offsets; }
372372
bool is_executable() { return _is_executable; }
373373
ulong get_tls_size();
374+
ulong get_aligned_tls_size();
374375
void copy_local_tls(void* to_addr);
375376
protected:
376377
virtual void load_segment(const Elf64_Phdr& segment) = 0;
@@ -409,7 +410,7 @@ protected:
409410
void* _base;
410411
void* _end;
411412
void* _tls_segment;
412-
ulong _tls_init_size, _tls_uninit_size;
413+
ulong _tls_init_size, _tls_uninit_size, _tls_alignment;
413414
bool _static_tls;
414415
ptrdiff_t _static_tls_offset;
415416
static std::atomic<ptrdiff_t> _static_tls_alloc;

tests/tst-tls.cc

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,10 @@ extern __thread int ex3 __attribute__ ((tls_model ("initial-exec")));
4242
// builds all tests as shared objects (.so), and the linker will report an
4343
// error, because local-exec is not allowed in shared libraries, just in
4444
// executables (including PIE).
45-
__thread int v7 __attribute__ ((tls_model ("local-exec"))) = 789;
46-
__thread int v8 __attribute__ ((tls_model ("local-exec")));
45+
__thread long v7 __attribute__ ((tls_model ("local-exec"))) = 987UL;
46+
__thread int v8 __attribute__ ((tls_model ("local-exec"))) = 789;
47+
__thread int v9 __attribute__ ((tls_model ("local-exec")));
48+
__thread int v10 __attribute__ ((tls_model ("local-exec"))) = 1111;
4749
#endif
4850

4951
extern void external_library();
@@ -67,7 +69,9 @@ int main(int argc, char** argv)
6769
report(v6 == 678, "v6");
6870
report(ex3 == 765, "ex3");
6971
#ifndef __SHARED_OBJECT__
70-
report(v7 == 789, "v7");
72+
report(v7 == 987UL, "v7");
73+
report(v8 == 789, "v8");
74+
report(v10 == 1111, "v10");
7175
#endif
7276

7377
external_library();
@@ -101,7 +105,7 @@ int main(int argc, char** argv)
101105
report(v6 == 678, "v6 in new thread");
102106
report(ex3 == 765, "ex3 in new thread");
103107
#ifndef __SHARED_OBJECT__
104-
report(v7 == 789, "v7 in new thread");
108+
report(v7 == 987UL, "v7 in new thread");
105109
#endif
106110

107111
external_library();
@@ -120,8 +124,8 @@ int main(int argc, char** argv)
120124
static void before_main(void) __attribute__((constructor));
121125
static void before_main(void)
122126
{
123-
report(v7 == 789, "v7 in init function");
124-
report(v8 == 0, "v8 in init function");
127+
report(v7 == 987UL, "v7 in init function");
128+
report(v9 == 0, "v8 in init function");
125129
}
126130
#endif
127131

0 commit comments

Comments
 (0)