Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 04502383 authored by David Anderson's avatar David Anderson Committed by Gerrit Code Review
Browse files

Merge changes Idcb30151,I0f746870 

* changes:
  libsnapshot: Harden merge-in-recovery for factory data resets.
  libsnapshot: Quell error spam during factory data resets.
parents d3ffa8ac 2a55e811

fs_mgr/libfiemap/image_manager.cpp

+9 −2
Original line number Diff line number Diff line
@@ -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];

        }

fs_mgr/libsnapshot/include/libsnapshot/snapshot.h

+2 −2
Original line number Diff line number Diff line
@@ -694,7 +694,7 @@ 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,

    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.

fs_mgr/libsnapshot/snapshot.cpp

+41 −11
Original line number Diff line number Diff line
@@ -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,7 +3283,7 @@ bool SnapshotManager::FinishMergeInRecovery() { 
    return true;

}



bool SnapshotManager::ProcessUpdateStateOnDataWipe(bool allow_forward_merge,

UpdateState SnapshotManager::ProcessUpdateStateOnDataWipe(bool allow_forward_merge,

                                                          const std::function<bool()>& callback) {

    auto slot_number = SlotNumberForSlotSuffix(device_->GetSlotSuffix());

    UpdateState state = ProcessUpdateState(callback);
@@ -3265,7 +3291,7 @@ bool SnapshotManager::ProcessUpdateStateOnDataWipe(bool allow_forward_merge, 
    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) {

fs_mgr/libsnapshot/snapshot_test.cpp

+9 −5
Original line number Diff line number Diff line
@@ -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.

    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());