Have patchoat --verify verify symlinks

patchoat --verify now verifies that the vdex and oat symlinks in
/data/dalvik-cache point to the correct vdex and oat files in /system/.
This is to protect attacks that would change the symlink targets.
Refactoring of patchoat code was done too.

Bug: 66697305
Test: make test-art-host-gtest-patchoat_test
Test: remove a symlink and ensure patchoat verification fails
Test: modify a symlink and ensure patchoat verification fails
Change-Id: I18e8f9f6363cf18ad8fa879aeb4d8c7badf679a7

2 files changed, 98 insertions, 118 deletions

diff --git a/patchoat/patchoat.cc b/patchoat/patchoat.cc
index d9cefee611..4900f178b9 100644
--- a/patchoat/patchoat.cc
+++ b/patchoat/patchoat.cc
@@ -357,6 +357,65 @@ static bool CheckImageIdenticalToOriginalExceptForRelocation(
   return true;
 }
 
+static bool VerifySymlink(const std::string& intended_target, const std::string& link_name) {
+  std::string actual_target;
+  if (!android::base::Readlink(link_name, &actual_target)) {
+    PLOG(ERROR) << "Readlink on " << link_name << " failed.";
+    return false;
+  }
+  return actual_target == intended_target;
+}
+
+static bool VerifyVdexAndOatSymlinks(const std::string& input_image_filename,
+                                     const std::string& output_image_filename) {
+  return VerifySymlink(ImageHeader::GetVdexLocationFromImageLocation(input_image_filename),
+                       ImageHeader::GetVdexLocationFromImageLocation(output_image_filename))
+      && VerifySymlink(ImageHeader::GetOatLocationFromImageLocation(input_image_filename),
+                       ImageHeader::GetOatLocationFromImageLocation(output_image_filename));
+}
+
+bool PatchOat::CreateVdexAndOatSymlinks(const std::string& input_image_filename,
+                                        const std::string& output_image_filename) {
+  std::string input_vdex_filename =
+      ImageHeader::GetVdexLocationFromImageLocation(input_image_filename);
+  std::string input_oat_filename =
+      ImageHeader::GetOatLocationFromImageLocation(input_image_filename);
+
+  std::unique_ptr<File> input_oat_file(OS::OpenFileForReading(input_oat_filename.c_str()));
+  if (input_oat_file.get() == nullptr) {
+    LOG(ERROR) << "Unable to open input oat file at " << input_oat_filename;
+    return false;
+  }
+  std::string error_msg;
+  std::unique_ptr<ElfFile> elf(ElfFile::Open(input_oat_file.get(),
+                                             PROT_READ | PROT_WRITE,
+                                             MAP_PRIVATE,
+                                             &error_msg));
+  if (elf.get() == nullptr) {
+    LOG(ERROR) << "Unable to open oat file " << input_oat_filename << " : " << error_msg;
+    return false;
+  }
+
+  MaybePic is_oat_pic = IsOatPic(elf.get());
+  if (is_oat_pic >= ERROR_FIRST) {
+    // Error logged by IsOatPic
+    return false;
+  } else if (is_oat_pic == NOT_PIC) {
+    LOG(ERROR) << "patchoat cannot be used on non-PIC oat file: " << input_oat_filename;
+    return false;
+  }
+
+  CHECK(is_oat_pic == PIC);
+
+  std::string output_vdex_filename =
+      ImageHeader::GetVdexLocationFromImageLocation(output_image_filename);
+  std::string output_oat_filename =
+      ImageHeader::GetOatLocationFromImageLocation(output_image_filename);
+
+  return SymlinkFile(input_oat_filename, output_oat_filename) &&
+         SymlinkFile(input_vdex_filename, output_vdex_filename);
+}
+
 bool PatchOat::Patch(const std::string& image_location,
                      off_t delta,
                      const std::string& output_image_directory,
@@ -401,13 +460,11 @@ bool PatchOat::Patch(const std::string& image_location,
   Thread::Current()->TransitionFromRunnableToSuspended(kNative);
   ScopedObjectAccess soa(Thread::Current());
 
-  t.NewTiming("Image Patching setup");
   std::vector<gc::space::ImageSpace*> spaces = Runtime::Current()->GetHeap()->GetBootImageSpaces();
-  std::map<gc::space::ImageSpace*, std::unique_ptr<File>> space_to_file_map;
   std::map<gc::space::ImageSpace*, std::unique_ptr<MemMap>> space_to_memmap_map;
-  std::map<gc::space::ImageSpace*, PatchOat> space_to_patchoat_map;
 
   for (size_t i = 0; i < spaces.size(); ++i) {
+    t.NewTiming("Image Patching setup");
     gc::space::ImageSpace* space = spaces[i];
     std::string input_image_filename = space->GetImageFilename();
     std::unique_ptr<File> input_image(OS::OpenFileForReading(input_image_filename.c_str()));
@@ -445,139 +502,76 @@ bool PatchOat::Patch(const std::string& image_location,
       LOG(ERROR) << "Unable to map image file " << input_image->GetPath() << " : " << error_msg;
       return false;
     }
-    space_to_file_map.emplace(space, std::move(input_image));
-    space_to_memmap_map.emplace(space, std::move(image));
-  }
 
-  // Symlink PIC oat and vdex files and patch the image spaces in memory.
-  for (size_t i = 0; i < spaces.size(); ++i) {
-    gc::space::ImageSpace* space = spaces[i];
-    std::string input_image_filename = space->GetImageFilename();
-    std::string input_vdex_filename =
-        ImageHeader::GetVdexLocationFromImageLocation(input_image_filename);
-    std::string input_oat_filename =
-        ImageHeader::GetOatLocationFromImageLocation(input_image_filename);
-    std::unique_ptr<File> input_oat_file(OS::OpenFileForReading(input_oat_filename.c_str()));
-    if (input_oat_file.get() == nullptr) {
-      LOG(ERROR) << "Unable to open input oat file at " << input_oat_filename;
-      return false;
-    }
-    std::string error_msg;
-    std::unique_ptr<ElfFile> elf(ElfFile::Open(input_oat_file.get(),
-                                               PROT_READ | PROT_WRITE, MAP_PRIVATE, &error_msg));
-    if (elf.get() == nullptr) {
-      LOG(ERROR) << "Unable to open oat file " << input_oat_file->GetPath() << " : " << error_msg;
-      return false;
-    }
 
-    if (output_image) {
-      MaybePic is_oat_pic = IsOatPic(elf.get());
-      if (is_oat_pic >= ERROR_FIRST) {
-        // Error logged by IsOatPic
-        return false;
-      } else if (is_oat_pic == NOT_PIC) {
-        LOG(ERROR) << "patchoat cannot be used on non-PIC oat file: " << input_oat_file->GetPath();
-        return false;
-      } else {
-        CHECK(is_oat_pic == PIC);
-
-        // Create a symlink.
-        std::string converted_image_filename = space->GetImageLocation();
-        std::replace(
-            converted_image_filename.begin() + 1, converted_image_filename.end(), '/', '@');
-        std::string output_image_filename = output_image_directory +
-            (android::base::StartsWith(converted_image_filename, "/") ? "" : "/") +
-            converted_image_filename;
-        std::string output_vdex_filename =
-            ImageHeader::GetVdexLocationFromImageLocation(output_image_filename);
-        std::string output_oat_filename =
-            ImageHeader::GetOatLocationFromImageLocation(output_image_filename);
-
-        if (!ReplaceOatFileWithSymlink(input_oat_file->GetPath(),
-                                       output_oat_filename) ||
-            !SymlinkFile(input_vdex_filename, output_vdex_filename)) {
-          // Errors already logged by above call.
-          return false;
-        }
-      }
-    }
-
-    PatchOat& p = space_to_patchoat_map.emplace(space,
-                                                PatchOat(
-                                                    isa,
-                                                    space_to_memmap_map.find(space)->second.get(),
-                                                    space->GetLiveBitmap(),
-                                                    space->GetMemMap(),
-                                                    delta,
-                                                    &space_to_memmap_map,
-                                                    timings)).first->second;
+    space_to_memmap_map.emplace(space, std::move(image));
+    PatchOat p = PatchOat(isa,
+                          space_to_memmap_map.at(space).get(),
+                          space->GetLiveBitmap(),
+                          space->GetMemMap(),
+                          delta,
+                          &space_to_memmap_map,
+                          timings);
 
     t.NewTiming("Patching image");
     if (!p.PatchImage(i == 0)) {
       LOG(ERROR) << "Failed to patch image file " << input_image_filename;
       return false;
     }
-  }
 
-  if (output_image) {
     // Write the patched image spaces.
-    for (size_t i = 0; i < spaces.size(); ++i) {
-      gc::space::ImageSpace* space = spaces[i];
+    if (output_image) {
+      std::string output_image_filename;
+      if (!GetDalvikCacheFilename(space->GetImageLocation().c_str(),
+                                  output_image_directory.c_str(),
+                                  &output_image_filename,
+                                  &error_msg)) {
+        LOG(ERROR) << "Failed to find relocated image file name: " << error_msg;
+        return false;
+      }
+
+      if (!CreateVdexAndOatSymlinks(input_image_filename, output_image_filename))
+        return false;
 
       t.NewTiming("Writing image");
-      std::string converted_image_filename = space->GetImageLocation();
-      std::replace(converted_image_filename.begin() + 1, converted_image_filename.end(), '/', '@');
-      std::string output_image_filename = output_image_directory +
-          (android::base::StartsWith(converted_image_filename, "/") ? "" : "/") +
-          converted_image_filename;
       std::unique_ptr<File> output_image_file(CreateOrOpen(output_image_filename.c_str()));
       if (output_image_file.get() == nullptr) {
         LOG(ERROR) << "Failed to open output image file at " << output_image_filename;
         return false;
       }
 
-      PatchOat& p = space_to_patchoat_map.find(space)->second;
-
       bool success = p.WriteImage(output_image_file.get());
       success = FinishFile(output_image_file.get(), success);
       if (!success) {
         return false;
       }
     }
-  }
-
-  if (output_image_relocation) {
-    // Write the image relocation information for each space.
-    for (size_t i = 0; i < spaces.size(); ++i) {
-      gc::space::ImageSpace* space = spaces[i];
 
+    if (output_image_relocation) {
       t.NewTiming("Writing image relocation");
       std::string original_image_filename(space->GetImageLocation() + ".rel");
       std::string image_relocation_filename =
           output_image_relocation_directory
               + (android::base::StartsWith(original_image_filename, "/") ? "" : "/")
               + original_image_filename.substr(original_image_filename.find_last_of("/"));
-      File& input_image = *space_to_file_map.find(space)->second;
-      int64_t input_image_size = input_image.GetLength();
+      int64_t input_image_size = input_image->GetLength();
       if (input_image_size < 0) {
         LOG(ERROR) << "Error while getting input image size";
         return false;
       }
-      std::string error_msg;
       std::unique_ptr<MemMap> original(MemMap::MapFile(input_image_size,
                                                        PROT_READ,
                                                        MAP_PRIVATE,
-                                                       input_image.Fd(),
+                                                       input_image->Fd(),
                                                        0,
                                                        /*low_4gb*/false,
-                                                       input_image.GetPath().c_str(),
+                                                       input_image->GetPath().c_str(),
                                                        &error_msg));
       if (original.get() == nullptr) {
-        LOG(ERROR) << "Unable to map image file " << input_image.GetPath() << " : " << error_msg;
+        LOG(ERROR) << "Unable to map image file " << input_image->GetPath() << " : " << error_msg;
         return false;
       }
 
-      PatchOat& p = space_to_patchoat_map.find(space)->second;
       const MemMap* relocated = p.image_;
 
       if (!WriteRelFile(*original, *relocated, image_relocation_filename, &error_msg)) {
@@ -643,11 +637,10 @@ bool PatchOat::Verify(const std::string& image_location,
   std::string image_location_dir = android::base::Dirname(image_location);
   for (size_t i = 0; i < spaces.size(); ++i) {
     gc::space::ImageSpace* space = spaces[i];
-    std::string image_filename = space->GetImageLocation();
 
     std::string relocated_image_filename;
     std::string error_msg;
-    if (!GetDalvikCacheFilename(image_filename.c_str(),
+    if (!GetDalvikCacheFilename(space->GetImageLocation().c_str(),
             output_image_directory.c_str(), &relocated_image_filename, &error_msg)) {
       LOG(ERROR) << "Failed to find relocated image file name: " << error_msg;
       success = false;
@@ -658,7 +651,8 @@ bool PatchOat::Verify(const std::string& image_location,
     // basename:     boot.art
     // original:     /system/framework/arm64/boot.art
     // relocation:   /system/framework/arm64/boot.art.rel
-    std::string original_image_filename = GetSystemImageFilename(image_filename.c_str(), isa);
+    std::string original_image_filename =
+        GetSystemImageFilename(space->GetImageLocation().c_str(), isa);
 
     if (!CheckImageIdenticalToOriginalExceptForRelocation(
             relocated_image_filename, original_image_filename, &error_msg)) {
@@ -666,6 +660,13 @@ bool PatchOat::Verify(const std::string& image_location,
       success = false;
       break;
     }
+
+    if (!VerifyVdexAndOatSymlinks(original_image_filename, relocated_image_filename)) {
+      LOG(ERROR) << "Verification of vdex and oat symlinks for "
+                 << space->GetImageLocation() << " failed.";
+      success = false;
+      break;
+    }
   }
 
   if (!kIsDebugBuild && !(RUNNING_ON_MEMORY_TOOL && kMemoryToolDetectsLeaks)) {
@@ -740,26 +741,6 @@ PatchOat::MaybePic PatchOat::IsOatPic(const ElfFile* oat_in) {
   return is_pic ? PIC : NOT_PIC;
 }
 
-bool PatchOat::ReplaceOatFileWithSymlink(const std::string& input_oat_filename,
-                                         const std::string& output_oat_filename) {
-  // Delete the original file, since we won't need it.
-  unlink(output_oat_filename.c_str());
-
-  // Create a symlink from the old oat to the new oat
-  if (symlink(input_oat_filename.c_str(), output_oat_filename.c_str()) < 0) {
-    int err = errno;
-    LOG(ERROR) << "Failed to create symlink at " << output_oat_filename
-               << " error(" << err << "): " << strerror(err);
-    return false;
-  }
-
-  if (kIsDebugBuild) {
-    LOG(INFO) << "Created symlink " << output_oat_filename << " -> " << input_oat_filename;
-  }
-
-  return true;
-}
-
 class PatchOat::PatchOatArtFieldVisitor : public ArtFieldVisitor {
  public:
   explicit PatchOatArtFieldVisitor(PatchOat* patch_oat) : patch_oat_(patch_oat) {}
diff --git a/patchoat/patchoat.h b/patchoat/patchoat.h
index ba59d570a7..feba523f56 100644
--- a/patchoat/patchoat.h
+++ b/patchoat/patchoat.h
@@ -91,10 +91,9 @@ class PatchOat {
   // Was the .oat image at oat_in made with --compile-pic ?
   static MaybePic IsOatPic(const ElfFile* oat_in);
 
-  // Attempt to replace the file with a symlink
-  // Returns false if it fails
-  static bool ReplaceOatFileWithSymlink(const std::string& input_oat_filename,
-                                        const std::string& output_oat_filename);
+  static bool CreateVdexAndOatSymlinks(const std::string& input_image_filename,
+                                       const std::string& output_image_filename);
+
 
   void VisitObject(mirror::Object* obj)
       REQUIRES_SHARED(Locks::mutator_lock_);