Make sure that files under bin/ directory of apk get X permission
This change adds support for adding executables under bin/ directory of
the apk, so that when that apk is mounted in the Microdroid the payload
can then execute such binaries.
As an example, put measure_io binary into the MicrodroidTestApp.apk, and
add a test that asserts that /mnt/apk/bin/measure_io has R & X
permissions.
In the following change the authfs benchmark tests will be migrated to
use the measure_io binary under /mnt/apk/bin/measure_io instead of
pushing one to the /data partition inside Microdroid. This is required
to reland the change that mounts /data with MS_NOEXEC.
Bug: 265261525
Bug: 270955654
Test: atest MicrodroidTestApp
Change-Id: Ia5294f2a1bc2a54505670425bbd835c7793c6f29
diff --git a/authfs/tests/benchmarks/Android.bp b/authfs/tests/benchmarks/Android.bp
index 9bdef7b..96cbd1d 100644
--- a/authfs/tests/benchmarks/Android.bp
+++ b/authfs/tests/benchmarks/Android.bp
@@ -36,3 +36,19 @@
"libbase",
],
}
+
+// Package measure_io binary into a jar, to bundle with the MicrodroidTestApp.
+// When MicrodroidTestApp is mounted inside the Microdroid, the zipfuse will
+// add the +x permission on it.
+java_genrule {
+ name: "measure_io_as_jar",
+ out: ["measure_io.jar"],
+ srcs: [
+ ":measure_io",
+ ],
+ cmd: "out_dir=$$(dirname $(out))" +
+ "&& bin_dir=\"bin\" " +
+ "&& mkdir -p $$out_dir/$$bin_dir" +
+ "&& cp $(in) $$out_dir/$$bin_dir" +
+ "&& jar cf $(out) -C $$out_dir $$bin_dir",
+}
diff --git a/tests/aidl/com/android/microdroid/testservice/ITestService.aidl b/tests/aidl/com/android/microdroid/testservice/ITestService.aidl
index 4611134..c1e39db 100644
--- a/tests/aidl/com/android/microdroid/testservice/ITestService.aidl
+++ b/tests/aidl/com/android/microdroid/testservice/ITestService.aidl
@@ -56,6 +56,9 @@
/* get the content of the specified file. */
String readFromFile(String path);
+ /* get file permissions of the give file by stat'ing it */
+ int getFilePermissions(String path);
+
/**
* Request the service to exit, triggering the termination of the VM. This may cause any
* requests in flight to fail.
diff --git a/tests/helper/src/java/com/android/microdroid/test/device/MicrodroidDeviceTestBase.java b/tests/helper/src/java/com/android/microdroid/test/device/MicrodroidDeviceTestBase.java
index be3c1da..4a33d6f 100644
--- a/tests/helper/src/java/com/android/microdroid/test/device/MicrodroidDeviceTestBase.java
+++ b/tests/helper/src/java/com/android/microdroid/test/device/MicrodroidDeviceTestBase.java
@@ -443,6 +443,7 @@
public String mFileContent;
public byte[] mBcc;
public long[] mTimings;
+ public int mFileMode;
public void assertNoException() {
if (mException != null) {
diff --git a/tests/testapk/Android.bp b/tests/testapk/Android.bp
index bafab53..9f80433 100644
--- a/tests/testapk/Android.bp
+++ b/tests/testapk/Android.bp
@@ -31,6 +31,7 @@
"cbor-java",
"truth-prebuilt",
"compatibility-common-util-devicesidelib",
+ "measure_io_as_jar",
],
jni_libs: [
"MicrodroidTestNativeLib",
diff --git a/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java b/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
index f3f1252..0a81542 100644
--- a/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
+++ b/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
@@ -45,6 +45,7 @@
import android.os.ParcelFileDescriptor.AutoCloseInputStream;
import android.os.ParcelFileDescriptor.AutoCloseOutputStream;
import android.os.SystemProperties;
+import android.system.OsConstants;
import android.system.virtualmachine.VirtualMachine;
import android.system.virtualmachine.VirtualMachineCallback;
import android.system.virtualmachine.VirtualMachineConfig;
@@ -1779,6 +1780,42 @@
}
}
+ @Test
+ @CddTest(requirements = {"9.17/C-1-5"})
+ public void testFileUnderBinHasExecutePermission() throws Exception {
+ assumeSupportedKernel();
+
+ VirtualMachineConfig vmConfig =
+ newVmConfigBuilder()
+ .setPayloadBinaryName("MicrodroidTestNativeLib.so")
+ .setMemoryBytes(minMemoryRequired())
+ .setDebugLevel(DEBUG_LEVEL_FULL)
+ .build();
+ VirtualMachine vm = forceCreateNewVirtualMachine("test_vm_perms", vmConfig);
+
+ TestResults testResults =
+ runVmTestService(
+ TAG,
+ vm,
+ (ts, tr) -> {
+ tr.mFileMode = ts.getFilePermissions("/mnt/apk/bin/measure_io");
+ });
+
+ testResults.assertNoException();
+ int allPermissionsMask =
+ OsConstants.S_IRUSR
+ | OsConstants.S_IWUSR
+ | OsConstants.S_IXUSR
+ | OsConstants.S_IRGRP
+ | OsConstants.S_IWGRP
+ | OsConstants.S_IXGRP
+ | OsConstants.S_IROTH
+ | OsConstants.S_IWOTH
+ | OsConstants.S_IXOTH;
+ assertThat(testResults.mFileMode & allPermissionsMask)
+ .isEqualTo(OsConstants.S_IRUSR | OsConstants.S_IXUSR);
+ }
+
private static class VmShareServiceConnection implements ServiceConnection {
private final CountDownLatch mLatch = new CountDownLatch(1);
diff --git a/tests/testapk/src/native/testbinary.cpp b/tests/testapk/src/native/testbinary.cpp
index 19d6cd4..07c8cd4 100644
--- a/tests/testapk/src/native/testbinary.cpp
+++ b/tests/testapk/src/native/testbinary.cpp
@@ -251,6 +251,18 @@
return ScopedAStatus::ok();
}
+ ScopedAStatus getFilePermissions(const std::string& path, int32_t* out) override {
+ struct stat sb;
+ if (stat(path.c_str(), &sb) != -1) {
+ *out = sb.st_mode;
+ } else {
+ std::string msg = "stat " + path + " failed : " + std::strerror(errno);
+ return ScopedAStatus::fromExceptionCodeWithMessage(EX_SERVICE_SPECIFIC,
+ msg.c_str());
+ }
+ return ScopedAStatus::ok();
+ }
+
ScopedAStatus quit() override { exit(0); }
};
auto testService = ndk::SharedRefBase::make<TestService>();
diff --git a/tests/vmshareapp/src/java/com/android/microdroid/test/sharevm/VmShareServiceImpl.java b/tests/vmshareapp/src/java/com/android/microdroid/test/sharevm/VmShareServiceImpl.java
index 278e1a2..8c995d7 100644
--- a/tests/vmshareapp/src/java/com/android/microdroid/test/sharevm/VmShareServiceImpl.java
+++ b/tests/vmshareapp/src/java/com/android/microdroid/test/sharevm/VmShareServiceImpl.java
@@ -230,6 +230,11 @@
}
@Override
+ public int getFilePermissions(String path) throws RemoteException {
+ throw new UnsupportedOperationException("Not supported");
+ }
+
+ @Override
public void quit() throws RemoteException {
throw new UnsupportedOperationException("Not supported");
}
diff --git a/zipfuse/src/inode.rs b/zipfuse/src/inode.rs
index 3edbc49..ea63422 100644
--- a/zipfuse/src/inode.rs
+++ b/zipfuse/src/inode.rs
@@ -99,12 +99,8 @@
InodeData { mode, size: 0, data: InodeDataData::Directory(HashMap::new()) }
}
- fn new_file(zip_index: ZipIndex, zip_file: &zip::read::ZipFile) -> InodeData {
- InodeData {
- mode: zip_file.unix_mode().unwrap_or(DEFAULT_FILE_MODE),
- size: zip_file.size(),
- data: InodeDataData::File(zip_index),
- }
+ fn new_file(zip_index: ZipIndex, mode: u32, zip_file: &zip::read::ZipFile) -> InodeData {
+ InodeData { mode, size: zip_file.size(), data: InodeDataData::File(zip_index) }
}
fn add_to_directory(&mut self, name: CString, entry: DirectoryEntry) {
@@ -188,6 +184,16 @@
let mut parent = ROOT;
let mut iter = path.iter().peekable();
+
+ let mut file_mode = DEFAULT_FILE_MODE;
+ if path.starts_with("bin/") {
+ // Allow files under bin to have execute permission, this enables payloads to bundle
+ // additional binaries that they might want to execute.
+ // An example of such binary is measure_io one used in the authfs performance tests.
+ // More context available at b/265261525 and b/270955654.
+ file_mode |= libc::S_IXUSR;
+ }
+
while let Some(name) = iter.next() {
// TODO(jiyong): remove this check by canonicalizing `path`
if name == ".." {
@@ -211,8 +217,11 @@
}
// No inode found. Create a new inode and add it to the inode table.
+ // At the moment of writing this comment the apk file doesn't specify any
+ // permissions (apart from the ones on lib/), but it might change in the future.
+ // TODO(b/270955654): should we control the file permissions ourselves?
let inode = if is_file {
- InodeData::new_file(i, &file)
+ InodeData::new_file(i, file.unix_mode().unwrap_or(file_mode), &file)
} else if is_leaf {
InodeData::new_dir(file.unix_mode().unwrap_or(DEFAULT_DIR_MODE))
} else {