Hi. Looks good. Just one question about the test - why is misc-non-fpic.so a shared object that another test runs, instead of just being an indepedent test of its own, tst-non-fpic.so (that we compile a bit differently from the other tests)?
On Thu, Oct 25, 2018 at 1:10 AM Waldemar Kozaczuk <[email protected]> wrote: > This patch enhances ELF dynamic loader to support position > dependent shared libraries. It does it by detecting presence > of DT_TEXTREL marker and temporarilily making PT_LOAD sections > writable so that corresponding references in code get updated > to point to correct addresses in memory. Eventually it fixes > permissions of PT_LOAD sections to make it non-writable. > > This patch most notably allows to run GraalVM generated > Java apps. > > Fixes #1004 > > Signed-off-by: Waldemar Kozaczuk <[email protected]> > --- > core/elf.cc | 47 +++++++++++++++++++++++++++++++++++------- > include/osv/elf.hh | 5 ++++- > modules/tests/Makefile | 9 +++++++- > tests/misc-non-fpic.cc | 13 ++++++++++++ > tests/tst-run.cc | 3 +++ > 5 files changed, 68 insertions(+), 9 deletions(-) > create mode 100644 tests/misc-non-fpic.cc > > diff --git a/core/elf.cc b/core/elf.cc > index b0e3c1aa..c0dfbbb9 100644 > --- a/core/elf.cc > +++ b/core/elf.cc > @@ -325,13 +325,7 @@ void file::load_segment(const Elf64_Phdr& phdr) > ulong filesz = align_up(filesz_unaligned, mmu::page_size); > ulong memsz = align_up(phdr.p_vaddr + phdr.p_memsz, mmu::page_size) - > vstart; > > - unsigned perm = 0; > - if (phdr.p_flags & PF_X) > - perm |= mmu::perm_exec; > - if (phdr.p_flags & PF_W) > - perm |= mmu::perm_write; > - if (phdr.p_flags & PF_R) > - perm |= mmu::perm_read; > + unsigned perm = get_segment_mmap_permissions(phdr); > > auto flag = mmu::mmap_fixed | (mlocked() ? mmu::mmap_populate : 0); > mmu::map_file(_base + vstart, filesz, flag, perm, _f, > align_down(phdr.p_offset, mmu::page_size)); > @@ -354,6 +348,11 @@ bool object::mlocked() > return false; > } > > +bool object::has_non_writable_text_relocations() > +{ > + return dynamic_exists(DT_TEXTREL); > +} > + > Elf64_Note::Elf64_Note(void *_base, char *str) > { > Elf64_Word *base = reinterpret_cast<Elf64_Word *>(_base); > @@ -468,8 +467,24 @@ void object::unload_segments() > } > } > > +unsigned object::get_segment_mmap_permissions(const Elf64_Phdr& phdr) > +{ > + unsigned perm = 0; > + if (phdr.p_flags & PF_X) > + perm |= mmu::perm_exec; > + if (phdr.p_flags & PF_W) > + perm |= mmu::perm_write; > + if (phdr.p_flags & PF_R) > + perm |= mmu::perm_read; > + return perm; > +} > + > void object::fix_permissions() > { > + if(has_non_writable_text_relocations()) { > + make_text_writable(false); > + } > + > for (auto&& phdr : _phdrs) { > if (phdr.p_type != PT_GNU_RELRO) > continue; > @@ -482,6 +497,20 @@ void object::fix_permissions() > } > } > > +void object::make_text_writable(bool flag) > +{ > + for (auto&& phdr : _phdrs) { > + if (phdr.p_type != PT_LOAD) > + continue; > + > + ulong vstart = align_down(phdr.p_vaddr, mmu::page_size); > + ulong memsz = align_up(phdr.p_vaddr + phdr.p_memsz, > mmu::page_size) - vstart; > + > + unsigned perm = get_segment_mmap_permissions(phdr); > + mmu::mprotect(_base + vstart, memsz, flag ? perm | > mmu::perm_write : perm); > + } > +} > + > template <typename T> > T* object::dynamic_ptr(unsigned tag) > { > @@ -600,6 +629,10 @@ symbol_module object::symbol_other(unsigned idx) > > void object::relocate_rela() > { > + if(has_non_writable_text_relocations()) { > + make_text_writable(true); > + } > + > auto rela = dynamic_ptr<Elf64_Rela>(DT_RELA); > assert(dynamic_val(DT_RELAENT) == sizeof(Elf64_Rela)); > unsigned nb = dynamic_val(DT_RELASZ) / sizeof(Elf64_Rela); > diff --git a/include/osv/elf.hh b/include/osv/elf.hh > index 5449f98f..19b24eec 100644 > --- a/include/osv/elf.hh > +++ b/include/osv/elf.hh > @@ -177,7 +177,7 @@ enum { > DT_PLTREL = 20, // d_val Type of relocation entry used for the > procedure linkage > // table. The d_val member contains either DT_REL or DT_RELA. > DT_DEBUG = 21, // d_ptr Reserved for debugger use. > - DT_TEXTREL = 22, // ignored The presence of this dynamic table entry > signals that the > + DT_TEXTREL = 22, // The presence of this dynamic table entry signals > that the > // relocation table contains relocations for a non-writable > // segment. > DT_JMPREL = 23, // d_ptr Address of the relocations associated with > the procedure > @@ -367,6 +367,8 @@ protected: > virtual void unload_segment(const Elf64_Phdr& segment) = 0; > virtual void read(Elf64_Off offset, void* data, size_t len) = 0; > bool mlocked(); > + bool has_non_writable_text_relocations(); > + unsigned get_segment_mmap_permissions(const Elf64_Phdr& phdr); > private: > Elf64_Sym* lookup_symbol_old(const char* name); > Elf64_Sym* lookup_symbol_gnu(const char* name); > @@ -388,6 +390,7 @@ private: > void collect_dependencies(std::unordered_set<elf::object*>& ds); > void prepare_initial_tls(void* buffer, size_t size, > std::vector<ptrdiff_t>& offsets); > void alloc_static_tls(); > + void make_text_writable(bool flag); > protected: > program& _prog; > std::string _pathname; > diff --git a/modules/tests/Makefile b/modules/tests/Makefile > index cec4febe..0fed7a3b 100644 > --- a/modules/tests/Makefile > +++ b/modules/tests/Makefile > @@ -60,6 +60,13 @@ $(out)/tests/rofs/%.o: $(src)/tests/%.c > $(out)/%.so: $(out)/%.o > $(call quiet, $(CXX) $(CXXFLAGS) -shared -o $@ $< $(LIBS), LD > $*.so) > > +NON_FPIC_CXXFLAGS = $(autodepend) $(INCLUDES) -g -O2 -mcmodel=large > -fno-pie -DBOOST_TEST_DYN_LINK \ > + -U _FORTIFY_SOURCE -D_KERNEL -D__OSV__ -DCONF_debug_memory=0 \ > + -Wall -Wno-pointer-arith -Wformat=0 -Wno-format-security > +$(out)/tests/misc-non-fpic.so: $(src)/tests/misc-non-fpic.cc > + $(makedir) > + $(call quiet, $(CXX) -shared $(NON_FPIC_CXXFLAGS) -o $@ $<, LD > $*.so) > + > # The rofs test image mounts /tmp as ramfs and 4 tests that exercise file > system > # fail due to some unresolved bugs or other shortcomings of the ramfs > implementation > # and are temporarily removed from the rofs-only-tests list. The tests > tst-readdir.so > @@ -93,7 +100,7 @@ tests := tst-pthread.so misc-ramdisk.so tst-vblk.so > tst-bsd-evh.so \ > misc-tcp-sendonly.so tst-tcp-nbwrite.so misc-tcp-hash-srv.so \ > misc-loadbalance.so misc-scheduler.so tst-console.so tst-app.so \ > misc-setpriority.so misc-timeslice.so misc-tls.so misc-gtod.so \ > - tst-dns-resolver.so tst-kill.so tst-truncate.so \ > + tst-dns-resolver.so tst-kill.so tst-truncate.so misc-non-fpic.so \ > misc-panic.so tst-utimes.so tst-utimensat.so tst-futimesat.so \ > misc-tcp.so tst-strerror_r.so misc-random.so misc-urandom.so \ > tst-commands.so tst-threadcomplete.so tst-timerfd.so \ > diff --git a/tests/misc-non-fpic.cc b/tests/misc-non-fpic.cc > new file mode 100644 > index 00000000..77cf17a5 > --- /dev/null > +++ b/tests/misc-non-fpic.cc > @@ -0,0 +1,13 @@ > +/* > + * Copyright (C) 2018 Waldemar Kozaczuk > + * > + * This work is open source software, licensed under the terms of the > + * BSD license as described in the LICENSE file in the top-level > directory. > + */ > +#include <stdio.h> > +#include <assert.h> > + > +int main() { > + printf("Hello\n"); > + assert(1); > +} > diff --git a/tests/tst-run.cc b/tests/tst-run.cc > index 3f1139c3..2d3d6317 100644 > --- a/tests/tst-run.cc > +++ b/tests/tst-run.cc > @@ -48,6 +48,9 @@ int main(int ac, char** av) > report(true, "Run nonexistant"); > } > > + b = (bool)osv::run("/tests/misc-non-fpic.so", 0, nullptr, nullptr); > + report(b == true, "Run non-PIC with non-writable relocations"); > + > return 0; > } > > -- > 2.17.1 > > -- > You received this message because you are subscribed to the Google Groups > "OSv Development" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > For more options, visit https://groups.google.com/d/optout. > -- You received this message because you are subscribed to the Google Groups "OSv Development" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
