Re: [PATCH v2] ext4: synchronize free block counter when detecting corruption

[Albin Babu Varghese <albinbabuvarghese20@gmail.com>]

Hey Ted, Thanks for the feedback.

On Thu, Nov 06, 2025 at 10:30:35AM -0500, Theodore Ts'o wrote:
> On Fri, Oct 10, 2025 at 03:38:00AM -0400, Albin Babu Varghese wrote:
> > When ext4_mb_generate_buddy() detects block group descriptor
> > corruption (free block count mismatch between descriptor and
> > bitmap), it corrects the in-memory group descriptor (grp->bb_free)
> > but does not synchronize the percpu free clusters counter.
> 
> Actually, we do.  This happens in ext4_mark_group_bitmap_corrupted in
> fs/ext4/super.c.
> 
> 	if (flags & EXT4_GROUP_INFO_BBITMAP_CORRUPT) {
> 		ret = ext4_test_and_set_bit(EXT4_GROUP_INFO_BBITMAP_CORRUPT_BIT,
> 					    &grp->bb_state);
> 		if (!ret)
> 			percpu_counter_sub(&sbi->s_freeclusters_counter,
> 					   grp->bb_free);
> 	}
> 
> So we've *already* subtracted out the blocks that were in the block
> group which we've busied out.

Thanks for pointing that out. It was naive of me to overlook the other
occurences.

> > This causes delayed allocation to read stale counter values when
> > checking for available space. The allocator believes space is
> > available based on the stale counter, makes reservation promises,
> > but later fails during writeback when trying to allocate actual
> > blocks from the bitmap. This results in "Delayed block allocation
> > failed" errors and potential system crashes.
> 
> I suspect there is something else going on with s_freeclusters_counter
> being incorrect, but adding an additional correction to
> s_freeclusters_counter is not the answer.
> 
> How is the system crashing?  If we have errors=continue, then we
> really shouldn't let the system crash.  If there is delayed allocation
> failures, the user might lose data, but if the user really cares about
> that, they shouldn't be using errors=continue.

I think the existing check in `ext4_mb_generate_buddy` is for runtime errors,
and the issue here is happening at mount time due to an already corrupted
filesystem. The value of `grp->bb_free` and `s_freeclusters_counter` was
`150994969` vs `25`, which is the actual free clusters count. The existing
update call subtracts `grp->bb_free` from `s_freeclusters_counter` assuming
that the group descriptor is accurate, but in this case it is not. So we
still end up with an incorrect global counter.

I tried the patch here: 
https://syzkaller.appspot.com/text?tag=Patch&x=1771a7cd980000

In that version, I attempted to compute and pass the corrected value to the
update function, but it failed with the warning at `ext4_dirty_folio`: 
https://syzkaller.appspot.com/x/report.txt?x=1306a7cd980000

From what I understand, even after adjusting the counter, the dirty buffers had
already been created, so it returns an error that unmapped dirty buffers
remain.

My earlier fix ended up making `s_freeclusters_counter` become 0 due to the
update in `ext4_mark_group_bitmap_corrupted()` that I had overlooked. As a
result, no warnings or errors were triggered at that time.

I might be off here, and I’m not sure how best to proceed.

Best,
	Albin