[PATCH] fs/ext2: Rework out-of-bounds read for inline and external extents

[PATCH] fs/ext2: Rework out-of-bounds read for inline and external extents

[Michael Chang via Grub-devel]

Previously, the number of extent entries was not properly capped based
on the actual available space. This could lead to insufficient reads for
external extents, since the computation was based solely on the inline
extent layout.

In this patch, when processing the extent header, we determine whether
the header is stored inline (i.e., at inode->blocks.dir_blocks) or in an
external extent block. We then clamp the number of entries accordingly
(using max_inline_ext for inline extents and max_external_ext for
external extent blocks).

This change ensures that only the valid number of extent entries is
processed, preventing out-of-bound reads and potential filesystem
corruption.

Fixes: 7e2f750f0a (fs/ext2: Fix out-of-bounds read for inline extents)

Signed-off-by: Michael Chang <mchang@suse.com>
---
 grub-core/fs/ext2.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/grub-core/fs/ext2.c b/grub-core/fs/ext2.c
index c3058f7e71..a5650c34cf 100644
--- a/grub-core/fs/ext2.c
+++ b/grub-core/fs/ext2.c
@@ -496,7 +496,10 @@ grub_ext2_read_block (grub_fshelp_node_t node, grub_disk_addr_t fileblock)
       int i;
       grub_disk_addr_t ret;
       grub_uint16_t nent;
+      /* maximum number of extent entries in the inode's inline extent area */
       const grub_uint16_t max_inline_ext = sizeof (inode->blocks) / sizeof (*ext) - 1; /* Minus 1 extent header. */
+      /* maximum number of extent entries in the external extent block */
+      const grub_uint16_t max_external_ext = EXT2_BLOCK_SIZE(data) / sizeof (*ext) - 1; /* Minus 1 extent header. */
 
       if (grub_ext4_find_leaf (data, (struct grub_ext4_extent_header *) inode->blocks.dir_blocks,
 			       fileblock, &leaf) != GRUB_ERR_NONE)
@@ -513,8 +516,18 @@ grub_ext2_read_block (grub_fshelp_node_t node, grub_disk_addr_t fileblock)
 
       nent = grub_le_to_cpu16 (leaf->entries);
 
-      if (leaf->depth == 0)
+      /*
+       * Determine the effective number of extent entries (nent) to process:
+       * If the extent header (leaf) is stored inline in the inode’s block
+       * area (i.e. at inode->blocks.dir_blocks), then only max_inline_ext
+       * entries can fit.
+       * Otherwise, if the header was read from an external extent block, use
+       * the larger limit, max_external_ext, based on the full block size.
+       */
+      if (leaf == (struct grub_ext4_extent_header *) inode->blocks.dir_blocks)
 	nent = grub_min (nent, max_inline_ext);
+      else
+	nent = grub_min (nent, max_external_ext);
 
       for (i = 0; i < nent; i++)
         {
-- 
2.48.1


_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] fs/ext2: Rework out-of-bounds read for inline and external extents

[Christian Hesse]

[-- Attachment #1.1: Type: text/plain, Size: 1291 bytes --]

Michael Chang via Grub-devel <grub-devel@gnu.org> on Fri, 2025/02/21 09:06:
> Previously, the number of extent entries was not properly capped based
> on the actual available space. This could lead to insufficient reads for
> external extents, since the computation was based solely on the inline
> extent layout.
> 
> In this patch, when processing the extent header, we determine whether
> the header is stored inline (i.e., at inode->blocks.dir_blocks) or in an
> external extent block. We then clamp the number of entries accordingly
> (using max_inline_ext for inline extents and max_external_ext for
> external extent blocks).
> 
> This change ensures that only the valid number of extent entries is
> processed, preventing out-of-bound reads and potential filesystem
> corruption.
> 
> Fixes: 7e2f750f0a (fs/ext2: Fix out-of-bounds read for inline extents)
> 
> Signed-off-by: Michael Chang <mchang@suse.com>

Tested and it fixes reading from loopback disk on ext4 for me. Thanks!

Tested-by: Christian Hesse <mail@eworm.de>
-- 
main(a){char*c=/*    Schoene Gruesse                         */"B?IJj;MEH"
"CX:;",b;for(a/*    Best regards             my address:    */=0;b=c[a++];)
putchar(b-1/(/*    Chris            cc -ox -xc - && ./x    */b/42*2-3)*42);}

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 141 bytes --]

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] fs/ext2: Rework out-of-bounds read for inline and external extents

[Daniel Kiper]

On Fri, Feb 21, 2025 at 10:02:54AM +0100, Christian Hesse wrote:
> Michael Chang via Grub-devel <grub-devel@gnu.org> on Fri, 2025/02/21 09:06:
> > Previously, the number of extent entries was not properly capped based
> > on the actual available space. This could lead to insufficient reads for
> > external extents, since the computation was based solely on the inline
> > extent layout.
> >
> > In this patch, when processing the extent header, we determine whether
> > the header is stored inline (i.e., at inode->blocks.dir_blocks) or in an
> > external extent block. We then clamp the number of entries accordingly
> > (using max_inline_ext for inline extents and max_external_ext for
> > external extent blocks).
> >
> > This change ensures that only the valid number of extent entries is
> > processed, preventing out-of-bound reads and potential filesystem
> > corruption.
> >
> > Fixes: 7e2f750f0a (fs/ext2: Fix out-of-bounds read for inline extents)
> >
> > Signed-off-by: Michael Chang <mchang@suse.com>
>
> Tested and it fixes reading from loopback disk on ext4 for me. Thanks!
>
> Tested-by: Christian Hesse <mail@eworm.de>

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Daniel

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel