snapuserd: fix race condition during merge-resume
When resuming a merge after a crash, a race condition can occur where
a block is written to both the scratch space and the base device, but
the merge is not yet committed.
This change ensures that when a merge is resumed, the scratch space is
always checked first. Data from the scratch space is prioritized over
the source block to handle overlapping blocks and XOR ops correctly.
This prevents returning wrong data by ensuring that the most up-to-date
block is used when serving the io.
Bug: 441666433
Test: th, manual OTA and reboots
Change-Id: Idacef9726d3f91a3e416f2b56e531a208d213232
Signed-off-by: Sandeep Dhavale <dhavale@google.com>
diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_core.cpp b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_core.cpp
index 4de0903..9f41993 100644
--- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_core.cpp
+++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_core.cpp
@@ -304,9 +304,9 @@
if (ra_thread_) {
ra_thread_status =
std::async(std::launch::async, &ReadAhead::RunThread, read_ahead_thread_.get());
- // If this is a merge-resume path, wait until RA thread is fully up as
- // the data has to be re-constructed from the scratch space.
- if (resume_merge_ && ShouldReconstructDataFromCow()) {
+ // If the data has to be re-constructed from scratch space,
+ // wait until RA thread is fully up.
+ if (ShouldReconstructDataFromCow()) {
WaitForRaThreadToStart();
}
}
diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_transitions.cpp b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_transitions.cpp
index ea1d8fd..6f4f676 100644
--- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_transitions.cpp
+++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_transitions.cpp
@@ -706,11 +706,13 @@
MERGE_GROUP_STATE state = blk_state->merge_state_;
switch (state) {
case MERGE_GROUP_STATE::GROUP_MERGE_PENDING: {
- // If this is a merge-resume path, check if the data is
- // available from scratch space. Data from scratch space takes
- // higher precedence than from source device for overlapping
- // blocks.
- if (resume_merge_ && GetRABuffer(&lock, new_block, buffer)) {
+ // When resuming a merge after a crash, a block may have been
+ // written to the scratch space and also the base device
+ // but not yet committed via CommitMerge().
+ // We must check for and prioritize this data
+ // from the scratch space over the source block, especially
+ // for overlapping blocks or XOR ops.
+ if (GetRABuffer(&lock, new_block, buffer)) {
return (MERGE_GROUP_STATE::GROUP_MERGE_IN_PROGRESS);
}
blk_state->num_ios_in_progress += 1; // ref count