Ext4Pkg: Skip zero-sized extents

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 <mhaeuser@posteo.de>
Fixes: d9ceedca6c8f ("Ext4Pkg: Add Ext4Dxe driver.")
Reported-by: Savva Mitrofanov <savvamtr@gmail.com>
Signed-off-by: Pedro Falcato <pedro.falcato@gmail.com>
This commit is contained in:
Pedro Falcato 2023-05-23 23:27:12 +01:00 committed by Savva Mitrofanov
parent 20a4d1a54e
commit 583b54f824
No known key found for this signature in database
GPG Key ID: 774924031750BF64

View File

@ -36,14 +36,17 @@ Ext4CalculateExtentChecksum (
/** /**
Caches a range of extents, by allocating pool memory for each extent and adding it to the tree. 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] File Pointer to the open file.
@param[in] Extents Pointer to an array of extents. @param[in] Extents Pointer to an array of extents.
@param[in] NumberExtents Length of the array. @param[in] NumberExtents Length of the array.
**/
VOID @return Result of the caching
Ext4CacheExtents ( **/
IN EXT4_FILE *File, STATIC
IN CONST EXT4_EXTENT *Extents, EFI_STATUS
Ext4CacheExtents (
IN EXT4_FILE *File,
IN CONST EXT4_EXTENT *Extents,
IN UINT16 NumberExtents IN UINT16 NumberExtents
); );
@ -280,13 +283,19 @@ Ext4GetExtent (
// If this is an older ext2/ext3 filesystem, emulate Ext4GetExtent using the block map // 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, // By specification files using block maps are limited to 2^32 blocks,
// so we can safely cast LogicalBlock to uint32 // so we can safely cast LogicalBlock to uint32
Status = Ext4GetBlocks (Partition, File, (UINT32)LogicalBlock, Extent); Status = Ext4GetBlocks (Partition, File, (UINT32)LogicalBlock, Extent);
if (!EFI_ERROR (Status)) { if (!EFI_ERROR (Status)) {
Ext4CacheExtents (File, Extent, 1); Status = Ext4CacheExtents (File, Extent, 1);
}
if (EFI_ERROR (Status) && (Status != EFI_OUT_OF_RESOURCES)) {
return Status; return Status;
}
Status = EFI_SUCCESS;
}
return Status;
} }
// Slow path, we'll need to read from disk and (try to) cache those extents. // 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 /* 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 * 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. * 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. * Therefore, we shouldn't have any memory issues.
**/ **/
Ext4CacheExtents (File, (EXT4_EXTENT *)(ExtHeader + 1), ExtHeader->eh_entries); Status = Ext4CacheExtents (File, (EXT4_EXTENT *)(ExtHeader + 1), ExtHeader->eh_entries);
Ext = Ext4BinsearchExtentExt (ExtHeader, LogicalBlock); if (EFI_ERROR (Status) && (Status != EFI_OUT_OF_RESOURCES)) {
if (Buffer != NULL) {
FreePool (Buffer);
}
return Status;
}
Ext = Ext4BinsearchExtentExt (ExtHeader, LogicalBlock);
if (!Ext) { if (!Ext) {
if (Buffer != NULL) { if (Buffer != NULL) {
FreePool (Buffer); 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. 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] File Pointer to the open file.
@param[in] Extents Pointer to an array of extents. @param[in] Extents Pointer to an array of extents.
@param[in] NumberExtents Length of the array. @param[in] NumberExtents Length of the array.
**/
VOID @return Result of the caching
Ext4CacheExtents ( **/
IN EXT4_FILE *File, STATIC
IN CONST EXT4_EXTENT *Extents, EFI_STATUS
Ext4CacheExtents (
IN EXT4_FILE *File,
IN CONST EXT4_EXTENT *Extents,
IN UINT16 NumberExtents 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 /* 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. * in which case, future insertions might fail.
*/ */
for (Idx = 0; Idx < NumberExtents; Idx++, Extents++) { for (Idx = 0; Idx < NumberExtents; Idx++, Extents++) {
Extent = AllocatePool (sizeof (EXT4_EXTENT)); if (Extents->ee_len == 0) {
// 0-sized extent, must be corruption
if (Extent == NULL) { return EFI_VOLUME_CORRUPTED;
return; }
}
Extent = AllocatePool (sizeof (EXT4_EXTENT));
CopyMem (Extent, Extents, sizeof (EXT4_EXTENT));
if (Extent == NULL) {
return EFI_OUT_OF_RESOURCES;
}
CopyMem (Extent, Extents, sizeof (EXT4_EXTENT));
Status = OrderedCollectionInsert (File->ExtentsMap, NULL, Extent); Status = OrderedCollectionInsert (File->ExtentsMap, NULL, Extent);
// EFI_ALREADY_STARTED = already exists in the tree. // EFI_ALREADY_STARTED = already exists in the tree.
@ -550,15 +575,17 @@ Ext4CacheExtents (
FreePool (Extent); FreePool (Extent);
if (Status == EFI_ALREADY_STARTED) { if (Status == EFI_ALREADY_STARTED) {
continue; continue;
} }
return; return EFI_SUCCESS;
} }
} }
}
return EFI_SUCCESS;
/** }
/**
Gets an extent from the extents cache of the file. Gets an extent from the extents cache of the file.
@param[in] File Pointer to the open file. @param[in] File Pointer to the open file.