commit 7e2d49ae3ebbae7ebf1ca033f86e87002c0dd83c
author Dimitry Ivanov <dimitry@google.com> Mon Aug 08 17:12:18 2016 -0700
committer Dimitry Ivanov <dimitry@google.com> Tue Aug 09 11:07:33 2016 -0700
tree 5ce154019a9faa30fcd1ed96a95ef5950e052b0f
parent 8bbcc61b54cf8ce95c4bbae7db7883442b7d1fd9

Check alignment along with range of mapped file fragments

Improve sanity-checks of elf-file by adding alignment check
of mapped sections and section headers.

Bug: http://b/30687964
Change-Id: I7f06ddaa56a13989ce7be847b3a73c352e32c008

linker/linker.h

diff --git a/linker/linker.h b/linker/linker.h
index 3ea601f..49eee78 100644
--- a/linker/linker.h
+++ b/linker/linker.h
@@ -59,6 +59,12 @@
       __libc_format_fd(2, "\n"); \
     } while (false)
 
+#define DL_ERR_AND_LOG(fmt, x...) \
+  do { \
+    DL_ERR(fmt, x); \
+    PRINT(fmt, x); \
+  } while (false)
+
 #if defined(__LP64__)
 #define ELFW(what) ELF64_ ## what
 #else

linker/linker_phdr.cpp

diff --git a/linker/linker_phdr.cpp b/linker/linker_phdr.cpp
index 136e432..3b60460 100644
--- a/linker/linker_phdr.cpp
+++ b/linker/linker_phdr.cpp
@@ -248,14 +248,15 @@
   return true;
 }
 
-bool ElfReader::CheckFileRange(ElfW(Addr) offset, size_t size) {
+bool ElfReader::CheckFileRange(ElfW(Addr) offset, size_t size, size_t alignment) {
   off64_t range_start;
   off64_t range_end;
 
   return safe_add(&range_start, file_offset_, offset) &&
          safe_add(&range_end, range_start, size) &&
-         range_start < file_size_ &&
-         range_end <= file_size_;
+         (range_start < file_size_) &&
+         (range_end <= file_size_) &&
+         ((offset % alignment) == 0);
 }
 
 // Loads the program header table from an ELF file into a read-only private
@@ -272,8 +273,11 @@
 
   // Boundary checks
   size_t size = phdr_num_ * sizeof(ElfW(Phdr));
-  if (!CheckFileRange(header_.e_phoff, size)) {
-    DL_ERR("\"%s\" has invalid phdr offset/size", name_.c_str());
+  if (!CheckFileRange(header_.e_phoff, size, alignof(ElfW(Phdr)))) {
+    DL_ERR_AND_LOG("\"%s\" has invalid phdr offset/size: %zu/%zu",
+                   name_.c_str(),
+                   static_cast<size_t>(header_.e_phoff),
+                   size);
     return false;
   }
 
@@ -290,13 +294,16 @@
   shdr_num_ = header_.e_shnum;
 
   if (shdr_num_ == 0) {
-    DL_ERR("\"%s\" has no section headers", name_.c_str());
+    DL_ERR_AND_LOG("\"%s\" has no section headers", name_.c_str());
     return false;
   }
 
   size_t size = shdr_num_ * sizeof(ElfW(Shdr));
-  if (!CheckFileRange(header_.e_shoff, size)) {
-    DL_ERR("\"%s\" has invalid shdr offset/size", name_.c_str());
+  if (!CheckFileRange(header_.e_shoff, size, alignof(const ElfW(Shdr)))) {
+    DL_ERR_AND_LOG("\"%s\" has invalid shdr offset/size: %zu/%zu",
+                   name_.c_str(),
+                   static_cast<size_t>(header_.e_shoff),
+                   size);
     return false;
   }
 
@@ -320,26 +327,27 @@
   }
 
   if (dynamic_shdr == nullptr) {
-    DL_ERR("\"%s\" .dynamic section header was not found", name_.c_str());
+    DL_ERR_AND_LOG("\"%s\" .dynamic section header was not found", name_.c_str());
     return false;
   }
 
   if (dynamic_shdr->sh_link >= shdr_num_) {
-    DL_ERR("\"%s\" .dynamic section has invalid sh_link: %d", name_.c_str(), dynamic_shdr->sh_link);
+    DL_ERR_AND_LOG("\"%s\" .dynamic section has invalid sh_link: %d",
+                   name_.c_str(),
+                   dynamic_shdr->sh_link);
     return false;
   }
 
   const ElfW(Shdr)* strtab_shdr = &shdr_table_[dynamic_shdr->sh_link];
 
   if (strtab_shdr->sh_type != SHT_STRTAB) {
-    DL_ERR("\"%s\" .dynamic section has invalid link(%d) sh_type: %d (expected SHT_STRTAB)",
-           name_.c_str(), dynamic_shdr->sh_link, strtab_shdr->sh_type);
+    DL_ERR_AND_LOG("\"%s\" .dynamic section has invalid link(%d) sh_type: %d (expected SHT_STRTAB)",
+                   name_.c_str(), dynamic_shdr->sh_link, strtab_shdr->sh_type);
     return false;
   }
 
-  if (!CheckFileRange(dynamic_shdr->sh_offset, dynamic_shdr->sh_size)) {
-    DL_ERR("\"%s\" has invalid offset/size of .dynamic section", name_.c_str());
-    PRINT("\"%s\" has invalid offset/size of .dynamic section", name_.c_str());
+  if (!CheckFileRange(dynamic_shdr->sh_offset, dynamic_shdr->sh_size, alignof(const ElfW(Dyn)))) {
+    DL_ERR_AND_LOG("\"%s\" has invalid offset/size of .dynamic section", name_.c_str());
     return false;
   }
 
@@ -350,9 +358,9 @@
 
   dynamic_ = static_cast<const ElfW(Dyn)*>(dynamic_fragment_.data());
 
-  if (!CheckFileRange(strtab_shdr->sh_offset, strtab_shdr->sh_size)) {
-    DL_ERR("\"%s\" has invalid offset/size of the .strtab section linked from .dynamic section",
-           name_.c_str());
+  if (!CheckFileRange(strtab_shdr->sh_offset, strtab_shdr->sh_size, alignof(const char))) {
+    DL_ERR_AND_LOG("\"%s\" has invalid offset/size of the .strtab section linked from .dynamic section",
+                   name_.c_str());
     return false;
   }
 

linker/linker_phdr.h

diff --git a/linker/linker_phdr.h b/linker/linker_phdr.h
index 89ec094..d6276ed 100644
--- a/linker/linker_phdr.h
+++ b/linker/linker_phdr.h
@@ -65,7 +65,7 @@
   bool LoadSegments();
   bool FindPhdr();
   bool CheckPhdr(ElfW(Addr));
-  bool CheckFileRange(ElfW(Addr) offset, size_t size);
+  bool CheckFileRange(ElfW(Addr) offset, size_t size, size_t alignment);
 
   bool did_read_;
   bool did_load_;