From ba8ffd939159fd8fc9e9d2accb8aa0197249a0db Mon Sep 17 00:00:00 2001
From: "Admiral H. Curtiss" <pikachu025@gmail.com>
Date: Thu, 15 Aug 2019 02:06:29 +0200
Subject: [PATCH] GCMemcard: Check if the Directory's number-of-blocks claim
 for files matches the BAT, and report an error if it doesn't.

---
 Source/Core/Core/HW/GCMemcard/GCMemcard.cpp | 70 +++++++++++++++++++--
 Source/Core/Core/HW/GCMemcard/GCMemcard.h   |  5 ++
 2 files changed, 70 insertions(+), 5 deletions(-)

diff --git a/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp b/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp
index adccd920fa..45a2141353 100644
--- a/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp
+++ b/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp
@@ -43,7 +43,8 @@ bool GCMemcardErrorCode::HasCriticalErrors() const
          Test(GCMemcardValidityIssues::INVALID_CARD_SIZE) ||
          Test(GCMemcardValidityIssues::INVALID_CHECKSUM) ||
          Test(GCMemcardValidityIssues::MISMATCHED_CARD_SIZE) ||
-         Test(GCMemcardValidityIssues::FREE_BLOCK_MISMATCH);
+         Test(GCMemcardValidityIssues::FREE_BLOCK_MISMATCH) ||
+         Test(GCMemcardValidityIssues::DIR_BAT_INCONSISTENT);
 }
 
 bool GCMemcardErrorCode::Test(GCMemcardValidityIssues code) const
@@ -265,10 +266,12 @@ std::pair<GCMemcardErrorCode, std::optional<GCMemcard>> GCMemcard::Open(std::str
   else
     card.m_active_bat = 1;
 
-  // TODO: Do a card-wide corruption check here, such as cross-checking the active Dir's files with
-  // the active BAT's allocated sectors. The GC BIOS does at the very least recognize if a file's
-  // number of used blocks is inconsistent between the two, and will count a card as corrupted if
-  // that is the case.
+  // check for consistency between the active Dir and BAT
+  const GCMemcardErrorCode dir_bat_consistency_error_code =
+      card.GetActiveDirectory().CheckForErrorsWithBat(card.GetActiveBat());
+  error_code |= dir_bat_consistency_error_code;
+  if (error_code.HasCriticalErrors())
+    return std::make_pair(error_code, std::nullopt);
 
   card.m_valid = true;
 
@@ -1623,3 +1626,60 @@ GCMemcardErrorCode Directory::CheckForErrors() const
 
   return error_code;
 }
+
+GCMemcardErrorCode Directory::CheckForErrorsWithBat(const BlockAlloc& bat) const
+{
+  GCMemcardErrorCode error_code;
+
+  for (u8 i = 0; i < DIRLEN; ++i)
+  {
+    const DEntry& entry = m_dir_entries[i];
+    if (entry.m_gamecode == DEntry::UNINITIALIZED_GAMECODE)
+      continue;
+
+    // check if we end up with the same number of blocks when traversing through the BAT using the
+    // given first block
+    const u16 dir_number_of_blocks = entry.m_block_count;
+    const u16 dir_first_block = entry.m_first_block;
+    bool bat_block_count_matches = false;
+    {
+      u16 remaining_blocks = dir_number_of_blocks;
+      u16 current_block = dir_first_block;
+      while (true)
+      {
+        if (remaining_blocks == 0)
+        {
+          // we should be at the last block but haven't seen the last-block BAT indicator yet, file
+          // is larger according to BAT, so we're inconsistent
+          break;
+        }
+        --remaining_blocks;
+        const u16 next_block = bat.GetNextBlock(current_block);
+        if (next_block == 0)
+        {
+          // current block is out-of-range or next block is unallocated, this is definitely wrong
+          break;
+        }
+        if (next_block == 0xFFFF)
+        {
+          // we're at the final block according to the BAT
+          // if there are zero remaining blocks according to the directory we're consistent,
+          // otherwise the file is smaller according to the BAT and we're inconsistent
+          bat_block_count_matches = remaining_blocks == 0;
+          break;
+        }
+        current_block = next_block;
+      }
+    }
+
+    if (!bat_block_count_matches)
+    {
+      error_code.Set(GCMemcardValidityIssues::DIR_BAT_INCONSISTENT);
+      break;
+    }
+  }
+
+  // TODO: We could also check if every allocated BAT block is actually reachable with the files.
+
+  return error_code;
+}
diff --git a/Source/Core/Core/HW/GCMemcard/GCMemcard.h b/Source/Core/Core/HW/GCMemcard/GCMemcard.h
index 50d0913787..c173138028 100644
--- a/Source/Core/Core/HW/GCMemcard/GCMemcard.h
+++ b/Source/Core/Core/HW/GCMemcard/GCMemcard.h
@@ -88,6 +88,7 @@ enum class GCMemcardValidityIssues
   INVALID_CHECKSUM,
   MISMATCHED_CARD_SIZE,
   FREE_BLOCK_MISMATCH,
+  DIR_BAT_INCONSISTENT,
   DATA_IN_UNUSED_AREA,
   COUNT
 };
@@ -302,6 +303,8 @@ struct DEntry
 };
 static_assert(sizeof(DEntry) == DENTRY_SIZE);
 
+struct BlockAlloc;
+
 struct Directory
 {
   // 127 files of 0x40 bytes each
@@ -330,6 +333,8 @@ struct Directory
   std::pair<u16, u16> CalculateChecksums() const;
 
   GCMemcardErrorCode CheckForErrors() const;
+
+  GCMemcardErrorCode CheckForErrorsWithBat(const BlockAlloc& bat) const;
 };
 static_assert(sizeof(Directory) == BLOCK_SIZE);