Skip to content

Commit

Permalink
tls: make local-exec tls aligned
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
wkozaczuk committed Nov 7, 2019

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
1 parent cccc7d4 commit b0478df
Showing 5 changed files with 27 additions and 16 deletions.
6 changes: 3 additions & 3 deletions arch/x64/arch-elf.cc
Original file line number Diff line number Diff line change
@@ -128,11 +128,11 @@ bool object::arch_relocate_rela(u32 type, u32 sym, void *addr,
auto sm = symbol(sym);
ulong tls_offset;
if (sm.obj->is_executable()) {
tls_offset = sm.obj->get_tls_size();
// If this is an executable (pie or position-dependant one)
// then the variable is located in the reserved slot of the TLS
// right where the kernel TLS lives
// So the offset is negative size of this ELF TLS block
// So the offset is negative aligned size of this ELF TLS block
tls_offset = sm.obj->get_aligned_tls_size();
} else {
// If shared library, the variable is located in one of TLS
// 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)
}

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

4 changes: 3 additions & 1 deletion arch/x64/arch-switch.hh
Original file line number Diff line number Diff line change
@@ -187,13 +187,15 @@ void thread::setup_tcb()
void* user_tls_data;
size_t user_tls_size = 0;
size_t executable_tls_size = 0;
size_t aligned_executable_tls_size = 0;
if (_app_runtime) {
auto obj = _app_runtime->app.lib();
assert(obj);
user_tls_size = obj->initial_tls_size();
user_tls_data = obj->initial_tls();
if (obj->is_executable()) {
executable_tls_size = obj->get_tls_size();
aligned_executable_tls_size = obj->get_aligned_tls_size();
}
}

@@ -222,7 +224,7 @@ void thread::setup_tcb()
// If executable copy its TLS block data at the designated offset
// at the end of area as described in the ascii art for executables
// TLS layout
auto executable_tls_offset = total_tls_size - executable_tls_size;
auto executable_tls_offset = total_tls_size - aligned_executable_tls_size;
_app_runtime->app.lib()->copy_local_tls(p + executable_tls_offset);
}
_tcb = static_cast<thread_control_block*>(p + total_tls_size);
14 changes: 9 additions & 5 deletions core/elf.cc
Original file line number Diff line number Diff line change
@@ -461,6 +461,7 @@ void object::load_segments()
_tls_segment = _base + phdr.p_vaddr;
_tls_init_size = phdr.p_filesz;
_tls_uninit_size = phdr.p_memsz - phdr.p_filesz;
_tls_alignment = phdr.p_align;
break;
default:
abort("Unknown p_type in executable %s: %d\n", pathname(), phdr.p_type);
@@ -469,13 +470,11 @@ void object::load_segments()
if (!is_core() && _ehdr.e_type == ET_EXEC && !_is_executable) {
abort("Statically linked executables are not supported!\n");
}
// As explained in issue #352, we currently don't correctly support TLS
// used in PIEs.
if (_is_executable && _tls_segment) {
auto tls_size = _tls_init_size + _tls_uninit_size;
auto needed_tls_size = get_aligned_tls_size();
ulong pie_static_tls_maximum_size = &_pie_static_tls_end - &_pie_static_tls_start;
if (tls_size > pie_static_tls_maximum_size) {
std::cout << "WARNING: " << pathname() << " is a PIE using TLS of size " << tls_size
if (needed_tls_size > pie_static_tls_maximum_size) {
std::cout << "WARNING: " << pathname() << " is a PIE using TLS of size " << needed_tls_size
<< " which is greater than " << pie_static_tls_maximum_size << " bytes limit. "
<< "Either increase the size of TLS reserve in arch/x64/loader.ld or "
<< "link with '-shared' instead of '-pie'.\n";
@@ -1003,6 +1002,11 @@ ulong object::get_tls_size()
return _tls_init_size + _tls_uninit_size;
}

ulong object::get_aligned_tls_size()
{
return align_up(_tls_init_size + _tls_uninit_size, _tls_alignment);
}

void object::collect_dependencies(std::unordered_set<elf::object*>& ds)
{
ds.insert(this);
3 changes: 2 additions & 1 deletion include/osv/elf.hh
Original file line number Diff line number Diff line change
@@ -371,6 +371,7 @@ public:
std::vector<ptrdiff_t>& initial_tls_offsets() { return _initial_tls_offsets; }
bool is_executable() { return _is_executable; }
ulong get_tls_size();
ulong get_aligned_tls_size();
void copy_local_tls(void* to_addr);
protected:
virtual void load_segment(const Elf64_Phdr& segment) = 0;
@@ -409,7 +410,7 @@ protected:
void* _base;
void* _end;
void* _tls_segment;
ulong _tls_init_size, _tls_uninit_size;
ulong _tls_init_size, _tls_uninit_size, _tls_alignment;
bool _static_tls;
ptrdiff_t _static_tls_offset;
static std::atomic<ptrdiff_t> _static_tls_alloc;
16 changes: 10 additions & 6 deletions tests/tst-tls.cc
Original file line number Diff line number Diff line change
@@ -42,8 +42,10 @@ extern __thread int ex3 __attribute__ ((tls_model ("initial-exec")));
// builds all tests as shared objects (.so), and the linker will report an
// error, because local-exec is not allowed in shared libraries, just in
// executables (including PIE).
__thread int v7 __attribute__ ((tls_model ("local-exec"))) = 789;
__thread int v8 __attribute__ ((tls_model ("local-exec")));
__thread long v7 __attribute__ ((tls_model ("local-exec"))) = 987UL;
__thread int v8 __attribute__ ((tls_model ("local-exec"))) = 789;
__thread int v9 __attribute__ ((tls_model ("local-exec")));
__thread int v10 __attribute__ ((tls_model ("local-exec"))) = 1111;
#endif

extern void external_library();
@@ -67,7 +69,9 @@ int main(int argc, char** argv)
report(v6 == 678, "v6");
report(ex3 == 765, "ex3");
#ifndef __SHARED_OBJECT__
report(v7 == 789, "v7");
report(v7 == 987UL, "v7");
report(v8 == 789, "v8");
report(v10 == 1111, "v10");
#endif

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

external_library();
@@ -120,8 +124,8 @@ int main(int argc, char** argv)
static void before_main(void) __attribute__((constructor));
static void before_main(void)
{
report(v7 == 789, "v7 in init function");
report(v8 == 0, "v8 in init function");
report(v7 == 987UL, "v7 in init function");
report(v9 == 0, "v8 in init function");
}
#endif

0 comments on commit b0478df

Please sign in to comment.