diff --git a/Features/Ext4Pkg/Ext4Dxe/Directory.c b/Features/Ext4Pkg/Ext4Dxe/Directory.c index c85c4df6d5c..7d1b2dcfe52 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 b387ebcd36a..070eb5a9c82 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 1aafc60ab57..db938c25244 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 a3eff2b48a0..021d10b1edf 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 982b19c763d..63cecec1f7c 100644 --- a/Features/Ext4Pkg/Ext4Dxe/Inode.c +++ b/Features/Ext4Pkg/Ext4Dxe/Inode.c @@ -89,7 +89,6 @@ Ext4Read ( IN OUT UINTN *Length ) { - DEBUG ((DEBUG_FS, "[ext4] Ext4Read(%s, Offset %lu, Length %lu)\n", File->FileName, Offset, *Length)); EXT4_INODE *Inode; UINT64 InodeSize; UINT64 CurrentSeek; @@ -116,6 +115,8 @@ Ext4Read ( RemainingRead = *Length; BeenRead = 0; + DEBUG ((DEBUG_FS, "[ext4] Ext4Read(%s, Offset %lu, Length %lu)\n", File->Dentry->Name, Offset, *Length)); + if (Offset > InodeSize) { return EFI_DEVICE_ERROR; } diff --git a/Features/Ext4Pkg/Ext4Dxe/Partition.c b/Features/Ext4Pkg/Ext4Dxe/Partition.c index 2258bac76a4..afa0392024e 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 8231831115f..c321d8c3d86 100644 --- a/Features/Ext4Pkg/Ext4Dxe/Superblock.c +++ b/Features/Ext4Pkg/Ext4Dxe/Superblock.c @@ -167,8 +167,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 +250,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 +258,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; }