From e334aab65bfb3d6f43d6ecf929d166a38fae2007 Mon Sep 17 00:00:00 2001 From: Daniel Rosenberg Date: Wed, 10 Oct 2018 18:52:04 -0700 Subject: [PATCH] Switch to exceptions for most cp calls This switches the checkpoint calls that don't need to return a value to return 0 on success, and an error on failure. This will be transalted to exceptions for java binder users. needsCheckpoint and needsRollback still return a boolean value. Test: vdc setCheckpoint 2 then reboot 3 times checking state Bug: 112901762 Change-Id: Idd3b5e6036631562a86d5123f533b86cf3bd6032 --- Checkpoint.cpp | 75 +++++++++++++++++++++++------------- Checkpoint.h | 13 ++++--- VoldNativeService.cpp | 29 ++++++-------- VoldNativeService.h | 10 ++--- binder/android/os/IVold.aidl | 10 ++--- vdc.cpp | 20 +++------- 6 files changed, 82 insertions(+), 75 deletions(-) diff --git a/Checkpoint.cpp b/Checkpoint.cpp index 8f86174..94a78eb 100644 --- a/Checkpoint.cpp +++ b/Checkpoint.cpp @@ -36,6 +36,7 @@ #include #include +using android::binder::Status; using android::hardware::hidl_string; using android::hardware::boot::V1_0::BoolResult; using android::hardware::boot::V1_0::IBootControl; @@ -64,8 +65,8 @@ bool setBowState(std::string const& block_device, std::string const& state) { } // namespace -bool cp_startCheckpoint(int retry) { - if (retry < -1) return false; +Status cp_startCheckpoint(int retry) { + if (retry < -1) return Status::fromExceptionCode(EINVAL, "Retry count must be more than -1"); std::string content = std::to_string(retry + 1); if (retry == -1) { sp module = IBootControl::getService(); @@ -75,21 +76,24 @@ bool cp_startCheckpoint(int retry) { if (module->getSuffix(module->getCurrentSlot(), cb).isOk()) content += " " + suffix; } } - return android::base::WriteStringToFile(content, kMetadataCPFile); + if (!android::base::WriteStringToFile(content, kMetadataCPFile)) + return Status::fromExceptionCode(errno, "Failed to write checkpoint file"); + return Status::ok(); } -bool cp_commitChanges() { +Status cp_commitChanges() { // Must take action for list of mounted checkpointed things here // To do this, we walk the list of mounted file systems. // But we also need to get the matching fstab entries to see // the original flags + std::string err_str; auto fstab_default = std::unique_ptr{ fs_mgr_read_fstab_default(), fs_mgr_free_fstab}; - if (!fstab_default) return false; + if (!fstab_default) return Status::fromExceptionCode(EINVAL, "Failed to get fstab"); auto mounts = std::unique_ptr{ fs_mgr_read_fstab("/proc/mounts"), fs_mgr_free_fstab}; - if (!mounts) return false; + if (!mounts) return Status::fromExceptionCode(EINVAL, "Failed to get /proc/mounts"); // Walk mounted file systems for (int i = 0; i < mounts->num_entries; ++i) { @@ -107,11 +111,14 @@ bool cp_commitChanges() { setBowState(mount_rec->blk_device, "2"); } } - return android::base::RemoveFileIfExists(kMetadataCPFile); + if (android::base::RemoveFileIfExists(kMetadataCPFile, &err_str)) + return Status::fromExceptionCode(errno, err_str.c_str()); + return Status::ok(); } -void cp_abortChanges() { +Status cp_abortChanges() { android_reboot(ANDROID_RB_RESTART2, 0, nullptr); + return Status::ok(); } bool cp_needsRollback() { @@ -148,14 +155,14 @@ bool cp_needsCheckpoint() { return false; } -bool cp_prepareCheckpoint() { +Status cp_prepareCheckpoint() { auto fstab_default = std::unique_ptr{ fs_mgr_read_fstab_default(), fs_mgr_free_fstab}; - if (!fstab_default) return false; + if (!fstab_default) return Status::fromExceptionCode(EINVAL, "Failed to get fstab"); auto mounts = std::unique_ptr{ fs_mgr_read_fstab("/proc/mounts"), fs_mgr_free_fstab}; - if (!mounts) return false; + if (!mounts) return Status::fromExceptionCode(EINVAL, "Failed to get /proc/mounts"); for (int i = 0; i < mounts->num_entries; ++i) { const fstab_rec* mount_rec = &mounts->recs[i]; @@ -181,7 +188,7 @@ bool cp_prepareCheckpoint() { setBowState(mount_rec->blk_device, "1"); } } - return true; + return Status::ok(); } namespace { @@ -261,19 +268,19 @@ void crc32(const void* data, size_t n_bytes, uint32_t* crc) { } // namespace -bool cp_restoreCheckpoint(const std::string& blockDevice) { - LOG(ERROR) << "Restoring checkpoint on " << blockDevice; +Status cp_restoreCheckpoint(const std::string& blockDevice) { + LOG(INFO) << "Restoring checkpoint on " << blockDevice; std::fstream device(blockDevice, std::ios::binary | std::ios::in | std::ios::out); if (!device) { PLOG(ERROR) << "Cannot open " << blockDevice; - return false; + return Status::fromExceptionCode(errno, ("Cannot open " + blockDevice).c_str()); } char buffer[kBlockSize]; device.read(buffer, kBlockSize); log_sector& ls = *(log_sector*)buffer; if (ls.magic != kMagic) { LOG(ERROR) << "No magic"; - return false; + return Status::fromExceptionCode(EINVAL, "No magic"); } LOG(INFO) << "Restoring " << ls.sequence << " log sectors"; @@ -285,12 +292,15 @@ bool cp_restoreCheckpoint(const std::string& blockDevice) { log_sector& ls = *(log_sector*)buffer; if (ls.magic != kMagic) { LOG(ERROR) << "No magic!"; - return false; + return Status::fromExceptionCode(EINVAL, "No magic"); } if ((int)ls.sequence != sequence) { LOG(ERROR) << "Expecting log sector " << sequence << " but got " << ls.sequence; - return false; + return Status::fromExceptionCode( + EINVAL, ("Expecting log sector " + std::to_string(sequence) + " but got " + + std::to_string(ls.sequence)) + .c_str()); } LOG(INFO) << "Restoring from log sector " << ls.sequence; @@ -309,7 +319,7 @@ bool cp_restoreCheckpoint(const std::string& blockDevice) { if (le->checksum && checksum != le->checksum) { LOG(ERROR) << "Checksums don't match " << std::hex << checksum; - return false; + return Status::fromExceptionCode(EINVAL, "Checksums don't match"); } device.seekg(le->source * kSectorSize); @@ -317,20 +327,33 @@ bool cp_restoreCheckpoint(const std::string& blockDevice) { } } - return true; + return Status::ok(); } -bool cp_markBootAttempt() { +Status cp_markBootAttempt() { std::string oldContent, newContent; int retry = 0; - if (!android::base::ReadFileToString(kMetadataCPFile, &oldContent)) return false; + struct stat st; + int result = stat(kMetadataCPFile.c_str(), &st); + + // If the file doesn't exist, we aren't managing a checkpoint retry counter + if (result != 0) return Status::ok(); + if (!android::base::ReadFileToString(kMetadataCPFile, &oldContent)) { + PLOG(ERROR) << "Failed to read checkpoint file"; + return Status::fromExceptionCode(errno, "Failed to read checkpoint file"); + } std::string retryContent = oldContent.substr(0, oldContent.find_first_of(" ")); - if (!android::base::ParseInt(retryContent, &retry)) return false; - if (retry > 0) retry--; + if (!android::base::ParseInt(retryContent, &retry)) + return Status::fromExceptionCode(EINVAL, "Could not parse retry count"); + if (retry > 0) { + retry--; - newContent = std::to_string(retry); - return android::base::WriteStringToFile(newContent, kMetadataCPFile); + newContent = std::to_string(retry); + if (!android::base::WriteStringToFile(newContent, kMetadataCPFile)) + return Status::fromExceptionCode(errno, "Could not write checkpoint file"); + } + return Status::ok(); } } // namespace vold diff --git a/Checkpoint.h b/Checkpoint.h index f3554cc..eac2f94 100644 --- a/Checkpoint.h +++ b/Checkpoint.h @@ -17,26 +17,27 @@ #ifndef _CHECKPOINT_H #define _CHECKPOINT_H +#include #include namespace android { namespace vold { -bool cp_startCheckpoint(int retry); +android::binder::Status cp_startCheckpoint(int retry); -bool cp_commitChanges(); +android::binder::Status cp_commitChanges(); -void cp_abortChanges(); +android::binder::Status cp_abortChanges(); bool cp_needsRollback(); bool cp_needsCheckpoint(); -bool cp_prepareCheckpoint(); +android::binder::Status cp_prepareCheckpoint(); -bool cp_restoreCheckpoint(const std::string& mountPoint); +android::binder::Status cp_restoreCheckpoint(const std::string& mountPoint); -bool cp_markBootAttempt(); +android::binder::Status cp_markBootAttempt(); } // namespace vold } // namespace android diff --git a/VoldNativeService.cpp b/VoldNativeService.cpp index f5cde42..11a9398 100644 --- a/VoldNativeService.cpp +++ b/VoldNativeService.cpp @@ -910,12 +910,11 @@ binder::Status VoldNativeService::destroySandboxForApp(const std::string& packag VolumeManager::Instance()->destroySandboxForApp(packageName, appId, sandboxId, userId)); } -binder::Status VoldNativeService::startCheckpoint(int32_t retry, bool* _aidl_return) { +binder::Status VoldNativeService::startCheckpoint(int32_t retry) { ENFORCE_UID(AID_SYSTEM); ACQUIRE_LOCK; - *_aidl_return = cp_startCheckpoint(retry); - return ok(); + return cp_startCheckpoint(retry); } binder::Status VoldNativeService::needsRollback(bool* _aidl_return) { @@ -934,46 +933,40 @@ binder::Status VoldNativeService::needsCheckpoint(bool* _aidl_return) { return ok(); } -binder::Status VoldNativeService::commitChanges(bool* _aidl_return) { +binder::Status VoldNativeService::commitChanges() { ENFORCE_UID(AID_SYSTEM); ACQUIRE_LOCK; - *_aidl_return = cp_commitChanges(); - return ok(); + return cp_commitChanges(); } -binder::Status VoldNativeService::prepareCheckpoint(bool* _aidl_return) { +binder::Status VoldNativeService::prepareCheckpoint() { ENFORCE_UID(AID_SYSTEM); ACQUIRE_LOCK; - *_aidl_return = cp_prepareCheckpoint(); - return ok(); + return cp_prepareCheckpoint(); } -binder::Status VoldNativeService::restoreCheckpoint(const std::string& mountPoint, - bool* _aidl_return) { +binder::Status VoldNativeService::restoreCheckpoint(const std::string& mountPoint) { ENFORCE_UID(AID_SYSTEM); CHECK_ARGUMENT_PATH(mountPoint); ACQUIRE_LOCK; - *_aidl_return = cp_restoreCheckpoint(mountPoint); - return ok(); + return cp_restoreCheckpoint(mountPoint); } -binder::Status VoldNativeService::markBootAttempt(bool* _aidl_return) { +binder::Status VoldNativeService::markBootAttempt() { ENFORCE_UID(AID_SYSTEM); ACQUIRE_LOCK; - *_aidl_return = cp_markBootAttempt(); - return ok(); + return cp_markBootAttempt(); } binder::Status VoldNativeService::abortChanges() { ENFORCE_UID(AID_SYSTEM); ACQUIRE_LOCK; - cp_abortChanges(); - return ok(); + return cp_abortChanges(); } } // namespace vold diff --git a/VoldNativeService.h b/VoldNativeService.h index 7f1d807..8a3ac00 100644 --- a/VoldNativeService.h +++ b/VoldNativeService.h @@ -121,13 +121,13 @@ class VoldNativeService : public BinderService, public os::Bn binder::Status destroySandboxForApp(const std::string& packageName, int32_t appId, const std::string& sandboxId, int32_t userId); - binder::Status startCheckpoint(int32_t retry, bool* _aidl_return); + binder::Status startCheckpoint(int32_t retry); binder::Status needsCheckpoint(bool* _aidl_return); binder::Status needsRollback(bool* _aidl_return); - binder::Status commitChanges(bool* _aidl_return); - binder::Status prepareCheckpoint(bool* _aidl_return); - binder::Status restoreCheckpoint(const std::string& mountPoint, bool* _aidl_return); - binder::Status markBootAttempt(bool* __aidl_return); + binder::Status commitChanges(); + binder::Status prepareCheckpoint(); + binder::Status restoreCheckpoint(const std::string& mountPoint); + binder::Status markBootAttempt(); binder::Status abortChanges(); }; diff --git a/binder/android/os/IVold.aidl b/binder/android/os/IVold.aidl index 809b9a0..96301b3 100644 --- a/binder/android/os/IVold.aidl +++ b/binder/android/os/IVold.aidl @@ -104,14 +104,14 @@ interface IVold { void destroySandboxForApp(in @utf8InCpp String packageName, int appId, in @utf8InCpp String sandboxId, int userId); - boolean startCheckpoint(int retry); + void startCheckpoint(int retry); boolean needsCheckpoint(); boolean needsRollback(); void abortChanges(); - boolean commitChanges(); - boolean prepareCheckpoint(); - boolean restoreCheckpoint(@utf8InCpp String device); - boolean markBootAttempt(); + void commitChanges(); + void prepareCheckpoint(); + void restoreCheckpoint(@utf8InCpp String device); + void markBootAttempt(); const int ENCRYPTION_FLAG_NO_UI = 4; diff --git a/vdc.cpp b/vdc.cpp index 6efd72c..e971d52 100644 --- a/vdc.cpp +++ b/vdc.cpp @@ -107,10 +107,8 @@ int main(int argc, char** argv) { checkStatus(vold->encryptFstab(args[2])); } else if (args[0] == "checkpoint" && args[1] == "startCheckpoint" && args.size() == 3) { int retry; - bool success = false; if (!android::base::ParseInt(args[2], &retry)) exit(EINVAL); - checkStatus(vold->startCheckpoint(retry, &success)); - return success ? 1 : 0; + checkStatus(vold->startCheckpoint(retry)); } else if (args[0] == "checkpoint" && args[1] == "needsCheckpoint" && args.size() == 2) { bool enabled = false; checkStatus(vold->needsCheckpoint(&enabled)); @@ -120,21 +118,13 @@ int main(int argc, char** argv) { checkStatus(vold->needsRollback(&enabled)); return enabled ? 1 : 0; } else if (args[0] == "checkpoint" && args[1] == "commitChanges" && args.size() == 2) { - bool success = false; - checkStatus(vold->commitChanges(&success)); - return success ? 1 : 0; + checkStatus(vold->commitChanges()); } else if (args[0] == "checkpoint" && args[1] == "prepareCheckpoint" && args.size() == 2) { - bool success = false; - checkStatus(vold->prepareCheckpoint(&success)); - return success ? 1 : 0; + checkStatus(vold->prepareCheckpoint()); } else if (args[0] == "checkpoint" && args[1] == "restoreCheckpoint" && args.size() == 3) { - bool success = false; - checkStatus(vold->restoreCheckpoint(args[2], &success)); - return success ? 1 : 0; + checkStatus(vold->restoreCheckpoint(args[2])); } else if (args[0] == "checkpoint" && args[1] == "markBootAttempt" && args.size() == 2) { - bool success = false; - checkStatus(vold->markBootAttempt(&success)); - return success ? 1 : 0; + checkStatus(vold->markBootAttempt()); } else if (args[0] == "checkpoint" && args[1] == "abortChanges" && args.size() == 2) { checkStatus(vold->abortChanges()); } else {