storage: Remove incorrect assert
write_current_super_block can be called on a fs with a failed
initial_super_block_tr from fs_unknown_super_block_state_all (so
reinitialize will be false) if:
1. transaction_initial_super_block_complete is called to complete the
initial_super_block_tr
2. One of the block writes for initial_super_block_tr's blocks fails
with BLOCK_WRITE_FAILED_UNKNOWN_STATE
The block write failure will fail the initial_super_block_tr, then the
UNKNOWN_STATE will cause a call to fs_unknown_super_block_state_all,
which then calls write_current_super_block(initial_super_block_tr, false).
Test: storage_host_test
Bug: 400931440
Change-Id: I538f71ab3c91f650737552c6c9563f32c99c0142
diff --git a/super.c b/super.c
index 0dba82e..3ade808 100644
--- a/super.c
+++ b/super.c
@@ -370,17 +370,7 @@
* only allow transaction_initial_super_block_complete() to reinitialize
* a failed special transaction after it attempts and fails to write the
* block to disk.
- *
- * Since we pin special superblock entries in the block cache and
- * therefore cannot evict them with normal transactions,
- * transaction_initial_super_block_complete() is the only place we can
- * attempt a special transaction write, and if it fails the transaction
- * is immediately reinitialized. Therefore we should only ever be in a
- * failed state if reinitialize is true (i.e. we are being called from
- * transaction_initial_super_block_complete()).
*/
-
- assert(reinitialize || !fs->initial_super_block_tr->failed);
if (!fs->initial_super_block_tr->failed || !reinitialize) {
return;
}
diff --git a/test/storage_host_test/storage_host_test.c b/test/storage_host_test/storage_host_test.c
index 4d18beb..74aac9a 100644
--- a/test/storage_host_test/storage_host_test.c
+++ b/test/storage_host_test/storage_host_test.c
@@ -705,6 +705,61 @@
test_abort:;
}
+TEST_P(StorageTest, FailedSpecialTransactionWithCommitWrites) {
+ /* Create a transaction with some changes. Make RPMB writes fail. */
+ fail_next_rpmb_writes(1, true);
+ file_test(&_state->tr, __func__, FILE_OPEN_CREATE_EXCLUSIVE, 1, 0, 0, false,
+ 0);
+
+ /* Try to commit the transaction while RPMB writes are failing.
+ * The failed RPMB write leaves the RPMB in unknown state (b/c the rpmb
+ * device has commit_failed_writes set), so an initial_super_block_tr is
+ * created for this filesystem. */
+ transaction_complete(&_state->tr);
+ ASSERT_EQ(true, _state->tr.failed);
+ ASSERT_NE(NULL, _state->tr.fs->initial_super_block_tr);
+ ASSERT_EQ(false, _state->tr.fs->initial_super_block_tr->failed);
+ ASSERT_NE(_state->tr.fs->super_block_version,
+ _state->tr.fs->written_super_block_version);
+ expect_errors(TRUSTY_STORAGE_ERROR_RPMB_COUNTER_MISMATCH_RECOVERED, 1);
+
+ /* Reactivate transaction. RPMB writes are still broken. */
+ transaction_activate(&_state->tr);
+ fail_next_rpmb_writes(1, true);
+ file_test(&_state->tr, __func__, FILE_OPEN_CREATE_EXCLUSIVE, 1, 0, 0, false,
+ 0);
+
+ /* initial_super_block_tr exists, so it has to be committed before the
+ * current transaction. That commit fails because its RPMB write fails,
+ * leaving the filesystem in unknown state (again b/c the rpmb device has
+ * commit_failed_writes set). */
+ block_cache_clean_transaction(&_state->tr);
+ ASSERT_EQ(true, _state->tr.failed);
+ ASSERT_NE(NULL, _state->tr.fs->initial_super_block_tr);
+ /* initial_super_block_tr failed, but
+ * transaction_initial_super_block_complete reinitializes it. */
+ EXPECT_EQ(false, _state->tr.fs->initial_super_block_tr->failed);
+
+ /* Reactivate transaction again. RPMB writes work now. */
+ transaction_activate(&_state->tr);
+ file_test(&_state->tr, __func__, FILE_OPEN_CREATE_EXCLUSIVE, 1, 0, 0, false,
+ 0);
+
+ /* initial_super_block_tr exists, so it has to be committed before the
+ * current transaction. Both transactions succeed. */
+ block_cache_clean_transaction(&_state->tr);
+ ASSERT_EQ(false, _state->tr.failed);
+ EXPECT_EQ(NULL, _state->tr.fs->initial_super_block_tr);
+ transaction_complete(&_state->tr);
+ ASSERT_EQ(false, _state->tr.failed);
+ ASSERT_EQ(NULL, _state->tr.fs->initial_super_block_tr);
+
+test_abort:;
+ if (!_state->tr.failed) {
+ transaction_fail(&_state->tr);
+ }
+}
+
TEST(StorageTest, FlushFailingSpecialTransaction) {
struct transaction td_tr;
struct transaction tp_tr;
