Use temp mounting when running pre/post-install hooks
* apexd will temp-mount all the apexes in the session and then fork into
apexd --pre-install <mount-point-with-hook> [<all-other-mount-points>]
* Now during submitStagedSession temp-mounting happens twice:
in verifyPackages() and in preInstallPackages() stages. This will be
fix when we will keep temp-mount alive while apexd session is not
verified/canceled.
* Since I was there, added some comments in pre/post-install flow
handling to make it easier for future readers to follow the code.
Note that adb install com.android.art.debug.apex still fails during
preInstallPackages(), but this failure is most likely specific to art
and doesn't come from apexd.
Test: adb install ${OUT}/system/apex/com.android.art.debug.apex
Test: verified that install doesn't fail with mount point clash issue
Test: adb shell cmd apexservice preinstallPackages /system/apex/com.android.art.debug.apex
Bug: 140935513
Change-Id: Ia67d3af60693ff485e17915ebb5a140a81d36542
diff --git a/apexd/apex_constants.h b/apexd/apex_constants.h
index aeb53b5..eb2ec7d 100644
--- a/apexd/apex_constants.h
+++ b/apexd/apex_constants.h
@@ -33,5 +33,6 @@
static constexpr const char* kApexPackageSuffix = ".apex";
+static constexpr const char* kManifestFilename = "apex_manifest.json";
} // namespace apex
} // namespace android
diff --git a/apexd/apex_file.cpp b/apexd/apex_file.cpp
index f45cc9c..89ae410 100644
--- a/apexd/apex_file.cpp
+++ b/apexd/apex_file.cpp
@@ -32,6 +32,7 @@
#include <google/protobuf/util/message_differencer.h>
#include <libavb/libavb.h>
+#include "apex_constants.h"
#include "apex_key.h"
#include "apexd_utils.h"
#include "string_log.h"
@@ -50,7 +51,6 @@
namespace {
constexpr const char* kImageFilename = "apex_payload.img";
-constexpr const char* kManifestFilename = "apex_manifest.json";
constexpr const char* kBundledPublicKeyFilename = "apex_pubkey";
#ifdef DEBUG_ALLOW_BUNDLED_KEY
constexpr const bool kDebugAllowBundledKey = true;
diff --git a/apexd/apex_manifest.cpp b/apexd/apex_manifest.cpp
index bad5647..1d1de4d 100644
--- a/apexd/apex_manifest.cpp
+++ b/apexd/apex_manifest.cpp
@@ -15,8 +15,9 @@
*/
#include "apex_manifest.h"
-#include "string_log.h"
+#include <android-base/file.h>
#include <android-base/logging.h>
+#include "string_log.h"
#include <google/protobuf/util/json_util.h>
#include <google/protobuf/util/type_resolver_util.h>
@@ -90,5 +91,13 @@
return apexManifest.name() + "@" + std::to_string(apexManifest.version());
}
+Result<ApexManifest> ReadManifest(const std::string& path) {
+ std::string content;
+ if (!android::base::ReadFileToString(path, &content)) {
+ return Error() << "Failed to read manifest file: " << path;
+ }
+ return ParseManifest(content);
+}
+
} // namespace apex
} // namespace android
diff --git a/apexd/apex_manifest.h b/apexd/apex_manifest.h
index 4c40844..f299648 100644
--- a/apexd/apex_manifest.h
+++ b/apexd/apex_manifest.h
@@ -31,7 +31,8 @@
android::base::Result<ApexManifest> ParseManifest(const std::string& content);
// Returns package id of an ApexManifest
std::string GetPackageId(const ApexManifest& apex_manifest);
-
+// Reads and parses APEX manifest from the file on disk.
+android::base::Result<ApexManifest> ReadManifest(const std::string& path);
} // namespace apex
} // namespace android
diff --git a/apexd/apexd.cpp b/apexd/apexd.cpp
index c2f7feb..c525ed2 100644
--- a/apexd/apexd.cpp
+++ b/apexd/apexd.cpp
@@ -550,6 +550,8 @@
}
Result<void> Unmount(const MountedApexData& data) {
+ LOG(DEBUG) << "Unmounting " << data.full_path << " from mount point "
+ << data.mount_point;
// Lazily try to umount whatever is mounted.
if (umount2(data.mount_point.c_str(), UMOUNT_NOFOLLOW | MNT_DETACH) != 0 &&
errno != EINVAL && errno != ENOENT) {
@@ -574,7 +576,7 @@
if (!data.loop_name.empty()) {
auto log_fn = [](const std::string& path,
const std::string& id ATTRIBUTE_UNUSED) {
- LOG(VERBOSE) << "Freeing loop device " << path << "for unmount.";
+ LOG(VERBOSE) << "Freeing loop device " << path << " for unmount.";
};
loop::DestroyLoopDevice(data.loop_name, log_fn);
}
@@ -582,10 +584,6 @@
return {};
}
-std::string GetPackageTempMountPoint(const ApexManifest& manifest) {
- return StringPrintf("%s.tmp",
- apexd_private::GetPackageMountPoint(manifest).c_str());
-}
template <typename VerifyFn>
Result<void> RunVerifyFnInsideTempMount(const ApexFile& apex,
@@ -594,7 +592,7 @@
// this will also read the entire block device through dm-verity, so
// we can be sure there is no corruption.
const std::string& temp_mount_point =
- GetPackageTempMountPoint(apex.GetManifest());
+ apexd_private::GetPackageTempMountPoint(apex.GetManifest());
Result<MountedApexData> mount_status =
VerifyAndTempMountPackage(apex, temp_mount_point);
@@ -1025,8 +1023,6 @@
} // namespace
-namespace apexd_private {
-
Result<void> MountPackage(const ApexFile& apex, const std::string& mountPoint) {
auto ret =
MountPackageImpl(apex, mountPoint, GetPackageId(apex.GetManifest()),
@@ -1035,13 +1031,21 @@
return ret.error();
}
- gMountedApexes.AddMountedApex(apex.GetManifest().name(), false,
- std::move(*ret));
+ gMountedApexes.AddMountedApex(apex.GetManifest().name(), false, *ret);
return {};
}
-Result<void> UnmountPackage(const ApexFile& apex) {
- return android::apex::UnmountPackage(apex, /* allow_latest= */ false);
+namespace apexd_private {
+
+Result<MountedApexData> TempMountPackage(const ApexFile& apex,
+ const std::string& mount_point) {
+ // TODO(ioffe): consolidate these two methods.
+ return android::apex::VerifyAndTempMountPackage(apex, mount_point);
+}
+
+Result<void> Unmount(const MountedApexData& data) {
+ // TODO(ioffe): consolidate these two methods.
+ return android::apex::Unmount(data);
}
bool IsMounted(const std::string& name, const std::string& full_path) {
@@ -1059,6 +1063,10 @@
return StringPrintf("%s/%s", kApexRoot, GetPackageId(manifest).c_str());
}
+std::string GetPackageTempMountPoint(const ApexManifest& manifest) {
+ return StringPrintf("%s.tmp", GetPackageMountPoint(manifest).c_str());
+}
+
std::string GetActiveMountPoint(const ApexManifest& manifest) {
return StringPrintf("%s/%s", kApexRoot, manifest.name().c_str());
}
@@ -1127,8 +1135,7 @@
const std::string& mountPoint = apexd_private::GetPackageMountPoint(manifest);
if (!version_found_mounted) {
- Result<void> mountStatus =
- apexd_private::MountPackage(apex_file, mountPoint);
+ auto mountStatus = MountPackage(apex_file, mountPoint);
if (!mountStatus) {
return mountStatus;
}
diff --git a/apexd/apexd_prepostinstall.cpp b/apexd/apexd_prepostinstall.cpp
index f88a793..d9ece73 100644
--- a/apexd/apexd_prepostinstall.cpp
+++ b/apexd/apexd_prepostinstall.cpp
@@ -33,6 +33,7 @@
#include <android-base/scopeguard.h>
#include <android-base/strings.h>
+#include "apex_database.h"
#include "apex_file.h"
#include "apexd.h"
#include "apexd_private.h"
@@ -47,6 +48,8 @@
namespace {
+using MountedApexData = MountedApexDatabase::MountedApexData;
+
void CloseSTDDescriptors() {
// exec()d process will reopen STD* file descriptors as
// /dev/null
@@ -55,30 +58,35 @@
close(STDERR_FILENO);
}
+// Instead of temp mounting inside this fuction, we can make a caller do it.
+// This will align with the plan of extending temp mounting to provide a
+// way to run additional pre-reboot verification of an APEX.
+// TODO(ioffe): pass mount points instead of apex files.
template <typename Fn>
Result<void> StageFnInstall(const std::vector<ApexFile>& apexes, Fn fn,
const char* arg, const char* name) {
// TODO: Support a session with more than one pre-install hook.
- const ApexFile* hook_file = nullptr;
- for (const ApexFile& f : apexes) {
- if (!(f.GetManifest().*fn)().empty()) {
- if (hook_file != nullptr) {
+ int hook_idx = -1;
+ for (size_t i = 0; i < apexes.size(); i++) {
+ if (!(apexes[i].GetManifest().*fn)().empty()) {
+ if (hook_idx != -1) {
return Error() << "Missing support for multiple " << name << " hooks";
}
- hook_file = &f;
+ hook_idx = i;
}
}
- CHECK(hook_file != nullptr);
- LOG(VERBOSE) << name << " for " << hook_file->GetPath();
+ CHECK(hook_idx != -1);
+ LOG(VERBOSE) << name << " for " << apexes[hook_idx].GetPath();
- std::vector<const ApexFile*> mounted_apexes;
+ std::vector<MountedApexData> mounted_apexes;
std::vector<std::string> activation_dirs;
auto preinstall_guard = android::base::make_scope_guard([&]() {
- for (const ApexFile* f : mounted_apexes) {
- Result<void> st = apexd_private::UnmountPackage(*f);
+ for (const auto& mount : mounted_apexes) {
+ Result<void> st = apexd_private::Unmount(mount);
if (!st) {
- LOG(ERROR) << "Failed to unmount " << f->GetPath() << " after " << name
- << ": " << st.error();
+ LOG(ERROR) << "Failed to unmount " << mount.full_path << " from "
+ << mount.mount_point << " after " << name << ": "
+ << st.error();
}
}
for (const std::string& active_point : activation_dirs) {
@@ -90,18 +98,21 @@
});
for (const ApexFile& apex : apexes) {
- // 1) Mount the package, if necessary.
+ // 1) Mount the package.
std::string mount_point =
- apexd_private::GetPackageMountPoint(apex.GetManifest());
+ apexd_private::GetPackageTempMountPoint(apex.GetManifest());
- if (!apexd_private::IsMounted(apex.GetManifest().name(), apex.GetPath())) {
- Result<void> mountStatus = apexd_private::MountPackage(apex, mount_point);
- if (!mountStatus) {
- return mountStatus;
- }
- mounted_apexes.push_back(&apex);
+ auto mount_data = apexd_private::TempMountPackage(apex, mount_point);
+ if (!mount_data) {
+ return mount_data.error();
}
+ mounted_apexes.push_back(std::move(*mount_data));
+ // Given the fact, that we only allow updates of existing APEXes, all the
+ // activation points will always be already created. Only scenario, when it
+ // won't be the case might be apexservice_test. But even then, it might be
+ // safer to move active_point creation logic to run after unshare.
+ // TODO(ioffe): move creation of activation points inside RunFnInstall?
// 2) Ensure there is an activation point, and we will clean it up.
std::string active_point =
apexd_private::GetActiveMountPoint(apex.GetManifest());
@@ -119,11 +130,11 @@
// 3) Create invocation args.
std::vector<std::string> args{
"/system/bin/apexd", arg,
- hook_file->GetPath(), // Make the APEX with hook first.
+ mounted_apexes[hook_idx].mount_point, // Make the APEX with hook first.
};
- for (const ApexFile& apex : apexes) {
- if (&apex != hook_file) {
- args.push_back(apex.GetPath());
+ for (size_t i = 0; i < mounted_apexes.size(); i++) {
+ if ((int)i != hook_idx) {
+ args.push_back(mounted_apexes[i].mount_point);
}
}
@@ -149,20 +160,19 @@
std::string hook_path;
{
- auto bind_fn = [&fn, name](const std::string& apex) {
+ auto bind_fn = [&fn, name](const std::string& mount_point) {
std::string hook;
- std::string mount_point;
std::string active_point;
{
- Result<ApexFile> apex_file = ApexFile::Open(apex);
- if (!apex_file) {
- LOG(ERROR) << "Could not open apex " << apex << " for " << name
- << ": " << apex_file.error();
+ Result<ApexManifest> manifest_or =
+ ReadManifest(mount_point + "/" + kManifestFilename);
+ if (!manifest_or) {
+ LOG(ERROR) << "Could not read manifest from " << mount_point
+ << " for " << name << ": " << manifest_or.error();
_exit(202);
}
- const ApexManifest& manifest = apex_file->GetManifest();
+ const auto& manifest = *manifest_or;
hook = (manifest.*fn)();
- mount_point = apexd_private::GetPackageMountPoint(manifest);
active_point = apexd_private::GetActiveMountPoint(manifest);
}
diff --git a/apexd/apexd_prepostinstall.h b/apexd/apexd_prepostinstall.h
index 5347e85..66cd2f5 100644
--- a/apexd/apexd_prepostinstall.h
+++ b/apexd/apexd_prepostinstall.h
@@ -27,10 +27,14 @@
class ApexFile;
+// Temp mounts given apexes and then forks into:
+// apexd --pre-install <mount-point-of-apex-with-hook> [<other-mount-points>]
android::base::Result<void> StagePreInstall(
const std::vector<ApexFile>& apexes);
int RunPreInstall(char** argv);
+// Temp mounts given apexes and then forks into:
+// apexd --post-install <mount-point-of-apex-with-hook> [<other-mount-points>]
android::base::Result<void> StagePostInstall(
const std::vector<ApexFile>& apexes);
int RunPostInstall(char** argv);
diff --git a/apexd/apexd_private.h b/apexd/apexd_private.h
index 8b59c3a..1706e08 100644
--- a/apexd/apexd_private.h
+++ b/apexd/apexd_private.h
@@ -32,19 +32,16 @@
namespace apexd_private {
-using MountedApexData = MountedApexDatabase::MountedApexData;
-
std::string GetPackageMountPoint(const ApexManifest& manifest);
+std::string GetPackageTempMountPoint(const ApexManifest& manifest);
std::string GetActiveMountPoint(const ApexManifest& manifest);
android::base::Result<void> BindMount(const std::string& target,
const std::string& source);
-
-bool IsMounted(const std::string& name, const std::string& full_path);
-
-android::base::Result<void> MountPackage(const ApexFile& apex,
- const std::string& mountPoint);
-android::base::Result<void> UnmountPackage(const ApexFile& apex);
+android::base::Result<MountedApexDatabase::MountedApexData> TempMountPackage(
+ const ApexFile& apex, const std::string& mount_point);
+android::base::Result<void> Unmount(
+ const MountedApexDatabase::MountedApexData& data);
} // namespace apexd_private
} // namespace apex
diff --git a/apexd/apexd_utils.h b/apexd/apexd_utils.h
index 4d35249..d5343c8 100644
--- a/apexd/apexd_utils.h
+++ b/apexd/apexd_utils.h
@@ -31,6 +31,7 @@
#include <android-base/chrono_utils.h>
#include <android-base/logging.h>
#include <android-base/result.h>
+#include <android-base/strings.h>
#include <cutils/android_reboot.h>
#include "string_log.h"
@@ -59,8 +60,10 @@
}
}
+// TODO(ioffe): change to Result<void>?
inline int ForkAndRun(const std::vector<std::string>& args,
std::string* error_msg) {
+ LOG(DEBUG) << "Forking : " << android::base::Join(args, " ");
std::vector<const char*> argv;
argv.resize(args.size() + 1, nullptr);
std::transform(args.begin(), args.end(), argv.begin(),
