ACPR/android_system_core
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge changes Idcb30151,I0f746870

Browse source 
* changes:
  libsnapshot: Harden merge-in-recovery for factory data resets.
  libsnapshot: Quell error spam during factory data resets.
This commit is contained in:
David Anderson 2021-03-02 17:51:57 +00:00 • committed by Gerrit Code Review
commit 04502383f6
4 changed files with 61 additions and 20 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
fs_mgr/libfiemap/image_manager.cpp
View file

@ -16,6 +16,8 @@
#include <libfiemap/image_manager.h>
#include <optional>
#include <android-base/file.h>
#include <android-base/logging.h>
#include <android-base/properties.h>
@ -574,7 +576,7 @@ bool ImageManager::UnmapImageDevice(const std::string& name, bool force) {
return false;
}
auto& dm = DeviceMapper::Instance();
LoopControl loop;
std::optional<LoopControl> loop;
std::string status;
auto status_file = GetStatusFilePath(name);
@ -598,9 +600,14 @@ bool ImageManager::UnmapImageDevice(const std::string& name, bool force) {
return false;
}
} else if (pieces[0] == "loop") {
// Lazily connect to loop-control to avoid spurious errors in recovery.
if (!loop.has_value()) {
loop.emplace();
}
// Failure to remove a loop device is not fatal, since we can still
// remove the backing file if we want.
loop.Detach(pieces[1]);
loop->Detach(pieces[1]);
} else {
LOG(ERROR) << "Unknown status: " << pieces[0];
}

4
fs_mgr/libsnapshot/include/libsnapshot/snapshot.h
View file

@ -694,8 +694,8 @@ class SnapshotManager final : public ISnapshotManager {
// Call ProcessUpdateState and handle states with special rules before data wipe. Specifically,
// if |allow_forward_merge| and allow-forward-merge indicator exists, initiate merge if
// necessary.
bool ProcessUpdateStateOnDataWipe(bool allow_forward_merge,
const std::function<bool()>& callback);
UpdateState ProcessUpdateStateOnDataWipe(bool allow_forward_merge,
const std::function<bool()>& callback);
// Return device string of a mapped image, or if it is not available, the mapped image path.
bool GetMappedImageDeviceStringOrPath(const std::string& device_name,

52
fs_mgr/libsnapshot/snapshot.cpp
View file

@ -894,6 +894,8 @@ UpdateState SnapshotManager::ProcessUpdateState(const std::function<bool()>& cal
const std::function<bool()>& before_cancel) {
while (true) {
UpdateState state = CheckMergeState(before_cancel);
LOG(INFO) << "ProcessUpdateState handling state: " << state;
if (state == UpdateState::MergeFailed) {
AcknowledgeMergeFailure();
}
@ -920,13 +922,15 @@ UpdateState SnapshotManager::CheckMergeState(const std::function<bool()>& before
}
UpdateState state = CheckMergeState(lock.get(), before_cancel);
LOG(INFO) << "CheckMergeState for snapshots returned: " << state;
if (state == UpdateState::MergeCompleted) {
// Do this inside the same lock. Failures get acknowledged without the
// lock, because flock() might have failed.
AcknowledgeMergeSuccess(lock.get());
} else if (state == UpdateState::Cancelled) {
if (!RemoveAllUpdateState(lock.get(), before_cancel)) {
return ReadSnapshotUpdateStatus(lock.get()).state();
if (!device_->IsRecovery() && !RemoveAllUpdateState(lock.get(), before_cancel)) {
LOG(ERROR) << "Failed to remove all update state after acknowleding cancelled update.";
}
}
return state;
@ -968,13 +972,23 @@ UpdateState SnapshotManager::CheckMergeState(LockedFile* lock,
return UpdateState::MergeFailed;
}
auto other_suffix = device_->GetOtherSlotSuffix();
bool cancelled = false;
bool failed = false;
bool merging = false;
bool needs_reboot = false;
bool wrong_phase = false;
for (const auto& snapshot : snapshots) {
if (android::base::EndsWith(snapshot, other_suffix)) {
// This will have triggered an error message in InitiateMerge already.
LOG(INFO) << "Skipping merge validation of unexpected snapshot: " << snapshot;
continue;
}
UpdateState snapshot_state = CheckTargetMergeState(lock, snapshot, update_status);
LOG(INFO) << "CheckTargetMergeState for " << snapshot << " returned: " << snapshot_state;
switch (snapshot_state) {
case UpdateState::MergeFailed:
failed = true;
@ -1173,7 +1187,7 @@ void SnapshotManager::AcknowledgeMergeSuccess(LockedFile* lock) {
// indicator that cleanup is needed on reboot. If a factory data reset
// was requested, it doesn't matter, everything will get wiped anyway.
// To make testing easier we consider a /data wipe as cleaned up.
if (device_->IsRecovery() && !in_factory_data_reset_) {
if (device_->IsRecovery()) {
WriteUpdateState(lock, UpdateState::MergeCompleted);
return;
}
@ -1692,6 +1706,7 @@ UpdateState SnapshotManager::GetUpdateState(double* progress) {
for (const auto& snapshot : snapshots) {
DmTargetSnapshot::Status current_status;
if (!IsSnapshotDevice(snapshot)) continue;
if (!QuerySnapshotStatus(snapshot, nullptr, &current_status)) continue;
fake_snapshots_status.sectors_allocated += current_status.sectors_allocated;
@ -3212,10 +3227,11 @@ bool SnapshotManager::HandleImminentDataWipe(const std::function<void()>& callba
};
in_factory_data_reset_ = true;
bool ok = ProcessUpdateStateOnDataWipe(true /* allow_forward_merge */, process_callback);
UpdateState state =
ProcessUpdateStateOnDataWipe(true /* allow_forward_merge */, process_callback);
in_factory_data_reset_ = false;
if (!ok) {
if (state == UpdateState::MergeFailed) {
return false;
}
@ -3223,6 +3239,16 @@ bool SnapshotManager::HandleImminentDataWipe(const std::function<void()>& callba
if (!UnmapAllPartitionsInRecovery()) {
LOG(ERROR) << "Unable to unmap all partitions; fastboot may fail to flash.";
}
if (state != UpdateState::None) {
auto lock = LockExclusive();
if (!lock) return false;
// Zap the update state so the bootloader doesn't think we're still
// merging. It's okay if this fails, it's informative only at this
// point.
WriteUpdateState(lock.get(), UpdateState::None);
}
return true;
}
@ -3257,15 +3283,15 @@ bool SnapshotManager::FinishMergeInRecovery() {
return true;
}
bool SnapshotManager::ProcessUpdateStateOnDataWipe(bool allow_forward_merge,
const std::function<bool()>& callback) {
UpdateState SnapshotManager::ProcessUpdateStateOnDataWipe(bool allow_forward_merge,
const std::function<bool()>& callback) {
auto slot_number = SlotNumberForSlotSuffix(device_->GetSlotSuffix());
UpdateState state = ProcessUpdateState(callback);
LOG(INFO) << "Update state in recovery: " << state;
switch (state) {
case UpdateState::MergeFailed:
LOG(ERROR) << "Unrecoverable merge failure detected.";
return false;
return state;
case UpdateState::Unverified: {
// If an OTA was just applied but has not yet started merging:
//
@ -3285,8 +3311,12 @@ bool SnapshotManager::ProcessUpdateStateOnDataWipe(bool allow_forward_merge,
if (allow_forward_merge &&
access(GetForwardMergeIndicatorPath().c_str(), F_OK) == 0) {
LOG(INFO) << "Forward merge allowed, initiating merge now.";
return InitiateMerge() &&
ProcessUpdateStateOnDataWipe(false /* allow_forward_merge */, callback);
if (!InitiateMerge()) {
LOG(ERROR) << "Failed to initiate merge on data wipe.";
return UpdateState::MergeFailed;
}
return ProcessUpdateStateOnDataWipe(false /* allow_forward_merge */, callback);
}
LOG(ERROR) << "Reverting to old slot since update will be deleted.";
@ -3304,7 +3334,7 @@ bool SnapshotManager::ProcessUpdateStateOnDataWipe(bool allow_forward_merge,
default:
break;
}
return true;
return state;
}
bool SnapshotManager::EnsureNoOverflowSnapshot(LockedFile* lock) {

14
fs_mgr/libsnapshot/snapshot_test.cpp
View file

@ -636,8 +636,8 @@ TEST_F(SnapshotTest, FlashSuperDuringMerge) {
// Because the status is Merging, we must call ProcessUpdateState, which should
// detect a cancelled update.
ASSERT_EQ(sm->ProcessUpdateState(), UpdateState::Cancelled);
ASSERT_EQ(sm->GetUpdateState(), UpdateState::None);
ASSERT_EQ(init->ProcessUpdateState(), UpdateState::Cancelled);
ASSERT_EQ(init->GetUpdateState(), UpdateState::None);
}
TEST_F(SnapshotTest, UpdateBootControlHal) {
@ -1767,7 +1767,7 @@ TEST_F(SnapshotUpdateTest, DataWipeRollbackInRecovery) {
ASSERT_TRUE(new_sm->HandleImminentDataWipe());
// Manually mount metadata so that we can call GetUpdateState() below.
MountMetadata();
EXPECT_EQ(new_sm->GetUpdateState(), UpdateState::Unverified);
EXPECT_EQ(new_sm->GetUpdateState(), UpdateState::None);
EXPECT_TRUE(test_device->IsSlotUnbootable(1));
EXPECT_FALSE(test_device->IsSlotUnbootable(0));
}
@ -2105,8 +2105,12 @@ TEST_P(FlashAfterUpdateTest, FlashSlotAfterUpdate) {
// There should be no snapshot to merge.
auto new_sm = SnapshotManager::New(new TestDeviceInfo(fake_super, flashed_slot_suffix));
// update_enigne calls ProcessUpdateState first -- should see Cancelled.
ASSERT_EQ(UpdateState::Cancelled, new_sm->ProcessUpdateState());
if (flashed_slot == 0 && after_merge) {
ASSERT_EQ(UpdateState::MergeCompleted, new_sm->ProcessUpdateState());
} else {
// update_engine calls ProcessUpdateState first -- should see Cancelled.
ASSERT_EQ(UpdateState::Cancelled, new_sm->ProcessUpdateState());
}
// Next OTA calls CancelUpdate no matter what.
ASSERT_TRUE(new_sm->CancelUpdate());