FatPkg: Validate Reserved FAT Entries on Volume Open

There are two reserved FAT entries in the FAT filesystem that
are expected to have valid contents in them. Today the FAT drivers
do not validate these entries when reading from a device for the
first time. This can cause infinite loops in the FAT driver when
trying to read corrupted disks as reported in
https://github.com/tianocore/edk2/issues/9679.

This PR follows the recommended update requested in that bug to
check the two reserved FAT entries and validate their contents
against the spec defined values in both FatPei and EnhancedFatDxe
when opening a device for the first time.

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

6 files changed, 147 insertions, 10 deletions

diff --git a/FatPkg/EnhancedFatDxe/Fat.h b/FatPkg/EnhancedFatDxe/Fat.h
index fb6699061e..525393dbd4 100644
--- a/FatPkg/EnhancedFatDxe/Fat.h
+++ b/FatPkg/EnhancedFatDxe/Fat.h
@@ -975,6 +975,22 @@ FatComputeFreeInfo (
   IN FAT_VOLUME  *Volume

   );

 

+/**

+

+  Get the FAT entry value of the volume, which is identified with the Index.

+

+  @param  Volume                - FAT file system volume.

+  @param  Index                 - The index of the FAT entry of the volume.

+

+  @return  The value of the FAT entry.

+

+**/

+UINTN

+FatGetFatEntry (

+  IN FAT_VOLUME  *Volume,

+  IN UINTN       Index

+  );

+

 //

 // Init.c

 //

diff --git a/FatPkg/EnhancedFatDxe/FatFileSystem.h b/FatPkg/EnhancedFatDxe/FatFileSystem.h
index 60b9c56b71..3bad7fceea 100644
--- a/FatPkg/EnhancedFatDxe/FatFileSystem.h
+++ b/FatPkg/EnhancedFatDxe/FatFileSystem.h
@@ -35,6 +35,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #define FAT_CLUSTER_SPECIAL_FAT32  0x0FFFFFF7

 #define FAT_CLUSTER_MASK_FAT12     0xFFF

 #define FAT_CLUSTER_UNMASK_FAT12   0xF000

+#define FAT_CLUSTER_MASK_FAT16     0xFFFF

+#define FAT_CLUSTER_UNMASK_FAT16   0xF0000

 #define FAT_CLUSTER_MASK_FAT32     0x0FFFFFFF

 #define FAT_CLUSTER_UNMASK_FAT32   0xF0000000

 #define FAT_POS_FAT12(a)          ((a) * 3 / 2)

diff --git a/FatPkg/EnhancedFatDxe/FileSpace.c b/FatPkg/EnhancedFatDxe/FileSpace.c
index 909d4980d2..6852c3ea00 100644
--- a/FatPkg/EnhancedFatDxe/FileSpace.c
+++ b/FatPkg/EnhancedFatDxe/FileSpace.c
@@ -80,7 +80,6 @@ FatLoadFatEntry (
   @return  The value of the FAT entry.

 

 **/

-STATIC

 UINTN

 FatGetFatEntry (

   IN FAT_VOLUME  *Volume,

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

 

   //

-  // Initialize cache

-  //

-  Status = FatInitializeDiskCache (Volume);

-  if (EFI_ERROR (Status)) {

-    goto Done;

-  }

-

-  //

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

   //

   Status = gBS->InstallMultipleProtocolInterfaces (

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

   UINT8                 SectorsPerClusterAlignment;

   UINT8                 BlockAlignment;

+  UINTN                 ReservedFatEntry;

 

   //

   // Read the FAT_BOOT_SECTOR BPB info

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

   //

   Volume->FatType = FatType;

-  ASSERT (FatType != FatUndefined);

+

+  //

+  // 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;

+  }

 

   return EFI_SUCCESS;

 }

diff --git a/FatPkg/FatPei/FatLiteAccess.c b/FatPkg/FatPei/FatLiteAccess.c
index 10df4516b2..46a7563ab6 100644
--- a/FatPkg/FatPei/FatLiteAccess.c
+++ b/FatPkg/FatPei/FatLiteAccess.c
@@ -43,6 +43,7 @@ FatGetBpbInfo (
   UINT64                  FatLba;

   UINT64                  RootLba;

   UINT64                  FirstClusterLba;

+  UINT32                  ReservedFatEntries[2];

 

   //

   // Read in the BPB

@@ -167,6 +168,74 @@ FatGetBpbInfo (
     Volume->FatType = Volume->MaxCluster < 4085 ? Fat12 : Fat16;

   }

 

+  //

+  // Read reserved FAT entries which are the first two entries from FatPos

+  //

+  Status = FatReadDisk (

+             PrivateData,

+             Volume->BlockDeviceNo,

+             Volume->FatPos,

+             (Volume->FatType == Fat32) ? sizeof (UINT32) * 2 : sizeof (UINT16) * 2,

+             ReservedFatEntries

+             );

+  if (EFI_ERROR (Status)) {

+    return Status;

+  }

+

+  //

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

+  // 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

+  //

+  switch (Volume->FatType) {

+    case Fat12:

+      // we read two entries and in FAT12, each entry is 12 bits, so we need to shift the first entry by 20 bits to

+      // only read it and not the second entry and beyond

+      if (((ReservedFatEntries[0] >> 20) & FAT_CLUSTER_MASK_FAT12) != ((UINTN)Bpb.Media | 0xF00)) {

+        return EFI_VOLUME_CORRUPTED;

+      }

+

+      // the second entry starts 12 bits in and is 12 bits in length, so we shift by 8 bits to remove the start of the

+      // third entry and then mask to only read the second entry

+      if (!FAT_CLUSTER_END_OF_CHAIN (ReservedFatEntries[0] >> 8)) {

+        return EFI_VOLUME_CORRUPTED;

+      }

+

+      break;

+

+    case Fat16:

+      // in FAT16, each entry is 16 bits, so the first entry is the upper 16 bits of ReservedFatEntries[0]

+      if (((ReservedFatEntries[0] >> 16) & FAT_CLUSTER_MASK_FAT16) != ((UINTN)Bpb.Media | 0xFF00)) {

+        return EFI_VOLUME_CORRUPTED;

+      }

+

+      // the second entry is simply the lower 16 bits of ReservedFatEntries[0], however, we must ignore the upper two

+      // bits. For the purposes of checking if the EOC mark exists, we treat those two bits as 1

+      if (!FAT_CLUSTER_END_OF_CHAIN ((ReservedFatEntries[0] & 0x3FFF) | 0xC000)) {

+        return EFI_VOLUME_CORRUPTED;

+      }

+

+      break;

+

+    case Fat32:

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

+      // FAT32 has 32 bit entries, so the first entry is ReservedFatEntries[0]

+      if ((ReservedFatEntries[0] & FAT_CLUSTER_MASK_FAT32) != ((UINTN)Bpb.Media | 0x0FFFFF00)) {

+        return EFI_VOLUME_CORRUPTED;

+      }

+

+      // the second entry is simply ReservedFatEntries[1], but we must ignore the upper two bits. For the purposes of

+      // checking if the EOC mark exists, we treat those two bits as 1

+      if (!FAT_CLUSTER_END_OF_CHAIN ((ReservedFatEntries[1] & 0x3FFFFFFF) | 0xC0000000)) {

+        return EFI_VOLUME_CORRUPTED;

+      }

+

+      break;

+

+    default:

+      return EFI_VOLUME_CORRUPTED;

+  }

+

   return EFI_SUCCESS;

 }

 

diff --git a/FatPkg/FatPei/FatLiteFmt.h b/FatPkg/FatPei/FatLiteFmt.h
index fef70fe1de..b179b5f95f 100644
--- a/FatPkg/FatPei/FatLiteFmt.h
+++ b/FatPkg/FatPei/FatLiteFmt.h
@@ -27,6 +27,13 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #define FAT_CLUSTER_BAD       (FAT_CLUSTER_SPECIAL)

 #define FAT_CLUSTER_LAST      (-1)

 

+#define FAT_CLUSTER_MASK_FAT12    0xFFF

+#define FAT_CLUSTER_UNMASK_FAT12  0xF000

+#define FAT_CLUSTER_MASK_FAT16    0xFFFF

+#define FAT_CLUSTER_UNMASK_FAT16  0xF0000

+#define FAT_CLUSTER_MASK_FAT32    0x0FFFFFFF

+#define FAT_CLUSTER_UNMASK_FAT32  0xF0000000

+

 #define DELETE_ENTRY_MARK  0xE5

 #define EMPTY_ENTRY_MARK   0x00