U-Boot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v2 3/3] fs: ext4: widen ext4fs_update() block-group loop counter
@ 2026-09-15 21:49 Sriram Sriram
  2026-09-15 22:12 ` Tom Rini
  0 siblings, 1 reply; 7+ messages in thread
From: Sriram Sriram @ 2026-09-15 21:49 UTC (permalink / raw)
  To: Tony Dinh; +Cc: u-boot

jerome.forissier@arm.com; ankluemk@microsoft.com; v-dmunic@microsoft.com
Bcc: 
Subject: Re: [PATCH v2 3/3] fs: ext4: widen ext4fs_update() block-group loop
 counter
Reply-To: 
In-Reply-To: <CAJaLiFxMbEO1hk0o6ja6w1SKN-hVK4tGkvUo90uO+eFgAc631Q@mail.gmail.com>

On Tue, Sep 15, 2026 at 02:23:12PM -0700, Tony Dinh wrote:
> Hi Sriram and Daniel,
> 
> On Tue, Sep 15, 2026 at 1:52 PM Sriram Sriram
> <sriramsriram@linux.microsoft.com> wrote:
> >
> > From: Daniel Munic <v-dmunic@microsoft.com>
> >
> > ext4fs_update() iterates over all block groups with a signed 16-bit
> > loop counter:
> >
> >         short i;
> >         ...
> >         for (i = 0; i < fs->no_blkgrp; i++)
> >
> > fs->no_blkgrp is a uint32_t. On a filesystem with more than 32767
> > block groups the counter cannot represent every index: incrementing
> > past SHRT_MAX is signed overflow (undefined behaviour) and the
> > comparison against the unsigned no_blkgrp never terminates correctly,
> > so the bitmap/group-descriptor write-back loops misbehave. The mixed
> > signed/unsigned comparison is also flagged by static analysis.
> >
> > Use u32 for the loop counter, matching the width of no_blkgrp.
> >
> > Signed-off-by: Daniel Munic <v-dmunic@microsoft.com>
> > Signed-off-by: Sriram Sriram <sriramsriram@linux.microsoft.com>
> Reviewed-by: Tony Dinh <mibodhi@gmail.com>
> 
> BTW, since you are testing this ext4_write, it might be worthwhile to
> see this commit:
> https://github.com/u-boot/u-boot/commit/53cc4332b3b37218a7cdab8bdb953da57eec2668
> 
> Thanks,
> Tony
> 
> > ---
> > Changes in v2:
> >  - No change, rebased onto current master.
> >
> >  fs/ext4/ext4_write.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/ext4/ext4_write.c b/fs/ext4/ext4_write.c
> > index 1abedcede72..2fba2197c19 100644
> > --- a/fs/ext4/ext4_write.c
> > +++ b/fs/ext4/ext4_write.c
> > @@ -67,7 +67,7 @@ static inline void ext4fs_bg_free_blocks_inc
> >
> >  static void ext4fs_update(void)
> >  {
> > -       short i;
> > +       u32 i;
> >         ext4fs_update_journal();
> >         struct ext_filesystem *fs = get_fs();
> >         struct ext2_block_group *bgd = NULL;
> > --
> > 2.49.0
> >


> Reviewed-by: Tony Dinh <mibodhi@gmail.com>

Hi Tony,

Thanks for the review.

> BTW, since you are testing this ext4_write, it might be worthwhile to
> see this commit:
> ext4fs: Fix: Read outside partition error (take 2)

Thanks for the pointer. 53cc4332b3b is already in the tree this series
is based on, so the read path is covered here.

What caught my eye is the note in that commit message:

	Looks like we also have this overflown problem in
	ext4_write.c that needs to be addressed.

As far as I can tell that is still unaddressed. ext4fs_write_file()
has the same wrap you fixed in ext4fs_read_file():

	long int blknr;
	...
	blknr = blknr << log2_fs_blocksize;

and on top of that delayed_start, delayed_next and
previous_block_number are all plain int:

	int previous_block_number = -1;
	int delayed_start = 0;
	int delayed_next = 0;

which makes the casts at the put_ext4() call sites

	put_ext4((uint64_t)((uint64_t)delayed_start << log2blksz),
		 delayed_buf, (uint32_t)delayed_extent);

ineffective, since the value has already been truncated on its way
into delayed_start. The fix would mirror 53cc4332b3b: lbaint_t blknr
plus a separate long for the read_allocated_block() return and error
check, and widening delayed_start/delayed_next to lbaint_t.

I am happy to write that up, but I should be up front that I cannot
test it. The largest medium I can reach here is well below the 2^31
sector point, and since this is the write path an untested patch
risks writing to the wrong LBA.

Would you rather take it yourself, given you already have the DS116
and the 4TB disk set up from the read-side reproducer? If you would
prefer that I send it, I can post it with you on Cc and leave it
pending your Tested-by.

Thanks,
Sriram

^ permalink raw reply	[flat|nested] 7+ messages in thread
* [PATCH 0/4] Bounds/overflow hardening in NFS, FIT, TFTP and ext4
@ 2026-09-09 19:20 Sriram Sriram
  2026-09-15 20:52 ` [PATCH v2 0/3] Bounds hardening in NFS, " Sriram Sriram
  0 siblings, 1 reply; 7+ messages in thread
From: Sriram Sriram @ 2026-09-09 19:20 UTC (permalink / raw)
  To: u-boot
  Cc: Tom Rini, Jerome Forissier, Simon Glass, Drew Kluemke,
	Daniel Munic, Sriram Sriram

This series collects four independent robustness fixes to network, boot
image and filesystem code that handle attacker-influenced or on-disk
input. Each was found by auditing length handling around memcpy(),
integer arithmetic on packet/image data, and loop-counter widths.

  1. net: nfs: the NFS client copies received UDP payloads into a
     stack-allocated struct rpc_t with memcpy() using the wire length
     without checking it against the destination size, allowing a
     malicious NFS server to overflow the stack buffer. Add a bounds
     check at each copy site.

  2. boot: image-fit: the decompression path computes the output buffer
     size as 'len * 20', which can wrap on 32/64-bit ulong for a large
     image and cause a heap buffer overflow. Reject sizes that would
     overflow before the multiplication.

  3. net: tftp: the OACK option parser uses strcasecmp() on packet data
     that may not be NUL-terminated within the received length, causing
     an out-of-bounds read. Use a bounded strncasecmp() plus an explicit
     terminator check.

  4. fs: ext4: ext4fs_update() walks all block groups with a signed
     16-bit loop counter while fs->no_blkgrp is a uint32_t. A filesystem
     with more than 32767 block groups overflows the counter (undefined
     behaviour) and the bitmap write-back loops fail to terminate
     correctly. Widen the counter to u32.

The fixes are independent and can be applied in any order. Built for
sandbox (net/tftp.o, boot/image-fit.o, fs/ext4/ext4_write.o, and
net/nfs-common.o with CONFIG_CMD_NFS=y) and checked with
scripts/checkpatch.pl.

Daniel Munic (1):
  fs: ext4: widen ext4fs_update() block-group loop counter

Drew Kluemke (3):
  net: nfs: add bounds checks on memcpy into stack-allocated rpc_pkt
  boot: image-fit: add overflow guard for FIT decompression buffer
  net: tftp: use bounded string compare for OACK option parsing

 boot/image-fit.c     | 10 +++++++++-
 fs/ext4/ext4_write.c |  2 +-
 net/nfs-common.c     | 10 ++++++++++
 net/tftp.c           | 13 +++++++++----
 4 files changed, 29 insertions(+), 6 deletions(-)

-- 
2.49.0


^ permalink raw reply	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-09-15 23:13 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-15 21:49 [PATCH v2 3/3] fs: ext4: widen ext4fs_update() block-group loop counter Sriram Sriram
2026-09-15 22:12 ` Tom Rini
2026-09-15 22:16   ` Sriram Sriram
2026-09-15 22:41     ` Tony Dinh
2026-09-15 23:12       ` Sriram Sriram
  -- strict thread matches above, loose matches on Subject: below --
2026-09-09 19:20 [PATCH 0/4] Bounds/overflow hardening in NFS, FIT, TFTP and ext4 Sriram Sriram
2026-09-15 20:52 ` [PATCH v2 0/3] Bounds hardening in NFS, " Sriram Sriram
2026-09-15 20:52   ` [PATCH v2 3/3] fs: ext4: widen ext4fs_update() block-group loop counter Sriram Sriram
2026-09-15 21:23     ` Tony Dinh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox