From 44fd7f61667d3ad2bc7a39dbfb5b5af1e5402ebb Mon Sep 17 00:00:00 2001 From: Yifan Hong Date: Fri, 15 Nov 2019 00:33:11 -0800 Subject: [PATCH] libsnapshot: bootloader rejects wipe in proper time. Calls into HAL first to reject wipes early. Otherwise, there may be a small window where wipes needs to be rejected but bootloader doesn't know about it. Consider the following flow in existing code: 1. sets file to merging 2. devices crashes / shuts down before calling into HAL 3. first-stage init maps dm-snapshot-merge 4. reboot into fastbootd / bootloader 5. wipe At this point, bootloader / fastbootd won't know that merge has already taken place. Reorder so that snapshotctl notifies bootloader before writing the file. When switching from merging back to none: 0. merge has completed 1. sets file to none 2. devices crashes / shuts down before calling into HAL 3. first-stage init maps dm-linear 4. reboot into fastbootd / bootloader and wipe (fail) 5. reboot, snapshotctl resets state to none (calls into HAL) 6. reboot into fastbootd / bootloader and wipe (successful) Test: libsnapshot_test Change-Id: I2b430049c79bf1a751167db7fce74502ac26490a --- fs_mgr/libsnapshot/snapshot.cpp | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp index 1de700861..d1b1cb45a 100644 --- a/fs_mgr/libsnapshot/snapshot.cpp +++ b/fs_mgr/libsnapshot/snapshot.cpp @@ -1663,10 +1663,6 @@ bool SnapshotManager::WriteUpdateState(LockedFile* file, UpdateState state) { if (contents.empty()) return false; if (!Truncate(file)) return false; - if (!android::base::WriteStringToFd(contents, file->fd())) { - PLOG(ERROR) << "Could not write to state file"; - return false; - } #ifdef LIBSNAPSHOT_USE_HAL auto merge_status = MergeStatus::UNKNOWN; @@ -1692,7 +1688,21 @@ bool SnapshotManager::WriteUpdateState(LockedFile* file, UpdateState state) { LOG(ERROR) << "Unexpected update status: " << state; break; } - if (!device_->SetBootControlMergeStatus(merge_status)) { + + bool set_before_write = + merge_status == MergeStatus::SNAPSHOTTED || merge_status == MergeStatus::MERGING; + if (set_before_write && !device_->SetBootControlMergeStatus(merge_status)) { + return false; + } +#endif + + if (!android::base::WriteStringToFd(contents, file->fd())) { + PLOG(ERROR) << "Could not write to state file"; + return false; + } + +#ifdef LIBSNAPSHOT_USE_HAL + if (!set_before_write && !device_->SetBootControlMergeStatus(merge_status)) { return false; } #endif @@ -2150,6 +2160,15 @@ std::unique_ptr SnapshotManager::EnsureMetadataMounted() { } UpdateState SnapshotManager::InitiateMergeAndWait() { + { + auto lock = LockExclusive(); + // Sync update state from file with bootloader. + if (!WriteUpdateState(lock.get(), ReadUpdateState(lock.get()))) { + LOG(WARNING) << "Unable to sync write update state, fastboot may " + << "reject / accept wipes incorrectly!"; + } + } + LOG(INFO) << "Waiting for any previous merge request to complete. " << "This can take up to several minutes."; auto state = ProcessUpdateState();