* [edk2-platforms][PATCH 01/10] Ext4Pkg: Replace SetMem(,,0) with ZeroMem
2022-07-19 12:10 [edk2-platforms][PATCH 00/10] Ext4Pkg: Code security and correctness improvements Savva Mitrofanov
@ 2022-07-19 12:10 ` Savva Mitrofanov
2022-07-19 12:10 ` [edk2-platforms][PATCH 02/10] Ext4Pkg: Change HoleLen type to UINT64 Savva Mitrofanov
` (9 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Savva Mitrofanov @ 2022-07-19 12:10 UTC (permalink / raw)
To: devel; +Cc: Marvin Häuser, Pedro Falcato, Vitaly Cheptsov
Cc: Marvin Häuser <mhaeuser@posteo.de>
Cc: Pedro Falcato <pedro.falcato@gmail.com>
Cc: Vitaly Cheptsov <vit9696@protonmail.com>
Signed-off-by: Savva Mitrofanov <savvamtr@gmail.com>
---
Features/Ext4Pkg/Ext4Dxe/Inode.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Features/Ext4Pkg/Ext4Dxe/Inode.c b/Features/Ext4Pkg/Ext4Dxe/Inode.c
index 831f5946e870..142ee6e3d78a 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Inode.c
+++ b/Features/Ext4Pkg/Ext4Dxe/Inode.c
@@ -158,7 +158,7 @@ Ext4Read (
WasRead = HoleLen > RemainingRead ? RemainingRead : HoleLen;
// Potential improvement: In the future, we could get the file hole's total
// size and memset all that
- SetMem (Buffer, WasRead, 0);
+ ZeroMem (Buffer, WasRead);
} else {
ExtentStartBytes = MultU64x32 (
LShiftU64 (Extent.ee_start_hi, 32) |
@@ -431,7 +431,7 @@ Ext4FileCreateTime (
Inode = File->Inode;
if (!EXT4_INODE_HAS_FIELD (Inode, i_crtime)) {
- SetMem (Time, sizeof (EFI_TIME), 0);
+ ZeroMem (Time, sizeof (EFI_TIME));
return;
}
--
2.37.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [edk2-platforms][PATCH 02/10] Ext4Pkg: Change HoleLen type to UINT64
2022-07-19 12:10 [edk2-platforms][PATCH 00/10] Ext4Pkg: Code security and correctness improvements Savva Mitrofanov
2022-07-19 12:10 ` [edk2-platforms][PATCH 01/10] Ext4Pkg: Replace SetMem(,,0) with ZeroMem Savva Mitrofanov
@ 2022-07-19 12:10 ` Savva Mitrofanov
2022-07-19 12:10 ` [edk2-platforms][PATCH 03/10] Ext4Pkg: Use 32-bit block number in BlockMap Savva Mitrofanov
` (8 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Savva Mitrofanov @ 2022-07-19 12:10 UTC (permalink / raw)
To: devel; +Cc: Marvin Häuser, Pedro Falcato, Vitaly Cheptsov
Set HoleLen to UINT64 to perform safe cast to UINTN in ternary operator
at WasRead assignment
Cc: Marvin Häuser <mhaeuser@posteo.de>
Cc: Pedro Falcato <pedro.falcato@gmail.com>
Cc: Vitaly Cheptsov <vit9696@protonmail.com>
Signed-off-by: Savva Mitrofanov <savvamtr@gmail.com>
---
Features/Ext4Pkg/Ext4Dxe/Inode.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Features/Ext4Pkg/Ext4Dxe/Inode.c b/Features/Ext4Pkg/Ext4Dxe/Inode.c
index 142ee6e3d78a..6416f5388486 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Inode.c
+++ b/Features/Ext4Pkg/Ext4Dxe/Inode.c
@@ -100,7 +100,7 @@ Ext4Read (
EFI_STATUS Status;
BOOLEAN HasBackingExtent;
UINT32 HoleOff;
- UINTN HoleLen;
+ UINT64 HoleLen;
UINT64 ExtentStartBytes;
UINT64 ExtentLengthBytes;
UINT64 ExtentLogicalBytes;
@@ -155,7 +155,7 @@ Ext4Read (
HoleLen = (Ext4GetExtentLength (&Extent) * Partition->BlockSize) - HoleOff;
}
- WasRead = HoleLen > RemainingRead ? RemainingRead : HoleLen;
+ WasRead = HoleLen > RemainingRead ? RemainingRead : (UINTN) HoleLen;
// Potential improvement: In the future, we could get the file hole's total
// size and memset all that
ZeroMem (Buffer, WasRead);
--
2.37.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [edk2-platforms][PATCH 03/10] Ext4Pkg: Use 32-bit block number in BlockMap
2022-07-19 12:10 [edk2-platforms][PATCH 00/10] Ext4Pkg: Code security and correctness improvements Savva Mitrofanov
2022-07-19 12:10 ` [edk2-platforms][PATCH 01/10] Ext4Pkg: Replace SetMem(,,0) with ZeroMem Savva Mitrofanov
2022-07-19 12:10 ` [edk2-platforms][PATCH 02/10] Ext4Pkg: Change HoleLen type to UINT64 Savva Mitrofanov
@ 2022-07-19 12:10 ` Savva Mitrofanov
2022-07-19 12:10 ` [edk2-platforms][PATCH 04/10] Ext4Pkg: Use assertion in Ext4CalculateChecksum Savva Mitrofanov
` (7 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Savva Mitrofanov @ 2022-07-19 12:10 UTC (permalink / raw)
To: devel; +Cc: Marvin Häuser, Pedro Falcato, Vitaly Cheptsov
Replace EXT4_BLOCK_NR with 32-bit EXT2_BLOCK_NR in BlockMap, because we
consider BlockMap is 32-bit fs ext2/3 feature
Cc: Marvin Häuser <mhaeuser@posteo.de>
Cc: Pedro Falcato <pedro.falcato@gmail.com>
Cc: Vitaly Cheptsov <vit9696@protonmail.com>
Signed-off-by: Savva Mitrofanov <savvamtr@gmail.com>
---
Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h | 1 +
Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h | 2 +-
Features/Ext4Pkg/Ext4Dxe/BlockMap.c | 14 ++++++++++----
Features/Ext4Pkg/Ext4Dxe/Extents.c | 3 ++-
4 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h b/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
index a55cd2fa68ad..3aef6f0e5bb4 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
+++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
@@ -463,6 +463,7 @@ typedef struct {
#define EXT4_EXTENT_MAX_INITIALIZED (1 << 15)
typedef UINT64 EXT4_BLOCK_NR;
+typedef UINT32 EXT2_BLOCK_NR;
typedef UINT32 EXT4_INO_NR;
// 2 is always the root inode number in ext4
diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
index b1508482b0a7..b446488b2112 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
+++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
@@ -1165,7 +1165,7 @@ EFI_STATUS
Ext4GetBlocks (
IN EXT4_PARTITION *Partition,
IN EXT4_FILE *File,
- IN EXT4_BLOCK_NR LogicalBlock,
+ IN EXT2_BLOCK_NR LogicalBlock,
OUT EXT4_EXTENT *Extent
);
diff --git a/Features/Ext4Pkg/Ext4Dxe/BlockMap.c b/Features/Ext4Pkg/Ext4Dxe/BlockMap.c
index 1a06ac9fbf86..3d9e16035bee 100644
--- a/Features/Ext4Pkg/Ext4Dxe/BlockMap.c
+++ b/Features/Ext4Pkg/Ext4Dxe/BlockMap.c
@@ -70,7 +70,7 @@ UINTN
Ext4GetBlockPath (
IN CONST EXT4_PARTITION *Partition,
IN UINT32 LogicalBlock,
- OUT EXT4_BLOCK_NR BlockPath[EXT4_MAX_BLOCK_PATH]
+ OUT EXT2_BLOCK_NR BlockPath[EXT4_MAX_BLOCK_PATH]
)
{
// The logic behind the block map is very much like a page table
@@ -213,12 +213,12 @@ EFI_STATUS
Ext4GetBlocks (
IN EXT4_PARTITION *Partition,
IN EXT4_FILE *File,
- IN EXT4_BLOCK_NR LogicalBlock,
+ IN EXT2_BLOCK_NR LogicalBlock,
OUT EXT4_EXTENT *Extent
)
{
EXT4_INODE *Inode;
- EXT4_BLOCK_NR BlockPath[EXT4_MAX_BLOCK_PATH];
+ EXT2_BLOCK_NR BlockPath[EXT4_MAX_BLOCK_PATH];
UINTN BlockPathLength;
UINTN Index;
UINT32 *Buffer;
@@ -272,7 +272,13 @@ Ext4GetBlocks (
}
}
- Ext4GetExtentInBlockMap (Buffer, Partition->BlockSize / sizeof (UINT32), BlockPath[BlockPathLength - 1], Extent);
+ Ext4GetExtentInBlockMap (
+ Buffer,
+ Partition->BlockSize / sizeof (UINT32),
+ BlockPath[BlockPathLength - 1],
+ Extent
+ );
+
FreePool (Buffer);
return EFI_SUCCESS;
diff --git a/Features/Ext4Pkg/Ext4Dxe/Extents.c b/Features/Ext4Pkg/Ext4Dxe/Extents.c
index c3874df71751..c5951f78aa62 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Extents.c
+++ b/Features/Ext4Pkg/Ext4Dxe/Extents.c
@@ -259,7 +259,8 @@ Ext4GetExtent (
if (!(Inode->i_flags & EXT4_EXTENTS_FL)) {
// If this is an older ext2/ext3 filesystem, emulate Ext4GetExtent using the block map
- Status = Ext4GetBlocks (Partition, File, LogicalBlock, Extent);
+ // We cast LogicalBlock to UINT32, considering ext2/3 are 32-bit
+ Status = Ext4GetBlocks (Partition, File, (UINT32) LogicalBlock, Extent);
if (!EFI_ERROR (Status)) {
Ext4CacheExtents (File, Extent, 1);
--
2.37.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [edk2-platforms][PATCH 04/10] Ext4Pkg: Use assertion in Ext4CalculateChecksum
2022-07-19 12:10 [edk2-platforms][PATCH 00/10] Ext4Pkg: Code security and correctness improvements Savva Mitrofanov
` (2 preceding siblings ...)
2022-07-19 12:10 ` [edk2-platforms][PATCH 03/10] Ext4Pkg: Use 32-bit block number in BlockMap Savva Mitrofanov
@ 2022-07-19 12:10 ` Savva Mitrofanov
2022-07-19 12:10 ` [edk2-platforms][PATCH 05/10] Ext4Pkg: Fix compiler warnings Savva Mitrofanov
` (6 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Savva Mitrofanov @ 2022-07-19 12:10 UTC (permalink / raw)
To: devel; +Cc: Marvin Häuser, Pedro Falcato, Vitaly Cheptsov
Replaced UNREACHABLE with ASSERT (FALSE) in case of new checksum
algorithms, due to it is an invariant violation rather than unreachable
path
Cc: Marvin Häuser <mhaeuser@posteo.de>
Cc: Pedro Falcato <pedro.falcato@gmail.com>
Cc: Vitaly Cheptsov <vit9696@protonmail.com>
Signed-off-by: Savva Mitrofanov <savvamtr@gmail.com>
---
Features/Ext4Pkg/Ext4Dxe/Superblock.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Features/Ext4Pkg/Ext4Dxe/Superblock.c b/Features/Ext4Pkg/Ext4Dxe/Superblock.c
index 47fc3a65507a..42762b6aa780 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Superblock.c
+++ b/Features/Ext4Pkg/Ext4Dxe/Superblock.c
@@ -342,7 +342,7 @@ Ext4CalculateChecksum (
// For some reason, EXT4 really likes non-inverted CRC32C checksums, so we stick to that here.
return ~CalculateCrc32c(Buffer, Length, ~InitialValue);
default:
- UNREACHABLE ();
+ ASSERT (FALSE);
return 0;
}
}
--
2.37.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [edk2-platforms][PATCH 05/10] Ext4Pkg: Fix compiler warnings
2022-07-19 12:10 [edk2-platforms][PATCH 00/10] Ext4Pkg: Code security and correctness improvements Savva Mitrofanov
` (3 preceding siblings ...)
2022-07-19 12:10 ` [edk2-platforms][PATCH 04/10] Ext4Pkg: Use assertion in Ext4CalculateChecksum Savva Mitrofanov
@ 2022-07-19 12:10 ` Savva Mitrofanov
2022-07-19 12:10 ` [edk2-platforms][PATCH 06/10] Ext4Pkg: Drop dir entry name_len limit extra check Savva Mitrofanov
` (5 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Savva Mitrofanov @ 2022-07-19 12:10 UTC (permalink / raw)
To: devel; +Cc: Marvin Häuser, Pedro Falcato, Vitaly Cheptsov
Initialize all fields in gExt4BindingProtocol. Fix comparison of integer
expressions of different signedness.
Cc: Marvin Häuser <mhaeuser@posteo.de>
Cc: Pedro Falcato <pedro.falcato@gmail.com>
Cc: Vitaly Cheptsov <vit9696@protonmail.com>
Signed-off-by: Savva Mitrofanov <savvamtr@gmail.com>
---
Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h | 2 +-
Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.c | 10 ++++++----
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h b/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
index 3aef6f0e5bb4..7a19d2f79d53 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
+++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
@@ -338,7 +338,7 @@ STATIC_ASSERT (
#define EXT4_TIND_BLOCK 14
#define EXT4_NR_BLOCKS 15
-#define EXT4_GOOD_OLD_INODE_SIZE 128
+#define EXT4_GOOD_OLD_INODE_SIZE 128U
typedef struct _Ext4_I_OSD2_Linux {
UINT16 l_i_blocks_high;
diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.c b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.c
index 43b9340d3956..2a4f5a7bd0ef 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.c
+++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.c
@@ -260,10 +260,12 @@ Ext4Stop (
EFI_DRIVER_BINDING_PROTOCOL gExt4BindingProtocol =
{
- Ext4IsBindingSupported,
- Ext4Bind,
- Ext4Stop,
- EXT4_DRIVER_VERSION
+ .Supported = Ext4IsBindingSupported,
+ .Start = Ext4Bind,
+ .Stop = Ext4Stop,
+ .Version = EXT4_DRIVER_VERSION,
+ .ImageHandle = NULL,
+ .DriverBindingHandle = NULL
};
/**
--
2.37.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [edk2-platforms][PATCH 06/10] Ext4Pkg: Drop dir entry name_len limit extra check
2022-07-19 12:10 [edk2-platforms][PATCH 00/10] Ext4Pkg: Code security and correctness improvements Savva Mitrofanov
` (4 preceding siblings ...)
2022-07-19 12:10 ` [edk2-platforms][PATCH 05/10] Ext4Pkg: Fix compiler warnings Savva Mitrofanov
@ 2022-07-19 12:10 ` Savva Mitrofanov
2022-07-19 12:10 ` [edk2-platforms][PATCH 07/10] Ext4Pkg: Simplify Ext4RemoveDentry logic Savva Mitrofanov
` (4 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Savva Mitrofanov @ 2022-07-19 12:10 UTC (permalink / raw)
To: devel; +Cc: Marvin Häuser, Pedro Falcato, Vitaly Cheptsov
Field name_len has type CHAR8, while filename limit is 255
(EXT4_NAME_MAX), so because structure EXT4_DIR_ENTRY would be
unchangeable in future, we could drop this check without any
assertions
Cc: Marvin Häuser <mhaeuser@posteo.de>
Cc: Pedro Falcato <pedro.falcato@gmail.com>
Cc: Vitaly Cheptsov <vit9696@protonmail.com>
Signed-off-by: Savva Mitrofanov <savvamtr@gmail.com>
---
Features/Ext4Pkg/Ext4Dxe/Directory.c | 11 -----------
1 file changed, 11 deletions(-)
diff --git a/Features/Ext4Pkg/Ext4Dxe/Directory.c b/Features/Ext4Pkg/Ext4Dxe/Directory.c
index 682f66ad5525..96c84c24243e 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Directory.c
+++ b/Features/Ext4Pkg/Ext4Dxe/Directory.c
@@ -160,17 +160,6 @@ Ext4RetrieveDirent (
return EFI_VOLUME_CORRUPTED;
}
- // Ignore names bigger than our limit.
-
- /* Note: I think having a limit is sane because:
- 1) It's nicer to work with.
- 2) Linux and a number of BSDs also have a filename limit of 255.
- */
- if (Entry->name_len > EXT4_NAME_MAX) {
- BlockOffset += Entry->rec_len;
- continue;
- }
-
// Unused entry
if (Entry->inode == 0) {
BlockOffset += Entry->rec_len;
--
2.37.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [edk2-platforms][PATCH 07/10] Ext4Pkg: Simplify Ext4RemoveDentry logic
2022-07-19 12:10 [edk2-platforms][PATCH 00/10] Ext4Pkg: Code security and correctness improvements Savva Mitrofanov
` (5 preceding siblings ...)
2022-07-19 12:10 ` [edk2-platforms][PATCH 06/10] Ext4Pkg: Drop dir entry name_len limit extra check Savva Mitrofanov
@ 2022-07-19 12:10 ` Savva Mitrofanov
2022-07-19 12:10 ` [edk2-platforms][PATCH 08/10] Ext4Pkg: Fix possible int overflow in Ext4ExtentsMapKeyCompare Savva Mitrofanov
` (3 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Savva Mitrofanov @ 2022-07-19 12:10 UTC (permalink / raw)
To: devel; +Cc: Marvin Häuser, Pedro Falcato, Vitaly Cheptsov
First of all BASE_LIST_FOR_EACH_SAFE doesn't have any sanity checks. So
its usage isn't "safe". We can drop this loop and use just IsNodeInList
Cc: Marvin Häuser <mhaeuser@posteo.de>
Cc: Pedro Falcato <pedro.falcato@gmail.com>
Cc: Vitaly Cheptsov <vit9696@protonmail.com>
Signed-off-by: Savva Mitrofanov <savvamtr@gmail.com>
---
Features/Ext4Pkg/Ext4Dxe/Directory.c | 16 ++--------------
1 file changed, 2 insertions(+), 14 deletions(-)
diff --git a/Features/Ext4Pkg/Ext4Dxe/Directory.c b/Features/Ext4Pkg/Ext4Dxe/Directory.c
index 96c84c24243e..d1038c04926e 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Directory.c
+++ b/Features/Ext4Pkg/Ext4Dxe/Directory.c
@@ -537,20 +537,8 @@ Ext4RemoveDentry (
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"));
+ ASSERT (IsNodeInList (&ToBeRemoved->ListNode, &Parent->Children));
+ RemoveEntryList (&ToBeRemoved->ListNode);
}
/**
--
2.37.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [edk2-platforms][PATCH 08/10] Ext4Pkg: Fix possible int overflow in Ext4ExtentsMapKeyCompare
2022-07-19 12:10 [edk2-platforms][PATCH 00/10] Ext4Pkg: Code security and correctness improvements Savva Mitrofanov
` (6 preceding siblings ...)
2022-07-19 12:10 ` [edk2-platforms][PATCH 07/10] Ext4Pkg: Simplify Ext4RemoveDentry logic Savva Mitrofanov
@ 2022-07-19 12:10 ` Savva Mitrofanov
2022-07-19 12:10 ` [edk2-platforms][PATCH 09/10] Ext4Pkg: Return bad block type in Ext4GetBlockpath Savva Mitrofanov
` (2 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Savva Mitrofanov @ 2022-07-19 12:10 UTC (permalink / raw)
To: devel; +Cc: Marvin Häuser, Pedro Falcato, Vitaly Cheptsov
Cc: Marvin Häuser <mhaeuser@posteo.de>
Cc: Pedro Falcato <pedro.falcato@gmail.com>
Cc: Vitaly Cheptsov <vit9696@protonmail.com>
Signed-off-by: Savva Mitrofanov <savvamtr@gmail.com>
---
Features/Ext4Pkg/Ext4Dxe/Extents.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Features/Ext4Pkg/Ext4Dxe/Extents.c b/Features/Ext4Pkg/Ext4Dxe/Extents.c
index c5951f78aa62..80427d869bd3 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Extents.c
+++ b/Features/Ext4Pkg/Ext4Dxe/Extents.c
@@ -421,7 +421,7 @@ Ext4ExtentsMapKeyCompare (
Extent = UserStruct;
Block = (UINT32)(UINTN)StandaloneKey;
- if ((Block >= Extent->ee_block) && (Block < Extent->ee_block + Ext4GetExtentLength (Extent))) {
+ if ((Block >= Extent->ee_block) && (Block - Extent->ee_block < Ext4GetExtentLength (Extent))) {
return 0;
}
--
2.37.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [edk2-platforms][PATCH 09/10] Ext4Pkg: Return bad block type in Ext4GetBlockpath
2022-07-19 12:10 [edk2-platforms][PATCH 00/10] Ext4Pkg: Code security and correctness improvements Savva Mitrofanov
` (7 preceding siblings ...)
2022-07-19 12:10 ` [edk2-platforms][PATCH 08/10] Ext4Pkg: Fix possible int overflow in Ext4ExtentsMapKeyCompare Savva Mitrofanov
@ 2022-07-19 12:10 ` Savva Mitrofanov
2022-07-19 12:10 ` [edk2-platforms][PATCH 10/10] Ext4Pkg: Group descriptor size must be 4-byte aligned Savva Mitrofanov
2022-07-19 16:19 ` [edk2-platforms][PATCH 00/10] Ext4Pkg: Code security and correctness improvements Pedro Falcato
10 siblings, 0 replies; 12+ messages in thread
From: Savva Mitrofanov @ 2022-07-19 12:10 UTC (permalink / raw)
To: devel; +Cc: Marvin Häuser, Pedro Falcato, Vitaly Cheptsov
Seems that returning maximum uintn in case of bad block was a mistake,
so return just bad block type
Cc: Marvin Häuser <mhaeuser@posteo.de>
Cc: Pedro Falcato <pedro.falcato@gmail.com>
Cc: Vitaly Cheptsov <vit9696@protonmail.com>
Signed-off-by: Savva Mitrofanov <savvamtr@gmail.com>
---
Features/Ext4Pkg/Ext4Dxe/BlockMap.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Features/Ext4Pkg/Ext4Dxe/BlockMap.c b/Features/Ext4Pkg/Ext4Dxe/BlockMap.c
index 3d9e16035bee..2bc629fe9d38 100644
--- a/Features/Ext4Pkg/Ext4Dxe/BlockMap.c
+++ b/Features/Ext4Pkg/Ext4Dxe/BlockMap.c
@@ -123,7 +123,7 @@ Ext4GetBlockPath (
break;
default:
// EXT4_TYPE_BAD_BLOCK
- return -1;
+ break;
}
return Type + 1;
@@ -230,7 +230,7 @@ Ext4GetBlocks (
BlockPathLength = Ext4GetBlockPath (Partition, LogicalBlock, BlockPath);
- if (BlockPathLength == (UINTN)-1) {
+ if (BlockPathLength - 1 == EXT4_TYPE_BAD_BLOCK) {
// Bad logical block (out of range)
return EFI_NO_MAPPING;
}
--
2.37.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [edk2-platforms][PATCH 10/10] Ext4Pkg: Group descriptor size must be 4-byte aligned
2022-07-19 12:10 [edk2-platforms][PATCH 00/10] Ext4Pkg: Code security and correctness improvements Savva Mitrofanov
` (8 preceding siblings ...)
2022-07-19 12:10 ` [edk2-platforms][PATCH 09/10] Ext4Pkg: Return bad block type in Ext4GetBlockpath Savva Mitrofanov
@ 2022-07-19 12:10 ` Savva Mitrofanov
2022-07-19 16:19 ` [edk2-platforms][PATCH 00/10] Ext4Pkg: Code security and correctness improvements Pedro Falcato
10 siblings, 0 replies; 12+ messages in thread
From: Savva Mitrofanov @ 2022-07-19 12:10 UTC (permalink / raw)
To: devel; +Cc: Marvin Häuser, Pedro Falcato, Vitaly Cheptsov
Adds 4-byte aligned check for superblock group descriptor size field
Cc: Marvin Häuser <mhaeuser@posteo.de>
Cc: Pedro Falcato <pedro.falcato@gmail.com>
Cc: Vitaly Cheptsov <vit9696@protonmail.com>
Signed-off-by: Savva Mitrofanov <savvamtr@gmail.com>
---
Features/Ext4Pkg/Ext4Dxe/Directory.c | 2 +-
Features/Ext4Pkg/Ext4Dxe/Superblock.c | 10 +++++-----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/Features/Ext4Pkg/Ext4Dxe/Directory.c b/Features/Ext4Pkg/Ext4Dxe/Directory.c
index d1038c04926e..4441e6d192b6 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Directory.c
+++ b/Features/Ext4Pkg/Ext4Dxe/Directory.c
@@ -74,7 +74,7 @@ Ext4ValidDirent (
}
// Dirent sizes need to be 4 byte aligned
- if (Dirent->rec_len % 4) {
+ if ((Dirent->rec_len % 4) != 0) {
return FALSE;
}
diff --git a/Features/Ext4Pkg/Ext4Dxe/Superblock.c b/Features/Ext4Pkg/Ext4Dxe/Superblock.c
index 42762b6aa780..3bf2e3001f26 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Superblock.c
+++ b/Features/Ext4Pkg/Ext4Dxe/Superblock.c
@@ -257,16 +257,16 @@ Ext4OpenSuperblock (
));
if (EXT4_IS_64_BIT (Partition)) {
+ // s_desc_size should be 4 byte aligned and
+ // 64 bit filesystems need DescSize to be 64 bytes
+ if (((Sb->s_desc_size % 4) != 0) || (Sb->s_desc_size < EXT4_64BIT_BLOCK_DESC_SIZE)) {
+ return EFI_VOLUME_CORRUPTED;
+ }
Partition->DescSize = Sb->s_desc_size;
} else {
Partition->DescSize = EXT4_OLD_BLOCK_DESC_SIZE;
}
- if ((Partition->DescSize < EXT4_64BIT_BLOCK_DESC_SIZE) && EXT4_IS_64_BIT (Partition)) {
- // 64 bit filesystems need DescSize to be 64 bytes
- return EFI_VOLUME_CORRUPTED;
- }
-
if (!Ext4VerifySuperblockChecksum (Partition, Sb)) {
DEBUG ((DEBUG_ERROR, "[ext4] Bad superblock checksum %lx\n", Ext4CalculateSuperblockChecksum (Partition, Sb)));
return EFI_VOLUME_CORRUPTED;
--
2.37.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [edk2-platforms][PATCH 00/10] Ext4Pkg: Code security and correctness improvements
2022-07-19 12:10 [edk2-platforms][PATCH 00/10] Ext4Pkg: Code security and correctness improvements Savva Mitrofanov
` (9 preceding siblings ...)
2022-07-19 12:10 ` [edk2-platforms][PATCH 10/10] Ext4Pkg: Group descriptor size must be 4-byte aligned Savva Mitrofanov
@ 2022-07-19 16:19 ` Pedro Falcato
10 siblings, 0 replies; 12+ messages in thread
From: Pedro Falcato @ 2022-07-19 16:19 UTC (permalink / raw)
To: Savva Mitrofanov
Cc: edk2-devel-groups-io, Marvin Häuser, Vitaly Cheptsov
[-- Attachment #1: Type: text/plain, Size: 1759 bytes --]
For the record, I suggested to Savva (off-list) to squash the patch set
into one, for v2.
On Tue, Jul 19, 2022 at 1:10 PM Savva Mitrofanov <savvamtr@gmail.com> wrote:
> Hi all,
>
> This patchset attempts to improve security of code sections by fixing
> integer overflows,
> missing aligment checks, unsafe casts. Also I simplified some routines,
> fixed compiler warnings and
> corrected some code mistakes.
>
> REF:
> https://github.com/savvamitrofanov/edk2-platforms/commits/ext4pkg_security_improvements
>
> Cc: Marvin Häuser <mhaeuser@posteo.de>
> Cc: Pedro Falcato <pedro.falcato@gmail.com>
> Cc: Vitaly Cheptsov <vit9696@protonmail.com>
>
> Savva Mitrofanov (10):
> Ext4Pkg: Replace SetMem(,,0) with ZeroMem
> Ext4Pkg: Change HoleLen type to UINT64
> Ext4Pkg: Use 32-bit block number in BlockMap
> Ext4Pkg: Use assertion in Ext4CalculateChecksum
> Ext4Pkg: Fix compiler warnings
> Ext4Pkg: Drop dir entry name_len limit extra check
> Ext4Pkg: Simplify Ext4RemoveDentry logic
> Ext4Pkg: Fix possible int overflow in Ext4ExtentsMapKeyCompare
> Ext4Pkg: Return bad block type in Ext4GetBlockpath
> Ext4Pkg: Group descriptor size must be 4-byte aligned
>
> Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h | 3 +-
> Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h | 2 +-
> Features/Ext4Pkg/Ext4Dxe/BlockMap.c | 18 ++++++++----
> Features/Ext4Pkg/Ext4Dxe/Directory.c | 29 ++------------------
> Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.c | 10 ++++---
> Features/Ext4Pkg/Ext4Dxe/Extents.c | 5 ++--
> Features/Ext4Pkg/Ext4Dxe/Inode.c | 8 +++---
> Features/Ext4Pkg/Ext4Dxe/Superblock.c | 12 ++++----
> 8 files changed, 37 insertions(+), 50 deletions(-)
>
> --
> 2.37.0
>
>
--
Pedro Falcato
[-- Attachment #2: Type: text/html, Size: 2558 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread