Revert "FatPkg: Validate Reserved FAT Entries on Volume Open"

This reverts commit 58766a472932c485d41163b1746fb1d9e7984f07.

In edk2 commit 58766a4, validation of the two reserved FAT entries
was added.

However, it also checked the return of FatGetFatEntry to MAX_UINT32,
which is what FatGetFatEntry returns when it encounters an error,
e.g. not being able to read the disk. However, MAX_UINT32 is also a
valid value for the reserved FAT entries and under some conditions
these will be returned in the success case.

A FAT volume formatted with these valid values of the reserved FAT
entries will fail to boot an OS because the opening of the volume
will fail.

However, the reason FatGetFatEntry returns MAX_UINT32 is that most
other uses of the function are comparing it against the END_OF_CHAIN
mark, which MAX_UINT32 will trip and those functions will fail out.

Because this is a critical bug that can prevent OS booting and the
bug the original commit was solving was accounting for a bad FAT
filesystem formatting tool, this commit is reverted for now.

Future work will clean up FatGetFatEntry so that it returns an
EFI_STATUS, but that involves more work and this bug needs to be
resolved in the meantime.

Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>

1 files changed, 9 insertions, 53 deletions

diff --git a/FatPkg/EnhancedFatDxe/Init.c b/FatPkg/EnhancedFatDxe/Init.c
index f020cf703d..9c51ed5b7b 100644
--- a/FatPkg/EnhancedFatDxe/Init.c
+++ b/FatPkg/EnhancedFatDxe/Init.c
@@ -96,6 +96,14 @@ FatAllocateVolume (
   }

 

   //

+  // Initialize cache

+  //

+  Status = FatInitializeDiskCache (Volume);

+  if (EFI_ERROR (Status)) {

+    goto Done;

+  }

+

+  //

   // Install our protocol interfaces on the device's handle

   //

   Status = gBS->InstallMultipleProtocolInterfaces (

@@ -229,7 +237,6 @@ FatOpenDevice (
   UINTN                 SectorsPerFat;

   UINT8                 SectorsPerClusterAlignment;

   UINT8                 BlockAlignment;

-  UINTN                 ReservedFatEntry;

 

   //

   // Read the FAT_BOOT_SECTOR BPB info

@@ -416,58 +423,7 @@ FatOpenDevice (
   // We are now defining FAT Type

   //

   Volume->FatType = FatType;

-

-  //

-  // Initialize cache before we use the helper functions that hit the cache

-  //

-  Status = FatInitializeDiskCache (Volume);

-  if (EFI_ERROR (Status)) {

-    return Status;

-  }

-

-  //

-  // Check the reserved FAT entries to ensure they contain valid values

-  //

-  ReservedFatEntry = FatGetFatEntry (Volume, 0);

-  if (Volume->FatEntryBuffer == MAX_UINT32) {

-    return EFI_VOLUME_CORRUPTED;

-  }

-

-  // Reserved FAT entry 0 should contain the BPB_MEDIA byte value in the low 8 bits with all other bits set to 1

-  switch (FatType) {

-    case Fat12:

-      if ((ReservedFatEntry & FAT_CLUSTER_MASK_FAT12) != ((UINTN)FatBs.FatBsb.Media | 0xF00)) {

-        return EFI_VOLUME_CORRUPTED;

-      }

-

-      break;

-

-    case Fat16:

-      if ((ReservedFatEntry & FAT_CLUSTER_MASK_FAT16) != ((UINTN)FatBs.FatBsb.Media | 0xFF00)) {

-        return EFI_VOLUME_CORRUPTED;

-      }

-

-      break;

-

-    case Fat32:

-      // the upper 4 bits of a FAT32 entry are reserved, so are unchecked here

-      if ((ReservedFatEntry & FAT_CLUSTER_MASK_FAT32) != ((UINTN)FatBs.FatBsb.Media | 0x0FFFFF00)) {

-        return EFI_VOLUME_CORRUPTED;

-      }

-

-      break;

-

-    default:

-      return EFI_VOLUME_CORRUPTED;

-  }

-

-  // Reserved FAT entry 1 should contain the end of chain mark. On FAT16 and FAT32, the high 2 bits may be used as

-  // dirty and hardware error bits, so are ignored in this check, but FatGetFatEntry already ignores them to unify the

-  // logic across FAT types

-  ReservedFatEntry = FatGetFatEntry (Volume, 1);

-  if ((Volume->FatEntryBuffer == MAX_UINT32) || !FAT_END_OF_FAT_CHAIN (ReservedFatEntry)) {

-    return EFI_VOLUME_CORRUPTED;

-  }

+  ASSERT (FatType != FatUndefined);

 

   return EFI_SUCCESS;

 }