From 609600b7ee00cb88e953fe829c55dcac142039e5 Mon Sep 17 00:00:00 2001 From: Pedro Falcato Date: Tue, 23 May 2023 23:27:12 +0100 Subject: [PATCH] Ext4Pkg: Skip zero-sized extents MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Zero-sized extents should not be cached nor considered valid, as they have no meaning. As such, reject them outright when caching extents. This makes it so callers do not need to check for ee_len = 0 before using the result, which, if not done, can result in possible infinite loops. Cc: Marvin Häuser Fixes: d9ceedca6c8f ("Ext4Pkg: Add Ext4Dxe driver.") Reported-by: Savva Mitrofanov Signed-off-by: Pedro Falcato --- Ext4Pkg/Ext4Dxe/Extents.c | 125 +++++++++++++++++++++++--------------- 1 file changed, 76 insertions(+), 49 deletions(-) diff --git a/Ext4Pkg/Ext4Dxe/Extents.c b/Ext4Pkg/Ext4Dxe/Extents.c index b7373948a7..eee8b705f9 100644 --- a/Ext4Pkg/Ext4Dxe/Extents.c +++ b/Ext4Pkg/Ext4Dxe/Extents.c @@ -36,14 +36,17 @@ Ext4CalculateExtentChecksum ( /** Caches a range of extents, by allocating pool memory for each extent and adding it to the tree. - @param[in] File Pointer to the open file. - @param[in] Extents Pointer to an array of extents. - @param[in] NumberExtents Length of the array. -**/ -VOID -Ext4CacheExtents ( - IN EXT4_FILE *File, - IN CONST EXT4_EXTENT *Extents, + @param[in] File Pointer to the open file. + @param[in] Extents Pointer to an array of extents. + @param[in] NumberExtents Length of the array. + + @return Result of the caching +**/ +STATIC +EFI_STATUS +Ext4CacheExtents ( + IN EXT4_FILE *File, + IN CONST EXT4_EXTENT *Extents, IN UINT16 NumberExtents ); @@ -280,13 +283,19 @@ Ext4GetExtent ( // If this is an older ext2/ext3 filesystem, emulate Ext4GetExtent using the block map // By specification files using block maps are limited to 2^32 blocks, // so we can safely cast LogicalBlock to uint32 - Status = Ext4GetBlocks (Partition, File, (UINT32)LogicalBlock, Extent); - - if (!EFI_ERROR (Status)) { - Ext4CacheExtents (File, Extent, 1); - } - - return Status; + Status = Ext4GetBlocks (Partition, File, (UINT32)LogicalBlock, Extent); + + if (!EFI_ERROR (Status)) { + Status = Ext4CacheExtents (File, Extent, 1); + + if (EFI_ERROR (Status) && (Status != EFI_OUT_OF_RESOURCES)) { + return Status; + } + + Status = EFI_SUCCESS; + } + + return Status; } // Slow path, we'll need to read from disk and (try to) cache those extents. @@ -360,13 +369,21 @@ Ext4GetExtent ( /* We try to cache every extent under a single leaf, since it's quite likely that we * may need to access things sequentially. Furthermore, ext4 block allocation as done - * by linux (and possibly other systems) is quite fancy and usually it results in a small number of extents. - * Therefore, we shouldn't have any memory issues. - **/ - Ext4CacheExtents (File, (EXT4_EXTENT *)(ExtHeader + 1), ExtHeader->eh_entries); - - Ext = Ext4BinsearchExtentExt (ExtHeader, LogicalBlock); - + * by linux (and possibly other systems) is quite fancy and usually it results in a small number of extents. + * Therefore, we shouldn't have any memory issues. + **/ + Status = Ext4CacheExtents (File, (EXT4_EXTENT *)(ExtHeader + 1), ExtHeader->eh_entries); + + if (EFI_ERROR (Status) && (Status != EFI_OUT_OF_RESOURCES)) { + if (Buffer != NULL) { + FreePool (Buffer); + } + + return Status; + } + + Ext = Ext4BinsearchExtentExt (ExtHeader, LogicalBlock); + if (!Ext) { if (Buffer != NULL) { FreePool (Buffer); @@ -516,14 +533,17 @@ Ext4FreeExtentsMap ( /** Caches a range of extents, by allocating pool memory for each extent and adding it to the tree. - @param[in] File Pointer to the open file. - @param[in] Extents Pointer to an array of extents. - @param[in] NumberExtents Length of the array. -**/ -VOID -Ext4CacheExtents ( - IN EXT4_FILE *File, - IN CONST EXT4_EXTENT *Extents, + @param[in] File Pointer to the open file. + @param[in] Extents Pointer to an array of extents. + @param[in] NumberExtents Length of the array. + + @return Result of the caching +**/ +STATIC +EFI_STATUS +Ext4CacheExtents ( + IN EXT4_FILE *File, + IN CONST EXT4_EXTENT *Extents, IN UINT16 NumberExtents ) { @@ -533,16 +553,21 @@ Ext4CacheExtents ( /* Note that any out of memory condition might mean we don't get to cache a whole leaf of extents * in which case, future insertions might fail. - */ - - for (Idx = 0; Idx < NumberExtents; Idx++, Extents++) { - Extent = AllocatePool (sizeof (EXT4_EXTENT)); - - if (Extent == NULL) { - return; - } - - CopyMem (Extent, Extents, sizeof (EXT4_EXTENT)); + */ + + for (Idx = 0; Idx < NumberExtents; Idx++, Extents++) { + if (Extents->ee_len == 0) { + // 0-sized extent, must be corruption + return EFI_VOLUME_CORRUPTED; + } + + Extent = AllocatePool (sizeof (EXT4_EXTENT)); + + if (Extent == NULL) { + return EFI_OUT_OF_RESOURCES; + } + + CopyMem (Extent, Extents, sizeof (EXT4_EXTENT)); Status = OrderedCollectionInsert (File->ExtentsMap, NULL, Extent); // EFI_ALREADY_STARTED = already exists in the tree. @@ -550,15 +575,17 @@ Ext4CacheExtents ( FreePool (Extent); if (Status == EFI_ALREADY_STARTED) { - continue; - } - - return; - } - } -} - -/** + continue; + } + + return EFI_SUCCESS; + } + } + + return EFI_SUCCESS; +} + +/** Gets an extent from the extents cache of the file. @param[in] File Pointer to the open file.