Skip to content

Commit

Permalink
FatPkg: Validate Reserved FAT Entries on Volume Open
Browse files Browse the repository at this point in the history
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
#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 <[email protected]>
  • Loading branch information
os-d committed Jan 10, 2025
1 parent 42a1418 commit bcb47aa
Show file tree
Hide file tree
Showing 6 changed files with 141 additions and 10 deletions.
16 changes: 16 additions & 0 deletions FatPkg/EnhancedFatDxe/Fat.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
//
Expand Down
2 changes: 2 additions & 0 deletions FatPkg/EnhancedFatDxe/FatFileSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 0 additions & 1 deletion FatPkg/EnhancedFatDxe/FileSpace.c
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ FatLoadFatEntry (
@return The value of the FAT entry.
**/
STATIC
UINTN
FatGetFatEntry (
IN FAT_VOLUME *Volume,
Expand Down
59 changes: 50 additions & 9 deletions FatPkg/EnhancedFatDxe/Init.c
Original file line number Diff line number Diff line change
Expand Up @@ -95,14 +95,6 @@ FatAllocateVolume (
goto Done;
}

//
// Initialize cache
//
Status = FatInitializeDiskCache (Volume);
if (EFI_ERROR (Status)) {
goto Done;
}

//
// Install our protocol interfaces on the device's handle
//
Expand Down Expand Up @@ -237,6 +229,7 @@ FatOpenDevice (
UINTN SectorsPerFat;
UINT8 SectorsPerClusterAlignment;
UINT8 BlockAlignment;
UINTN ReservedFatEntry;

//
// Read the FAT_BOOT_SECTOR BPB info
Expand Down Expand Up @@ -423,7 +416,55 @@ 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 ((ReservedFatEntry == (UINT32)-1) || (Volume->FatEntryBuffer == (UINT32)-1)) {
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;
}

// 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 ((ReservedFatEntry == (UINT32)-1) || (Volume->FatEntryBuffer == (UINT32)-1) || !FAT_END_OF_FAT_CHAIN (ReservedFatEntry)) {
return EFI_VOLUME_CORRUPTED;
}

return EFI_SUCCESS;
}
66 changes: 66 additions & 0 deletions FatPkg/FatPei/FatLiteAccess.c
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ FatGetBpbInfo (
UINT64 FatLba;
UINT64 RootLba;
UINT64 FirstClusterLba;
UINT32 ReservedFatEntries[2];

//
// Read in the BPB
Expand Down Expand Up @@ -167,6 +168,71 @@ 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;
}

return EFI_SUCCESS;
}

Expand Down
7 changes: 7 additions & 0 deletions FatPkg/FatPei/FatLiteFmt.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit bcb47aa

Please sign in to comment.