odrefresh: enable loading of system_server image files
Fix check in ValidateBootImageChecksum() to account for image files
with multiple components which exists for boot-framework when
compiling on device.
Update logic for only_load_system_executable to be
only_load_trusted_executable and treat /system and the ART APEX data
directory as trusted.
Add test to check .art,.odex,.vdex files derived from the
system_server classpath are mapped when the ART module updates.
Add test to check .art,.oat,.vdex files for the boot class path
extensions are present in the zygote processes.
Bug: 180949581
Test: atest com.android.tests.odsign.OnDeviceSigningHostTest#verifyGeneratedArtifactsLoaded
Change-Id: I3114fc6393402d8da2eb16ba756ab5fab713dc20
diff --git a/dexoptanalyzer/dexoptanalyzer.cc b/dexoptanalyzer/dexoptanalyzer.cc
index 5863e45..a6fcd02 100644
--- a/dexoptanalyzer/dexoptanalyzer.cc
+++ b/dexoptanalyzer/dexoptanalyzer.cc
@@ -316,7 +316,7 @@
isa_,
class_loader_context.get(),
/*load_executable=*/ false,
- /*only_load_system_executable=*/ false,
+ /*only_load_trusted_executable=*/ false,
vdex_fd_,
oat_fd_,
zip_fd_);
diff --git a/libartbase/base/file_utils.cc b/libartbase/base/file_utils.cc
index e39e49a..45c3e3e 100644
--- a/libartbase/base/file_utils.cc
+++ b/libartbase/base/file_utils.cc
@@ -612,18 +612,22 @@
return android::base::StartsWith(full_path, kApexDefaultPath);
}
-bool LocationIsOnSystem(const char* path) {
+bool LocationIsOnSystem(const std::string& location) {
#ifdef _WIN32
- UNUSED(path);
+ UNUSED(location);
LOG(FATAL) << "LocationIsOnSystem is unsupported on Windows.";
return false;
#else
- UniqueCPtr<const char[]> full_path(realpath(path, nullptr));
+ UniqueCPtr<const char[]> full_path(realpath(location.c_str(), nullptr));
return full_path != nullptr &&
android::base::StartsWith(full_path.get(), GetAndroidRoot().c_str());
#endif
}
+bool LocationIsTrusted(const std::string& location) {
+ return LocationIsOnSystem(location) || LocationIsOnArtApexData(location);
+}
+
bool ArtModuleRootDistinctFromAndroidRoot() {
std::string error_msg;
const char* android_root = GetAndroidDirSafe(kAndroidRootEnvVar,
diff --git a/libartbase/base/file_utils.h b/libartbase/base/file_utils.h
index ac611ad..6af82ef 100644
--- a/libartbase/base/file_utils.h
+++ b/libartbase/base/file_utils.h
@@ -138,7 +138,7 @@
bool LocationIsOnI18nModule(std::string_view location);
// Return whether the location is on system (i.e. android root).
-bool LocationIsOnSystem(const char* location);
+bool LocationIsOnSystem(const std::string& location);
// Return whether the location is on system/framework (i.e. $ANDROID_ROOT/framework).
bool LocationIsOnSystemFramework(std::string_view location);
@@ -149,6 +149,11 @@
// Return whether the location is on /apex/.
bool LocationIsOnApex(std::string_view location);
+// Returns whether the location is trusted for loading oat files. Trusted locations are protected
+// by dm-verity or fs-verity. The recognized locations are on /system or
+// /data/misc/apexdata/com.android.art.
+bool LocationIsTrusted(const std::string& location);
+
// Compare the ART module root against android root. Returns true if they are
// both known and distinct. This is meant to be a proxy for 'running with apex'.
bool ArtModuleRootDistinctFromAndroidRoot();
diff --git a/runtime/gc/space/image_space.cc b/runtime/gc/space/image_space.cc
index 5ee126c..750256d 100644
--- a/runtime/gc/space/image_space.cc
+++ b/runtime/gc/space/image_space.cc
@@ -846,11 +846,11 @@
// Use the boot image component count to calculate the checksum from
// the appropriate number of boot image chunks.
uint32_t boot_image_component_count = image_header.GetBootImageComponentCount();
- size_t boot_image_spaces_size = boot_image_spaces.size();
- if (boot_image_component_count > boot_image_spaces_size) {
+ size_t expected_image_component_count = ImageSpace::GetNumberOfComponents(boot_image_spaces);
+ if (boot_image_component_count > expected_image_component_count) {
*error_msg = StringPrintf("Too many boot image dependencies (%u > %zu) in image %s",
boot_image_component_count,
- boot_image_spaces_size,
+ expected_image_component_count,
image_filename);
return false;
}
diff --git a/runtime/native/dalvik_system_ZygoteHooks.cc b/runtime/native/dalvik_system_ZygoteHooks.cc
index cdb63f3..2c18ddb 100644
--- a/runtime/native/dalvik_system_ZygoteHooks.cc
+++ b/runtime/native/dalvik_system_ZygoteHooks.cc
@@ -144,7 +144,7 @@
DEBUG_NATIVE_DEBUGGABLE = 1 << 7,
DEBUG_JAVA_DEBUGGABLE = 1 << 8,
DISABLE_VERIFIER = 1 << 9,
- ONLY_USE_SYSTEM_OAT_FILES = 1 << 10,
+ ONLY_USE_TRUSTED_OAT_FILES = 1 << 10, // Formerly ONLY_USE_SYSTEM_OAT_FILES
DEBUG_GENERATE_MINI_DEBUG_INFO = 1 << 11,
HIDDEN_API_ENFORCEMENT_POLICY_MASK = (1 << 12)
| (1 << 13),
@@ -318,9 +318,9 @@
runtime_flags &= ~DISABLE_VERIFIER;
}
- if ((runtime_flags & ONLY_USE_SYSTEM_OAT_FILES) != 0 || is_system_server) {
- runtime->GetOatFileManager().SetOnlyUseSystemOatFiles();
- runtime_flags &= ~ONLY_USE_SYSTEM_OAT_FILES;
+ if ((runtime_flags & ONLY_USE_TRUSTED_OAT_FILES) != 0 || is_system_server) {
+ runtime->GetOatFileManager().SetOnlyUseTrustedOatFiles();
+ runtime_flags &= ~ONLY_USE_TRUSTED_OAT_FILES;
}
api_enforcement_policy = hiddenapi::EnforcementPolicyFromInt(
diff --git a/runtime/oat_file_assistant.cc b/runtime/oat_file_assistant.cc
index a5ccb69..d176eb6 100644
--- a/runtime/oat_file_assistant.cc
+++ b/runtime/oat_file_assistant.cc
@@ -80,12 +80,12 @@
const InstructionSet isa,
ClassLoaderContext* context,
bool load_executable,
- bool only_load_system_executable)
+ bool only_load_trusted_executable)
: OatFileAssistant(dex_location,
isa,
context,
load_executable,
- only_load_system_executable,
+ only_load_trusted_executable,
/*vdex_fd=*/ -1,
/*oat_fd=*/ -1,
/*zip_fd=*/ -1) {}
@@ -95,14 +95,14 @@
const InstructionSet isa,
ClassLoaderContext* context,
bool load_executable,
- bool only_load_system_executable,
+ bool only_load_trusted_executable,
int vdex_fd,
int oat_fd,
int zip_fd)
: context_(context),
isa_(isa),
load_executable_(load_executable),
- only_load_system_executable_(only_load_system_executable),
+ only_load_trusted_executable_(only_load_trusted_executable),
odex_(this, /*is_oat_location=*/ false),
oat_(this, /*is_oat_location=*/ true),
vdex_for_odex_(this, /*is_oat_location=*/ false),
@@ -453,8 +453,8 @@
// zip_file_only_contains_uncompressed_dex_ is only set during fetching the dex checksums.
DCHECK(required_dex_checksums_attempted_);
- if (only_load_system_executable_ &&
- !LocationIsOnSystem(file.GetLocation().c_str()) &&
+ if (only_load_trusted_executable_ &&
+ !LocationIsTrusted(file.GetLocation()) &&
file.ContainsDexCode() &&
zip_file_only_contains_uncompressed_dex_) {
LOG(ERROR) << "Not loading "
@@ -846,8 +846,8 @@
&error_msg));
}
} else {
- if (executable && oat_file_assistant_->only_load_system_executable_) {
- executable = LocationIsOnSystem(filename_.c_str());
+ if (executable && oat_file_assistant_->only_load_trusted_executable_) {
+ executable = LocationIsTrusted(filename_);
}
VLOG(oat) << "Loading " << filename_ << " with executable: " << executable;
if (use_fd_) {
diff --git a/runtime/oat_file_assistant.h b/runtime/oat_file_assistant.h
index 50b54af..9cfa781 100644
--- a/runtime/oat_file_assistant.h
+++ b/runtime/oat_file_assistant.h
@@ -107,13 +107,14 @@
// load_executable should be true if the caller intends to try and load
// executable code for this dex location.
//
- // only_load_system_executable should be true if the caller intends to have
- // only oat files from /system loaded executable.
+ // only_load_trusted_executable should be true if the caller intends to have
+ // only oat files from trusted locations loaded executable. See IsTrustedLocation() for
+ // details on trusted locations.
OatFileAssistant(const char* dex_location,
const InstructionSet isa,
ClassLoaderContext* context,
bool load_executable,
- bool only_load_system_executable = false);
+ bool only_load_trusted_executable = false);
// Similar to this(const char*, const InstructionSet, bool), however, if a valid zip_fd is
// provided, vdex, oat, and zip files will be read from vdex_fd, oat_fd and zip_fd respectively.
@@ -122,7 +123,7 @@
const InstructionSet isa,
ClassLoaderContext* context,
bool load_executable,
- bool only_load_system_executable,
+ bool only_load_trusted_executable,
int vdex_fd,
int oat_fd,
int zip_fd);
@@ -425,8 +426,8 @@
// Whether we will attempt to load oat files executable.
bool load_executable_ = false;
- // Whether only oat files on /system are loaded executable.
- const bool only_load_system_executable_ = false;
+ // Whether only oat files from trusted locations are loaded executable.
+ const bool only_load_trusted_executable_ = false;
// Whether the potential zip file only contains uncompressed dex.
// Will be set during GetRequiredDexChecksums.
bool zip_file_only_contains_uncompressed_dex_ = true;
diff --git a/runtime/oat_file_manager.cc b/runtime/oat_file_manager.cc
index 92929b3..391d597 100644
--- a/runtime/oat_file_manager.cc
+++ b/runtime/oat_file_manager.cc
@@ -72,7 +72,7 @@
WriterMutexLock mu(Thread::Current(), *Locks::oat_file_manager_lock_);
CHECK(!only_use_system_oat_files_ ||
- LocationIsOnSystem(oat_file->GetLocation().c_str()) ||
+ LocationIsTrusted(oat_file->GetLocation()) ||
!oat_file->IsExecutable())
<< "Registering a non /system oat file: " << oat_file->GetLocation();
DCHECK(oat_file != nullptr);
@@ -800,7 +800,7 @@
}
}
-void OatFileManager::SetOnlyUseSystemOatFiles() {
+void OatFileManager::SetOnlyUseTrustedOatFiles() {
ReaderMutexLock mu(Thread::Current(), *Locks::oat_file_manager_lock_);
// Make sure all files that were loaded up to this point are on /system.
// Skip the image files as they can encode locations that don't exist (eg not
@@ -810,8 +810,8 @@
for (const std::unique_ptr<const OatFile>& oat_file : oat_files_) {
if (boot_set.find(oat_file.get()) == boot_set.end()) {
- if (!LocationIsOnSystem(oat_file->GetLocation().c_str())) {
- // When the file is not on system, we check whether the oat file has any
+ if (!LocationIsTrusted(oat_file->GetLocation())) {
+ // When the file is not in a trusted location, we check whether the oat file has any
// AOT or DEX code. It is a fatal error if it has.
if (CompilerFilter::IsAotCompilationEnabled(oat_file->GetCompilerFilter()) ||
oat_file->ContainsDexCode()) {
diff --git a/runtime/oat_file_manager.h b/runtime/oat_file_manager.h
index 25d0023..f8a7341 100644
--- a/runtime/oat_file_manager.h
+++ b/runtime/oat_file_manager.h
@@ -123,7 +123,7 @@
void DumpForSigQuit(std::ostream& os);
- void SetOnlyUseSystemOatFiles();
+ void SetOnlyUseTrustedOatFiles();
// Spawn a background thread which verifies all classes in the given dex files.
void RunBackgroundVerification(const std::vector<const DexFile*>& dex_files,
diff --git a/runtime/parsed_options.cc b/runtime/parsed_options.cc
index e5e7c4f..38552c6 100644
--- a/runtime/parsed_options.cc
+++ b/runtime/parsed_options.cc
@@ -432,7 +432,7 @@
.WithType<unsigned int>()
.IntoKey(M::MetricsReportingPeriod)
.Define("-Xonly-use-system-oat-files")
- .IntoKey(M::OnlyUseSystemOatFiles)
+ .IntoKey(M::OnlyUseTrustedOatFiles)
.Define("-Xverifier-logging-threshold=_")
.WithType<unsigned int>()
.IntoKey(M::VerifierLoggingThreshold)
diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index d8f7639..35b7055 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -1931,9 +1931,9 @@
VLOG(startup) << "Runtime::Init exiting";
- // Set OnlyUseSystemOatFiles only after boot classpath has been set up.
- if (runtime_options.Exists(Opt::OnlyUseSystemOatFiles)) {
- oat_file_manager_->SetOnlyUseSystemOatFiles();
+ // Set OnlyUseTrustedOatFiles only after the boot classpath has been set up.
+ if (runtime_options.Exists(Opt::OnlyUseTrustedOatFiles)) {
+ oat_file_manager_->SetOnlyUseTrustedOatFiles();
}
return true;
diff --git a/runtime/runtime_options.def b/runtime/runtime_options.def
index c60c0b3..7be2fa4 100644
--- a/runtime/runtime_options.def
+++ b/runtime/runtime_options.def
@@ -169,7 +169,7 @@
RUNTIME_OPTIONS_KEY (unsigned int, GlobalRefAllocStackTraceLimit, 0) // 0 = off
RUNTIME_OPTIONS_KEY (Unit, UseStderrLogger)
-RUNTIME_OPTIONS_KEY (Unit, OnlyUseSystemOatFiles)
+RUNTIME_OPTIONS_KEY (Unit, OnlyUseTrustedOatFiles)
RUNTIME_OPTIONS_KEY (unsigned int, VerifierLoggingThreshold, 100)
RUNTIME_OPTIONS_KEY (bool, FastClassNotFoundException, true)
diff --git a/test/odsign/test-src/com/android/tests/odsign/OnDeviceSigningHostTest.java b/test/odsign/test-src/com/android/tests/odsign/OnDeviceSigningHostTest.java
index b2d5624..1ac89fd 100644
--- a/test/odsign/test-src/com/android/tests/odsign/OnDeviceSigningHostTest.java
+++ b/test/odsign/test-src/com/android/tests/odsign/OnDeviceSigningHostTest.java
@@ -18,14 +18,16 @@
import static com.google.common.truth.Truth.assertWithMessage;
+import static org.junit.Assert.assertTrue;
import static org.junit.Assume.assumeTrue;
import android.cts.install.lib.host.InstallUtilsHost;
-import com.android.tradefed.testtype.DeviceJUnit4ClassRunner;
import com.android.tradefed.device.ITestDevice.ApexInfo;
+import com.android.tradefed.testtype.DeviceJUnit4ClassRunner;
import com.android.tradefed.testtype.junit4.BaseHostJUnit4Test;
import com.android.tradefed.testtype.junit4.DeviceTestRunOptions;
+import com.android.tradefed.util.CommandResult;
import org.junit.After;
import org.junit.Before;
@@ -33,11 +35,16 @@
import org.junit.runner.RunWith;
import java.time.Duration;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
@RunWith(DeviceJUnit4ClassRunner.class)
public class OnDeviceSigningHostTest extends BaseHostJUnit4Test {
private static final String APEX_FILENAME = "test_com.android.art.apex";
+ private static final String ART_APEX_DALVIK_CACHE_DIRNAME =
+ "/data/misc/apexdata/com.android.art/dalvik-cache";
private static final String TEST_APP_PACKAGE_NAME = "com.android.tests.odsign";
private static final String TEST_APP_APK = "odsign_e2e_test_app.apk";
@@ -77,6 +84,137 @@
runDeviceTests(options);
}
+ private Set<String> getMappedArtifacts(String pid, String grepPattern) throws Exception {
+ final String grepCommand = String.format("grep \"%s\" /proc/%s/maps", grepPattern, pid);
+ CommandResult result = getDevice().executeShellV2Command(grepCommand);
+ assertTrue(result.toString(), result.getExitCode() == 0);
+ Set<String> mappedFiles = new HashSet<>();
+ for (String line : result.getStdout().split("\\R")) {
+ int start = line.indexOf(ART_APEX_DALVIK_CACHE_DIRNAME);
+ if (line.contains("[")) {
+ continue; // ignore anonymously mapped sections which are quoted in square braces.
+ }
+ mappedFiles.add(line.substring(start));
+ }
+ return mappedFiles;
+ }
+
+ private String[] getSystemServerClasspath() throws Exception {
+ String systemServerClasspath =
+ getDevice().executeShellCommand("echo $SYSTEMSERVERCLASSPATH");
+ return systemServerClasspath.split(":");
+ }
+
+ private String getSystemServerIsa(String mappedArtifact) {
+ // Artifact path for system server artifacts has the form:
+ // ART_APEX_DALVIK_CACHE_DIRNAME + "/<arch>/system@framework@some.jar@classes.odex"
+ // `mappedArtifacts` may include other artifacts, such as boot-framework.oat that are not
+ // prefixed by the architecture.
+ String[] pathComponents = mappedArtifact.split("/");
+ return pathComponents[pathComponents.length - 2];
+ }
+
+ private void verifySystemServerLoadedArtifacts() throws Exception {
+ String[] classpathElements = getSystemServerClasspath();
+ assertTrue("SYSTEMSERVERCLASSPATH is empty", classpathElements.length > 0);
+
+ String systemServerPid = getDevice().executeShellCommand("pgrep system_server");
+ assertTrue(systemServerPid != null);
+
+ // system_server artifacts are in the APEX data dalvik cache and names all contain
+ // the word "@classes". Look for mapped files that match this pattern in the proc map for
+ // system_server.
+ final String grepPattern = ART_APEX_DALVIK_CACHE_DIRNAME + ".*@classes";
+ final Set<String> mappedArtifacts = getMappedArtifacts(systemServerPid, grepPattern);
+ assertTrue(
+ "No mapped artifacts under " + ART_APEX_DALVIK_CACHE_DIRNAME,
+ mappedArtifacts.size() > 0);
+ final String isa = getSystemServerIsa(mappedArtifacts.iterator().next());
+ final String isaCacheDirectory = String.format("%s/%s", ART_APEX_DALVIK_CACHE_DIRNAME, isa);
+
+ // Extension types for artifacts that this test looks for.
+ final String[] extensions = new String[] {".art", ".odex", ".vdex"};
+
+ // Check the non-APEX components in the system_server classpath have mapped artifacts.
+ for (String element : classpathElements) {
+ // Skip system_server classpath elements from APEXes as these are not currently
+ // compiled.
+ if (element.startsWith("/apex")) {
+ continue;
+ }
+ String escapedPath = element.substring(1).replace('/', '@');
+ for (String extension : extensions) {
+ final String fullArtifactPath =
+ String.format("%s/%s@classes%s", isaCacheDirectory, escapedPath, extension);
+ assertTrue(
+ "Missing " + fullArtifactPath, mappedArtifacts.contains(fullArtifactPath));
+ }
+ }
+
+ for (String mappedArtifact : mappedArtifacts) {
+ // Check no APEX JAR artifacts are mapped for system_server since if there
+ // are, then the policy around not compiling APEX jars for system_server has
+ // changed and this test needs updating here and in the system_server classpath
+ // check above.
+ assertTrue(
+ "Unexpected mapped artifact: " + mappedArtifact,
+ mappedArtifact.contains("/apex"));
+
+ // Check the mapped artifact has a .art, .odex or .vdex extension.
+ final boolean knownArtifactKind =
+ Arrays.stream(extensions).anyMatch(e -> mappedArtifact.endsWith(e));
+ assertTrue("Unknown artifact kind: " + mappedArtifact, knownArtifactKind);
+ }
+ }
+
+ private void verifyZygoteLoadedArtifacts(String zygotePid) throws Exception {
+ final String bootExtensionName = "boot-framework";
+ final Set<String> mappedArtifacts = getMappedArtifacts(zygotePid, bootExtensionName);
+
+ assertTrue("Expect 3 boot-framework artifacts", mappedArtifacts.size() == 3);
+
+ // Extension types for artifacts that this test looks for.
+ final String[] extensions = new String[] {".art", ".oat", ".vdex"};
+
+ for (String extension : extensions) {
+ final String artifact = bootExtensionName + extension;
+ final boolean found = mappedArtifacts.stream().anyMatch(a -> a.endsWith(artifact));
+ assertTrue(artifact + " not found", found);
+ }
+ }
+
+ private void verifyZygotesLoadedArtifacts() throws Exception {
+ // There are potentially two zygote processes "zygote" and "zygote64". These are
+ // instances 32-bit and 64-bit unspecialized app_process processes.
+ // (frameworks/base/cmds/app_process).
+ int zygoteCount = 0;
+ for (String processName : new String[] {"zygote", "zygote64"}) {
+ final CommandResult pgrepResult =
+ getDevice().executeShellV2Command("pgrep " + processName);
+ if (pgrepResult.getExitCode() != 0) {
+ continue;
+ }
+ final String zygotePid = pgrepResult.getStdout();
+ verifyZygoteLoadedArtifacts(zygotePid);
+ zygoteCount += 1;
+ }
+ assertTrue("No zygote processes found", zygoteCount > 0);
+ }
+
+ @Test
+ public void verifyGeneratedArtifactsLoaded() throws Exception {
+ // Checking zygote and system_server need the device have adb root to walk process maps.
+ final boolean adbEnabled = getDevice().enableAdbRoot();
+ assertTrue("ADB root failed and required to get process maps", adbEnabled);
+
+ // Check both zygote and system_server processes to see that they have loaded the
+ // artifacts compiled and signed by odrefresh and odsign. We check both here rather than
+ // having a separate test because the device reboots between each @Test method and
+ // that is an expensive use of time.
+ verifyZygotesLoadedArtifacts();
+ verifySystemServerLoadedArtifacts();
+ }
+
private void reboot() throws Exception {
getDevice().reboot();
boolean success = getDevice().waitForBootComplete(BOOT_COMPLETE_TIMEOUT.toMillis());