[edk2-devel] [PATCH 3/3] Ext4Pkg: Add a directory entry tree.
Pedro Falcato
pedro.falcato at gmail.com
Thu Aug 19 21:28:43 UTC 2021
This helps us track directories and directory entries,
which helps us getting a general idea of how the filesystem
looks. In the future, it might serve as a directory cache.
Right now, it only lets us know which name "." and ".." may refer
to, which fixes a EFI_FILE_PROTOCOL::GetInfo() bug that got triggered
by ShellPkg, in some MdePkg code.
Cc: Leif Lindholm <leif at nuviainc.com>
Cc: Michael D Kinney <michael.d.kinney at intel.com>
Cc: Bret Barkelew <Bret.Barkelew at microsoft.com>
Signed-off-by: Pedro Falcato <pedro.falcato at gmail.com>
---
Features/Ext4Pkg/Ext4Dxe/Directory.c | 238 +++++++++++++++++++++-----
Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h | 3 +
Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h | 67 +++++++-
Features/Ext4Pkg/Ext4Dxe/File.c | 24 +--
Features/Ext4Pkg/Ext4Dxe/Inode.c | 2 +-
Features/Ext4Pkg/Ext4Dxe/Partition.c | 7 +
Features/Ext4Pkg/Ext4Dxe/Superblock.c | 24 ++-
7 files changed, 301 insertions(+), 64 deletions(-)
diff --git a/Features/Ext4Pkg/Ext4Dxe/Directory.c b/Features/Ext4Pkg/Ext4Dxe/Directory.c
index c85c4df6d5..7d1b2dcfe5 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Directory.c
+++ b/Features/Ext4Pkg/Ext4Dxe/Directory.c
@@ -194,13 +194,8 @@ Ext4OpenDirent (
)
{
EFI_STATUS Status;
- CHAR16 FileNameBuf[EXT4_NAME_MAX + 1];
+ CHAR16 FileName[EXT4_NAME_MAX + 1];
EXT4_FILE *File;
- CHAR16 *FileName;
- UINTN DestMax;
-
- FileName = FileNameBuf;
- DestMax = ARRAY_SIZE (FileNameBuf);
File = AllocateZeroPool (sizeof (EXT4_FILE));
@@ -209,23 +204,32 @@ Ext4OpenDirent (
goto Error;
}
- Status = Ext4GetUcs2DirentName (Entry, FileNameBuf);
+ Status = Ext4GetUcs2DirentName (Entry, FileName);
if (EFI_ERROR (Status)) {
goto Error;
}
- if (StrCmp (FileNameBuf, L".") == 0) {
- // We're using the parent directory's name
- FileName = Directory->FileName;
- DestMax = StrLen (FileName) + 1;
- }
+ if (StrCmp (FileName, L".") == 0) {
+ // We're using the parent directory's dentry
+ File->Dentry = Directory->Dentry;
- File->FileName = AllocateZeroPool (StrSize (FileName));
+ ASSERT (File->Dentry != NULL);
- if (!File->FileName) {
- Status = EFI_OUT_OF_RESOURCES;
- goto Error;
+ Ext4RefDentry (File->Dentry);
+ } else if (StrCmp (FileName, L"..") == 0) {
+ // Using the parent's parent's dentry
+ File->Dentry = Directory->Dentry->Parent;
+
+ ASSERT (File->Dentry != NULL);
+
+ Ext4RefDentry (File->Dentry);
+ } else {
+ File->Dentry = Ext4CreateDentry (FileName, Directory->Dentry);
+
+ if (!File->Dentry) {
+ goto Error;
+ }
}
Status = Ext4InitExtentsMap (File);
@@ -234,9 +238,6 @@ Ext4OpenDirent (
goto Error;
}
- // This should not fail.
- StrCpyS (File->FileName, DestMax, FileName);
-
File->InodeNum = Entry->inode;
Ext4SetupFile (File, Partition);
@@ -255,8 +256,8 @@ Ext4OpenDirent (
Error:
if (File != NULL) {
- if (File->FileName != NULL) {
- FreePool (File->FileName);
+ if (File->Dentry != NULL) {
+ Ext4UnrefDentry (File->Dentry);
}
if (File->ExtentsMap != NULL) {
@@ -333,52 +334,47 @@ Ext4OpenVolume (
OUT EFI_FILE_PROTOCOL **Root
)
{
- EXT4_INODE *RootInode;
- EFI_STATUS Status;
- EXT4_FILE *RootDir;
+ EXT4_INODE *RootInode;
+ EFI_STATUS Status;
+ EXT4_FILE *RootDir;
+ EXT4_PARTITION *Partition;
+
+ Partition = (EXT4_PARTITION *)This;
- // 2 is always the root inode number in ext4
- Status = Ext4ReadInode ((EXT4_PARTITION *)This, 2, &RootInode);
+ Status = Ext4ReadInode (Partition, EXT4_ROOT_INODE_NR, &RootInode);
if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_ERROR, "[ext4] Could not open root inode - status %x\n", Status));
+ DEBUG ((DEBUG_ERROR, "[ext4] Could not open root inode - error %r\n", Status));
return Status;
}
RootDir = AllocateZeroPool (sizeof (EXT4_FILE));
- if (!RootDir) {
+ if (RootDir == NULL) {
FreePool (RootInode);
return EFI_OUT_OF_RESOURCES;
}
- // The filename will be "\"(null terminated of course)
- RootDir->FileName = AllocateZeroPool (2 * sizeof (CHAR16));
-
- if (!RootDir->FileName) {
- FreePool (RootDir);
- FreePool (RootInode);
- return EFI_OUT_OF_RESOURCES;
- }
-
- RootDir->FileName[0] = L'\\';
-
RootDir->Inode = RootInode;
- RootDir->InodeNum = 2;
+ RootDir->InodeNum = EXT4_ROOT_INODE_NR;
Status = Ext4InitExtentsMap (RootDir);
if (EFI_ERROR (Status)) {
- FreePool (RootDir->FileName);
FreePool (RootInode);
FreePool (RootDir);
return EFI_OUT_OF_RESOURCES;
}
- Ext4SetupFile (RootDir, (EXT4_PARTITION *)This);
+ Ext4SetupFile (RootDir, Partition);
*Root = &RootDir->Protocol;
- InsertTailList (&((EXT4_PARTITION *)This)->OpenFiles, &RootDir->OpenFilesListNode);
+ InsertTailList (&Partition->OpenFiles, &RootDir->OpenFilesListNode);
+
+ ASSERT (Partition->RootDentry != NULL);
+ RootDir->Dentry = Partition->RootDentry;
+
+ Ext4RefDentry (RootDir->Dentry);
return EFI_SUCCESS;
}
@@ -525,3 +521,159 @@ Ext4ReadDir (
Out:
return Status;
}
+
+/**
+ Removes a dentry from the other's list.
+
+ @param[in out] Parent Pointer to the parent EXT4_DENTRY.
+ @param[in out] ToBeRemoved Pointer to the child EXT4_DENTRY.
+**/
+STATIC
+VOID
+Ext4RemoveDentry (
+ IN OUT EXT4_DENTRY *Parent,
+ IN OUT EXT4_DENTRY *ToBeRemoved
+ )
+{
+ EXT4_DENTRY *D;
+ LIST_ENTRY *Entry;
+ LIST_ENTRY *NextEntry;
+
+ BASE_LIST_FOR_EACH_SAFE (Entry, NextEntry, &Parent->Children) {
+ D = EXT4_DENTRY_FROM_DENTRY_LIST (Entry);
+
+ if (D == ToBeRemoved) {
+ RemoveEntryList (Entry);
+ return;
+ }
+ }
+
+ DEBUG ((DEBUG_ERROR, "[ext4] Ext4RemoveDentry did not find the asked-for dentry\n"));
+}
+
+/**
+ Adds a dentry to the other's list.
+
+ The dentry that is added to the other one's list gets ->Parent set to Parent,
+ and the parent gets its reference count incremented.
+
+ @param[in out] Parent Pointer to the parent EXT4_DENTRY.
+ @param[in out] ToBeAdded Pointer to the child EXT4_DENTRY.
+**/
+STATIC
+VOID
+Ext4AddDentry (
+ IN OUT EXT4_DENTRY *Parent,
+ IN OUT EXT4_DENTRY *ToBeAdded
+ )
+{
+ ToBeAdded->Parent = Parent;
+ InsertTailList (&Parent->Children, &ToBeAdded->ListNode);
+ Ext4RefDentry (Parent);
+}
+
+/**
+ Creates a new dentry object.
+
+ @param[in] Name Name of the dentry.
+ @param[in out opt] Parent Parent dentry, if it's not NULL.
+
+ @return The new allocated and initialised dentry.
+ The ref count will be set to 1.
+**/
+EXT4_DENTRY *
+Ext4CreateDentry (
+ IN CONST CHAR16 *Name,
+ IN OUT EXT4_DENTRY *Parent OPTIONAL
+ )
+{
+ EXT4_DENTRY *Dentry;
+ EFI_STATUS Status;
+
+ Dentry = AllocateZeroPool (sizeof (EXT4_DENTRY));
+
+ if (Dentry == NULL) {
+ return NULL;
+ }
+
+ Dentry->RefCount = 1;
+
+ // This StrCpyS should not fail.
+ Status = StrCpyS (Dentry->Name, ARRAY_SIZE (Dentry->Name), Name);
+
+ ASSERT_EFI_ERROR (Status);
+
+ InitializeListHead (&Dentry->Children);
+
+ if (Parent != NULL) {
+ Ext4AddDentry (Parent, Dentry);
+ }
+
+ DEBUG ((DEBUG_FS, "[ext4] Created dentry %s\n", Name));
+
+ return Dentry;
+}
+
+/**
+ Increments the ref count of the dentry.
+
+ @param[in out] Dentry Pointer to a valid EXT4_DENTRY.
+**/
+VOID
+Ext4RefDentry (
+ IN OUT EXT4_DENTRY *Dentry
+ )
+{
+ UINTN OldRef;
+
+ OldRef = Dentry->RefCount;
+
+ Dentry->RefCount++;
+
+ // I'm not sure if this (Refcount overflow) is a valid concern,
+ // but it's better to be safe than sorry.
+ ASSERT (OldRef < Dentry->RefCount);
+}
+
+/**
+ Deletes the dentry.
+
+ @param[in out] Dentry Pointer to a valid EXT4_DENTRY.
+**/
+STATIC
+VOID
+Ext4DeleteDentry (
+ IN OUT EXT4_DENTRY *Dentry
+ )
+{
+ if (Dentry->Parent) {
+ Ext4RemoveDentry (Dentry->Parent, Dentry);
+ Ext4UnrefDentry (Dentry->Parent);
+ }
+
+ DEBUG ((DEBUG_FS, "[ext4] Deleted dentry %s\n", Dentry->Name));
+ FreePool (Dentry);
+}
+
+/**
+ Decrements the ref count of the dentry.
+ If the ref count is 0, it's destroyed.
+
+ @param[in out] Dentry Pointer to a valid EXT4_DENTRY.
+
+ @retval True if it was destroyed, false if it's alive.
+**/
+BOOLEAN
+Ext4UnrefDentry (
+ IN OUT EXT4_DENTRY *Dentry
+ )
+{
+ Dentry->RefCount--;
+
+ if (Dentry->RefCount == 0) {
+ Ext4DeleteDentry (Dentry);
+ return TRUE;
+ }
+
+ return FALSE;
+}
diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h b/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
index b387ebcd36..070eb5a9c8 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
+++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
@@ -451,4 +451,7 @@ typedef struct {
typedef UINT64 EXT4_BLOCK_NR;
typedef UINT32 EXT4_INO_NR;
+// 2 is always the root inode number in ext4
+#define EXT4_ROOT_INODE_NR 2
+
#endif
diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
index 1aafc60ab5..db938c2524 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
+++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
@@ -55,6 +55,7 @@ Ext4OpenPartition (
);
typedef struct _Ext4File EXT4_FILE;
+typedef struct _Ext4_Dentry EXT4_DENTRY;
typedef struct _Ext4_PARTITION {
EFI_SIMPLE_FILE_SYSTEM_PROTOCOL Interface;
@@ -81,8 +82,68 @@ typedef struct _Ext4_PARTITION {
UINT32 InitialSeed;
LIST_ENTRY OpenFiles;
+
+ EXT4_DENTRY *RootDentry;
} EXT4_PARTITION;
+/**
+ This structure represents a directory entry inside our directory entry tree.
+ For now, it will be used as a way to track file names inside our opening code,
+ but it may very well be used as a directory cache in the future.
+ Because it's not being used as a directory cache right now,
+ an EXT4_DENTRY structure is not necessarily unique name-wise in the list of
+ children. Therefore, the dentry tree does not accurately reflect the filesystem
+ structure.
+ */
+struct _Ext4_Dentry {
+ UINTN RefCount;
+ CHAR16 Name[EXT4_NAME_MAX + 1];
+ EXT4_INO_NR Inode;
+ struct _Ext4_Dentry *Parent;
+ LIST_ENTRY Children;
+ LIST_ENTRY ListNode;
+};
+
+#define EXT4_DENTRY_FROM_DENTRY_LIST(Node) BASE_CR (Node, EXT4_DENTRY, ListNode)
+
+/**
+ Creates a new dentry object.
+
+ @param[in] Name Name of the dentry.
+ @param[in out opt] Parent Parent dentry, if it's not NULL.
+
+ @return The new allocated and initialised dentry.
+ The ref count will be set to 1.
+**/
+EXT4_DENTRY *
+Ext4CreateDentry (
+ IN CONST CHAR16 *Name,
+ IN OUT EXT4_DENTRY *Parent OPTIONAL
+ );
+
+/**
+ Increments the ref count of the dentry.
+
+ @param[in out] Dentry Pointer to a valid EXT4_DENTRY.
+**/
+VOID
+Ext4RefDentry (
+ IN OUT EXT4_DENTRY *Dentry
+ );
+
+/**
+ Decrements the ref count of the dentry.
+ If the ref count is 0, it's destroyed.
+
+ @param[in out] Dentry Pointer to a valid EXT4_DENTRY.
+
+ @retval True if it was destroyed, false if it's alive.
+**/
+BOOLEAN
+Ext4UnrefDentry (
+ IN OUT EXT4_DENTRY *Dentry
+ );
+
/**
Opens and parses the superblock.
@@ -126,7 +187,7 @@ Ext4OpenSuperblock (
@param[in] Partition Pointer to the opened ext4 partition.
@return The media ID associated with the partition.
**/
-#define EXT4_MEDIA_ID(Partition) Partition->BlockIo->Media->MediaId
+#define EXT4_MEDIA_ID(Partition) Partition->BlockIo->Media->MediaId
/**
Reads from the partition's disk using the DISK_IO protocol.
@@ -299,11 +360,13 @@ struct _Ext4File {
UINT64 Position;
EXT4_PARTITION *Partition;
- CHAR16 *FileName;
ORDERED_COLLECTION *ExtentsMap;
LIST_ENTRY OpenFilesListNode;
+
+ // Owning reference to this file's directory entry.
+ EXT4_DENTRY *Dentry;
};
#define EXT4_FILE_FROM_OPEN_FILES_NODE(Node) BASE_CR (Node, EXT4_FILE, OpenFilesListNode)
diff --git a/Features/Ext4Pkg/Ext4Dxe/File.c b/Features/Ext4Pkg/Ext4Dxe/File.c
index a3eff2b48a..021d10b1ed 100644
--- a/Features/Ext4Pkg/Ext4Dxe/File.c
+++ b/Features/Ext4Pkg/Ext4Dxe/File.c
@@ -207,7 +207,7 @@ Ext4Open (
FileName += Length;
- if (StrCmp(PathSegment, L".") == 0) {
+ if (StrCmp (PathSegment, L".") == 0) {
// Opens of "." are a no-op
continue;
}
@@ -272,7 +272,7 @@ Ext4Open (
*NewHandle = &Current->Protocol;
- DEBUG ((DEBUG_FS, "Opened filename %s\n", Current->FileName));
+ DEBUG ((DEBUG_FS, "[ext4] Opened filename %s\n", Current->Dentry->Name));
return EFI_SUCCESS;
}
@@ -312,9 +312,9 @@ Ext4CloseInternal (
DEBUG ((DEBUG_FS, "[ext4] Closed file %p (inode %lu)\n", File, File->InodeNum));
RemoveEntryList (&File->OpenFilesListNode);
- FreePool (File->FileName);
FreePool (File->Inode);
Ext4FreeExtentsMap (File);
+ Ext4UnrefDentry (File->Dentry);
FreePool (File);
return EFI_SUCCESS;
}
@@ -522,11 +522,11 @@ Ext4GetFileInfo (
UINTN NeededLength;
CONST CHAR16 *FileName;
- if (File->InodeNum == 2) {
+ if (File->InodeNum == EXT4_ROOT_INODE_NR) {
// Root inode gets a filename of "", regardless of how it was opened.
FileName = L"";
} else {
- FileName = File->FileName;
+ FileName = File->Dentry->Name;
}
FileNameLen = StrLen (FileName);
@@ -717,15 +717,6 @@ Ext4DuplicateFile (
CopyMem (File->Inode, Original->Inode, Partition->InodeSize);
- File->FileName = AllocateZeroPool (StrSize (Original->FileName));
- if (File->FileName == NULL) {
- FreePool (File->Inode);
- FreePool (File);
- return NULL;
- }
-
- StrCpyS (File->FileName, StrLen (Original->FileName) + 1, Original->FileName);
-
File->Position = 0;
Ext4SetupFile (File, Partition);
File->InodeNum = Original->InodeNum;
@@ -733,12 +724,15 @@ Ext4DuplicateFile (
Status = Ext4InitExtentsMap (File);
if (EFI_ERROR (Status)) {
- FreePool (File->FileName);
FreePool (File->Inode);
FreePool (File);
return NULL;
}
+ File->Dentry = Original->Dentry;
+
+ Ext4RefDentry (File->Dentry);
+
InsertTailList (&Partition->OpenFiles, &File->OpenFilesListNode);
return File;
diff --git a/Features/Ext4Pkg/Ext4Dxe/Inode.c b/Features/Ext4Pkg/Ext4Dxe/Inode.c
index 982b19c763..a2f8b4135f 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Inode.c
+++ b/Features/Ext4Pkg/Ext4Dxe/Inode.c
@@ -89,7 +89,7 @@ Ext4Read (
IN OUT UINTN *Length
)
{
- DEBUG ((DEBUG_FS, "[ext4] Ext4Read(%s, Offset %lu, Length %lu)\n", File->FileName, Offset, *Length));
+ DEBUG ((DEBUG_FS, "[ext4] Ext4Read(%s, Offset %lu, Length %lu)\n", File->Dentry->Name, Offset, *Length));
EXT4_INODE *Inode;
UINT64 InodeSize;
UINT64 CurrentSeek;
diff --git a/Features/Ext4Pkg/Ext4Dxe/Partition.c b/Features/Ext4Pkg/Ext4Dxe/Partition.c
index 2258bac76a..afa0392024 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Partition.c
+++ b/Features/Ext4Pkg/Ext4Dxe/Partition.c
@@ -108,6 +108,7 @@ Ext4UnmountAndFreePartition (
LIST_ENTRY *Entry;
LIST_ENTRY *NextEntry;
EXT4_FILE *File;
+ BOOLEAN DeletedRootDentry;
Partition->Unmounting = TRUE;
Ext4CloseInternal (Partition->Root);
@@ -118,6 +119,12 @@ Ext4UnmountAndFreePartition (
Ext4CloseInternal (File);
}
+ DeletedRootDentry = Ext4UnrefDentry (Partition->RootDentry);
+
+ if (!DeletedRootDentry) {
+ DEBUG ((DEBUG_ERROR, "[ext4] Failed to delete root dentry - resource leak present.\n"));
+ }
+
FreePool (Partition->BlockGroups);
FreePool (Partition);
diff --git a/Features/Ext4Pkg/Ext4Dxe/Superblock.c b/Features/Ext4Pkg/Ext4Dxe/Superblock.c
index 8231831115..3efb047dc5 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Superblock.c
+++ b/Features/Ext4Pkg/Ext4Dxe/Superblock.c
@@ -6,6 +6,7 @@
**/
#include "Ext4Dxe.h"
+#include "Uefi/UefiBaseType.h"
STATIC CONST UINT32 gSupportedCompatFeat = EXT4_FEATURE_COMPAT_EXT_ATTR;
@@ -167,8 +168,11 @@ Ext4OpenSuperblock (
// accidentally opening an ext2/3/4 filesystem we don't understand, which would be disasterous.
if (Partition->FeaturesIncompat & ~gSupportedIncompatFeat) {
- DEBUG ((DEBUG_ERROR, "[ext4] Unsupported features %lx\n",
- Partition->FeaturesIncompat & ~gSupportedIncompatFeat));
+ DEBUG ((
+ DEBUG_ERROR,
+ "[ext4] Unsupported features %lx\n",
+ Partition->FeaturesIncompat & ~gSupportedIncompatFeat
+ ));
return EFI_UNSUPPORTED;
}
@@ -247,7 +251,7 @@ Ext4OpenSuperblock (
Partition->BlockGroups = Ext4AllocAndReadBlocks (Partition, NrBlocks, Partition->BlockSize == 1024 ? 2 : 1);
- if (!Partition->BlockGroups) {
+ if (Partition->BlockGroups == NULL) {
return EFI_OUT_OF_RESOURCES;
}
@@ -255,13 +259,27 @@ Ext4OpenSuperblock (
Desc = Ext4GetBlockGroupDesc (Partition, Index);
if (!Ext4VerifyBlockGroupDescChecksum (Partition, Desc, Index)) {
DEBUG ((DEBUG_ERROR, "[ext4] Block group descriptor %u has an invalid checksum\n", Index));
+ FreePool (Partition->BlockGroups);
return EFI_VOLUME_CORRUPTED;
}
}
+ // RootDentry will serve as the basis of our directory entry tree.
+ Partition->RootDentry = Ext4CreateDentry (L"\\", NULL);
+
+ if (Partition->RootDentry == NULL) {
+ FreePool (Partition->BlockGroups);
+ return EFI_OUT_OF_RESOURCES;
+ }
+
// Note that the cast below is completely safe, because EXT4_FILE is a specialisation of EFI_FILE_PROTOCOL
Status = Ext4OpenVolume (&Partition->Interface, (EFI_FILE_PROTOCOL **)&Partition->Root);
+ if (EFI_ERROR (Status)) {
+ Ext4UnrefDentry (Partition->RootDentry);
+ FreePool (Partition->BlockGroups);
+ }
+
return Status;
}
--
2.33.0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#79594): https://edk2.groups.io/g/devel/message/79594
Mute This Topic: https://groups.io/mt/85007624/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list