On Mon, Oct 29, 2018 at 2:28 PM Waldek Kozaczuk <[email protected]> wrote:
> > On Mon, Oct 29, 2018 at 07:23 Nadav Har'El <[email protected]> wrote: > >> 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)? >> > Because it better resembles how graalvm-Example is run plus I guess it was > less code. If you insist I will change it into separate tst. > Yes, I think this would be better. I don't want to mix two completely unrelated tests. I also don't understand how it's less code - why is it simpler or less code to run "misc-non-fpic.so" from "tst-run.so" instead of just running it directly? By the way, what's the point with assert(1)? I think it should be "return 0". >> >> 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.
