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
diff --git a/patchoat/patchoat.cc b/patchoat/patchoat.cc
index d9cefee..4900f17 100644
--- a/patchoat/patchoat.cc
+++ b/patchoat/patchoat.cc
@@ -357,6 +357,65 @@
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 @@
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 @@
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;
+ 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 @@
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 @@
// 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 @@
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 @@
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) {}