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.

>
>
> 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.

Reply via email to