OpenNtfsDxe: Fixed several issues in the NTFS driver (#588)

* Incorrect EFI_FILE_PROTOCOL version

The driver implements EFI_FILE_PROTOCOL_REVISION version,
not EFI_FILE_PROTOCOL_REVISION2 version.

* Fix directory reading logic by implementing EFI_FILE_INFO cache

Problem:
 According to the UEFI specification, reading from a directory must
 return zero Size for the EFI_FILE_INFO structure when directory
 entries are exhausted. The original FileReadDir() implementation
 always returned a fixed EFI_FILE_INFO size before reporting
 end-of-directory. This caused fuzzing tests to enter an infinite
 directory iteration loop due to unexpected behavior.

Solution:
 Introduced an EFI_FILE_INFO cache with the following logic:
 1. FileReadDir() caches EFI_FILE_INFO on first read. The cache key
    combines directory path hash (with FNV-1a 64-bit hashing) and
    DirIndex value
 2. When the key matches and buffer size is sufficient:
    - Data is returned from cache
    - Cache is cleared
 3. When the key matches with insufficent buffer size:
    - Cache is preserved for subsequent retries
 4. Key mismatch triggers cache reset

Additional benefits:
 - Eliminates fixed MINIMUM_INFO_LENGTH requirement
 - FileReadDir() now requests only required buffer size

Note:
 A more elegant solution is being considered for future NTFS
 driver improvements, requiring further analysis and testing.

Links:
 FNV Hash: http://www.isthe.com/chongo/tech/comp/fnv/index.html

* Fix memory leak in FileReadDir() function

Function NtfsOppen() allocates additional memory for
file structure. After use it, need to free by call FreeFile()
function.

* Fixed invalid pointer access in ReadAttr() function

Solution:
 - Added a null check for the Current field in ReadAttr() function
 - Added validation for MFT record flags in the InitFile() function

Signed-off-by: Pavel Naberezhnev <pavelnaberezhnev@gmail.com>

Platform/OpenNtfsDxe/Cache.c

@@ -0,0 +1,120 @@
+/** @file
+  Copyright (c) 2025, Pavel Naberezhnev. All rights reserved.
+  SPDX-License-Identifier: BSD-3-Clause
+**/
+
+#include "NTFS.h"
+#include "Helper.h"
+
+#define FNVHASH_PRIME   (0x100000001B3ULL)
+#define FNVHASH_OFFSET  (0xCBF29CE484222325ULL)
+
+STATIC
+UINT64
+FnvHash (
+  IN CONST CHAR16  *Str
+  )
+{
+  UINT64  Hash;
+  UINT64  Char;
+
+  ASSERT (Str != NULL);
+
+  Hash = FNVHASH_OFFSET;
+  while ((Char = (UINT64)*Str) != 0) {
+    Hash = Hash ^ (Char & 0x0FF);
+    Hash = Hash * FNVHASH_PRIME;
+
+    Char = Char >> 8;
+    Hash = Hash ^ (Char & 0x0FF);
+    Hash = Hash * FNVHASH_PRIME;
+
+    ++Str;
+  }
+
+  return Hash;
+}
+
+EFI_STATUS
+NtfsCfiInit (
+  IN EFI_FS  *Fs
+  )
+{
+  ASSERT (Fs != NULL);
+
+  Fs->CFIData = AllocateZeroPool (MINIMUM_INFO_LENGTH);
+  if (Fs->CFIData == NULL) {
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  //
+  // Set init state
+  //
+  Fs->CFIDataSize = 0;
+
+  return EFI_SUCCESS;
+}
+
+VOID
+NtfsCfiFree (
+  IN EFI_FS  *Fs
+  )
+{
+  ASSERT (Fs != NULL);
+
+  if (Fs->CFIData != NULL) {
+    FreePool (Fs->CFIData);
+    Fs->CFIData = NULL;
+  }
+}
+
+VOID
+NtfsCfiPush (
+  IN EFI_FS        *Fs,
+  IN CONST CHAR16  *Path,
+  IN UINTN         Size,
+  IN INT64         DirIndex
+  )
+{
+  ASSERT (Fs != NULL);
+  ASSERT (Path != NULL);
+
+  Fs->CFIHash     = FnvHash (Path);
+  Fs->CFIDataSize = Size;
+  Fs->CFIDirIndex = DirIndex;
+}
+
+INTN
+NtfsCfiPop (
+  IN EFI_FS        *Fs,
+  IN CONST CHAR16  *Path,
+  IN UINTN         Size,
+  IN INT64         DirIndex,
+  OUT VOID         *Buffer
+  )
+{
+  UINT64  Hash;
+
+  ASSERT (Fs != NULL);
+  ASSERT (Path != NULL);
+
+  if (Fs->CFIDataSize == 0) {
+    return 0;
+  }
+
+  Hash = FnvHash (Path);
+  if ((Hash == Fs->CFIHash) && (DirIndex == Fs->CFIDirIndex)) {
+    if (Size < Fs->CFIDataSize) {
+      return -1;
+    }
+
+    CopyMem (Buffer, Fs->CFIData, Fs->CFIDataSize);
+    ZeroMem (Fs->CFIData, Fs->CFIDataSize);
+    Fs->CFIDataSize = 0;
+    return 1;
+  }
+
+  ZeroMem (Fs->CFIData, Fs->CFIDataSize);
+  Fs->CFIDataSize = 0;
+  return 0;
+}

Platform/OpenNtfsDxe/Data.c

@@ -200,7 +200,12 @@ ReadAttr (
   ASSERT (Attr != NULL);
   ASSERT (Dest != NULL);
 
-  Current     = Attr->Current;
+  Current = Attr->Current;
+  if (Current == NULL) {
+    DEBUG ((DEBUG_INFO, "NTFS: Internal error! Incorrect initialization of attribute iterator\n"));
+    return EFI_VOLUME_CORRUPTED;
+  }
+
   Attr->Next  = Attr->Current;
   ClusterSize = Attr->BaseMftRecord->File->FileSystem->ClusterSize;
 

Platform/OpenNtfsDxe/Disc.c

@@ -751,6 +751,13 @@ InitFile (
 
   Record = (FILE_RECORD_HEADER *)File->FileRecord;
 
+  if ((Record->Flags & ~IS_SUPPORTED_FLAGS) != 0) {
+    DEBUG ((DEBUG_INFO, "NTFS: MFT Record 0x%Lx has invalid or unsupported flags\n", RecordNumber));
+    FreePool (File->FileRecord);
+    File->FileRecord = NULL;
+    return EFI_VOLUME_CORRUPTED;
+  }
+
   if ((Record->Flags & IS_IN_USE) == 0) {
     DEBUG ((DEBUG_INFO, "NTFS: MFT Record 0x%Lx is not in use\n", RecordNumber));
     FreePool (File->FileRecord);

Platform/OpenNtfsDxe/Driver.h

@@ -185,8 +185,15 @@ enum {
 /// Table 4.22. File record flags
 ///
 enum {
-  IS_IN_USE      = 0x01,
-  IS_A_DIRECTORY = 0x02,
+  IS_IN_USE       = 0x01,
+  IS_A_DIRECTORY  = 0x02,
+  IS_AN_EXTENSION = 0x04,
+  IS_A_SPECIAL    = 0x08,
+
+  IS_SUPPORTED_FLAGS =    IS_IN_USE
+                       | IS_A_DIRECTORY
+                       | IS_AN_EXTENSION
+                       | IS_A_SPECIAL,
 };
 
 ///
@@ -649,6 +656,14 @@ typedef struct _EFI_FS {
   UINTN                              IndexRecordSize;
   UINTN                              SectorSize;
   UINTN                              ClusterSize;
+
+  //
+  // CFI = Cache for FileInfo
+  //
+  UINTN                              CFIDataSize;
+  VOID                               *CFIData;
+  UINT64                             CFIHash;
+  INT64                              CFIDirIndex;
 } EFI_FS;
 
 typedef struct {

Platform/OpenNtfsDxe/Helper.h

@@ -202,4 +202,31 @@ NtfsToEfiTime (
   UINT64    NtfsTime
   );
 
+EFI_STATUS
+NtfsCfiInit (
+  IN EFI_FS  *Fs
+  );
+
+VOID
+NtfsCfiFree (
+  IN EFI_FS  *Fs
+  );
+
+VOID
+NtfsCfiPush (
+  IN EFI_FS        *Fs,
+  IN CONST CHAR16  *Path,
+  IN UINTN         Size,
+  IN INT64         DirIndex
+  );
+
+INTN
+NtfsCfiPop (
+  IN EFI_FS        *Fs,
+  IN CONST CHAR16  *Path,
+  IN UINTN         Size,
+  IN INT64         DirIndex,
+  OUT VOID         *Buffer
+  );
+
 #endif // HELPER_H

Platform/OpenNtfsDxe/NTFS.c

@@ -267,7 +267,7 @@ NTFSStart (
     return Status;
   }
 
-  Instance->EfiFile.Revision    = EFI_FILE_PROTOCOL_REVISION2;
+  Instance->EfiFile.Revision    = EFI_FILE_PROTOCOL_REVISION;
   Instance->EfiFile.Open        = FileOpen;
   Instance->EfiFile.Close       = FileClose;
   Instance->EfiFile.Delete      = FileDelete;
@@ -286,6 +286,8 @@ NTFSStart (
     return Status;
   }
 
+  NtfsCfiInit (Instance);
+
   Status = gBS->InstallMultipleProtocolInterfaces (
                   &Controller,
                   &gEfiSimpleFileSystemProtocolGuid,
@@ -373,6 +375,7 @@ NTFSStop (
   FreePool (Instance->RootIndex->FileRecord);
   FreePool (Instance->MftStart->FileRecord);
   FreePool (Instance->RootIndex->File);
+  NtfsCfiFree (Instance);
 
   return EFI_SUCCESS;
 }

Platform/OpenNtfsDxe/Open.c

@@ -163,22 +163,13 @@ FileReadDir (
   CHAR16         Path[MAX_PATH];
   EFI_NTFS_FILE  *TmpFile = NULL;
   UINTN          Length;
+  INTN           Res;
 
   ASSERT (File != NULL);
   ASSERT (Size != NULL);
   ASSERT ((Data != NULL) || ((Data == NULL) && (*Size == 0)));
 
-  Info = (EFI_FILE_INFO *)Data;
-
-  if (*Size < MINIMUM_INFO_LENGTH) {
-    *Size = MINIMUM_INFO_LENGTH;
-    return EFI_BUFFER_TOO_SMALL;
-  }
-
   ZeroMem (Path, sizeof (Path));
-  ZeroMem (Data, *Size);
-  Info->Size = *Size;
-
   Status = StrCpyS (Path, MAX_PATH, File->Path);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_INFO, "NTFS: Could not copy string.\n"));
@@ -196,11 +187,35 @@ FileReadDir (
     Length++;
   }
 
+  Res = NtfsCfiPop (
+          File->FileSystem,
+          Path,
+          *Size,
+          (INT64)File->DirIndex,
+          Data
+          );
+
+  if (Res != 0) {
+    if (Res < 0) {
+      *Size = File->FileSystem->CFIDataSize;
+      return EFI_BUFFER_TOO_SMALL;
+    }
+
+    Info  = (EFI_FILE_INFO *)Data;
+    *Size = (UINTN)Info->Size;
+    ++File->DirIndex;
+    return EFI_SUCCESS;
+  }
+
+  ZeroMem (File->FileSystem->CFIData, MINIMUM_INFO_LENGTH);
+  Info       = (EFI_FILE_INFO *)File->FileSystem->CFIData;
+  Info->Size = MINIMUM_INFO_LENGTH;
+
   mIndexCounter = (INT64)File->DirIndex;
   if (Length == 0) {
-    Status = NtfsDir (File->FileSystem, L"/", Data, DIR_HOOK);
+    Status = NtfsDir (File->FileSystem, L"/", File->FileSystem->CFIData, DIR_HOOK);
   } else {
-    Status = NtfsDir (File->FileSystem, File->Path, Data, DIR_HOOK);
+    Status = NtfsDir (File->FileSystem, File->Path, File->FileSystem->CFIData, DIR_HOOK);
   }
 
   if (mIndexCounter >= 0) {
@@ -245,9 +260,18 @@ FileReadDir (
       Info->PhysicalSize = TmpFile->RootFile.DataAttributeSize;
     }
 
+    FreeFile (&TmpFile->RootFile);
     FreePool (TmpFile);
   }
 
+  if (*Size < Info->Size) {
+    *Size = (UINTN)Info->Size;
+    NtfsCfiPush (File->FileSystem, Path, (UINTN)Info->Size, (INT64)File->DirIndex);
+    return EFI_BUFFER_TOO_SMALL;
+  } else {
+    CopyMem (Data, File->FileSystem->CFIData, (UINTN)Info->Size);
+  }
+
   *Size = (UINTN)Info->Size;
   ++File->DirIndex;
 

Platform/OpenNtfsDxe/OpenNtfsDxe.inf

@@ -33,6 +33,7 @@
   Data.c
   Index.c
   Compression.c
+  Cache.c
 
 [Packages]
   MdePkg/MdePkg.dec

Utilities/TestNtfsDxe/Makefile

@@ -6,7 +6,7 @@
 PROJECT = TestNtfsDxe
 PRODUCT = $(PROJECT)$(INFIX)$(SUFFIX)
 OBJS    = $(PROJECT).o
-OBJS    += Compression.o Data.o Disc.o Index.o Info.o NTFS.o Open.o Position.o
+OBJS    += Compression.o Data.o Disc.o Index.o Info.o NTFS.o Open.o Position.o Cache.o
 
 include  ../../User/Makefile