[PATCH] RFC: ext4: Add a few overflow checks in the writing code

[PATCH] RFC: ext4: Add a few overflow checks in the writing code

[Simon Glass]

From: Simon Glass <simon.glass@canonical.com>

Some memory allocations make use of data from the disk, so add some
overflow checks.

Adjust LOG2_BLOCK_SIZE() so it is easier to read.

Note: This is a trial to help figure out the best way to deal with these
sorts of things. Feedback welcome.

Signed-off-by: Simon Glass <simon.glass@canonical.com>
---

 fs/ext4/ext4_write.c | 24 ++++++++++++++++++++----
 include/ext_common.h |  3 +--
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/fs/ext4/ext4_write.c b/fs/ext4/ext4_write.c
index d109ed6e90d..ebb1dad4fc9 100644
--- a/fs/ext4/ext4_write.c
+++ b/fs/ext4/ext4_write.c
@@ -108,8 +108,15 @@ int ext4fs_get_bgdtable(void)
 {
 	int status;
 	struct ext_filesystem *fs = get_fs();
-	int gdsize_total = ROUND(fs->no_blkgrp * fs->gdsize, fs->blksz);
+	size_t alloc_size;
+	int gdsize_total;
+
+	if (__builtin_mul_overflow(fs->no_blkgrp, fs->gdsize, &alloc_size))
+		return -1;
+	gdsize_total = ROUND(alloc_size, fs->blksz);
 	fs->no_blk_pergdt = gdsize_total / fs->blksz;
+	if (!fs->no_blk_pergdt)
+		return -1;
 
 	/* allocate memory for gdtable */
 	fs->gdtable = zalloc(gdsize_total);
@@ -117,7 +124,7 @@ int ext4fs_get_bgdtable(void)
 		return -ENOMEM;
 	/* read the group descriptor table */
 	status = ext4fs_devread((lbaint_t)fs->gdtable_blkno * fs->sect_perblk,
-				0, fs->blksz * fs->no_blk_pergdt, fs->gdtable);
+				0, gdsize_total, fs->gdtable);
 	if (status == 0)
 		goto fail;
 
@@ -599,10 +606,17 @@ int ext4fs_init(void)
 	int i;
 	uint32_t real_free_blocks = 0;
 	struct ext_filesystem *fs = get_fs();
+	size_t alloc_size;
+
+	/* check for a reasonable block size, no more than 64K */
+	if (LOG2_BLOCK_SIZE(ext4fs_root) > 16)
+		goto fail;
 
 	/* populate fs */
 	fs->blksz = EXT2_BLOCK_SIZE(ext4fs_root);
 	fs->sect_perblk = fs->blksz >> fs->dev_desc->log2blksz;
+	if (!fs->sect_perblk)
+		goto fail;
 
 	/* get the superblock */
 	fs->sb = zalloc(SUPERBLOCK_SIZE);
@@ -629,7 +643,9 @@ int ext4fs_init(void)
 	}
 
 	/* load all the available bitmap block of the partition */
-	fs->blk_bmaps = zalloc(fs->no_blkgrp * sizeof(char *));
+	if (__builtin_mul_overflow(fs->no_blkgrp, sizeof(char *), &alloc_size))
+		goto fail;
+	fs->blk_bmaps = zalloc(alloc_size);
 	if (!fs->blk_bmaps)
 		goto fail;
 	for (i = 0; i < fs->no_blkgrp; i++) {
@@ -649,7 +665,7 @@ int ext4fs_init(void)
 	}
 
 	/* load all the available inode bitmap of the partition */
-	fs->inode_bmaps = zalloc(fs->no_blkgrp * sizeof(unsigned char *));
+	fs->inode_bmaps = zalloc(alloc_size);
 	if (!fs->inode_bmaps)
 		goto fail;
 	for (i = 0; i < fs->no_blkgrp; i++) {
diff --git a/include/ext_common.h b/include/ext_common.h
index 6e17fbd2771..35d3a1844eb 100644
--- a/include/ext_common.h
+++ b/include/ext_common.h
@@ -67,8 +67,7 @@ struct cmd_tbl;
 #define EXT2_BLOCK_SIZE(data)	   (1 << LOG2_BLOCK_SIZE(data))
 
 /* Log2 size of ext2 block in bytes.  */
-#define LOG2_BLOCK_SIZE(data)	   (le32_to_cpu		   \
-				    (data->sblock.log2_block_size) \
+#define LOG2_BLOCK_SIZE(data)	(le32_to_cpu((data)->sblock.log2_block_size) \
 				    + EXT2_MIN_BLOCK_LOG_SIZE)
 
 #define EXT2_FT_DIR	2
-- 
2.43.0

base-commit: ea9e72801c4c3e3542afe40f89e7e0822cabfcf8
branch: sec

Re: [PATCH] RFC: ext4: Add a few overflow checks in the writing code

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 818 bytes --]

On Wed, Apr 09, 2025 at 03:02:37PM -0600, Simon Glass wrote:

> From: Simon Glass <simon.glass@canonical.com>
> 
> Some memory allocations make use of data from the disk, so add some
> overflow checks.
> 
> Adjust LOG2_BLOCK_SIZE() so it is easier to read.
> 
> Note: This is a trial to help figure out the best way to deal with these
> sorts of things. Feedback welcome.
> 
> Signed-off-by: Simon Glass <simon.glass@canonical.com>
> ---
> 
>  fs/ext4/ext4_write.c | 24 ++++++++++++++++++++----
>  include/ext_common.h |  3 +--
>  2 files changed, 21 insertions(+), 6 deletions(-)

I'd like to see us grab the current include/linux/overflow.h from the
Linux kernel and make use of that rather than further call the built-ins
directly. But yes, we should do something here in general.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]