fs/ext: support flex-bg writing and update new blocks allocation selection #584

2026-07-19T23:49:21Z

Matou1306 commented

2026-07-19 23:49:21 +00:00

Update extfsExtentAlloc to take edi as a goal block to start allocation at the previous block's group, falling back to the file's inode block group if edi is zero.

Update `extfsExtentAlloc` to take edi as a goal block to start allocation at the previous block's group, falling back to the file's inode block group if edi is zero.

Burer requested review from Ghost 2026-07-22 08:10:11 +00:00

Burer requested review from Ghost 2026-07-22 08:10:12 +00:00

Burer requested changes

2026-07-23 07:13:15 +00:00

Dismissed

Burer left a comment

🔴 OOB block-group descriptor access from an unvalidated goal block

The struct offset (logGroupsPerFlex at 0x174, checksumSeed preserved at 0x270), the symbolic INCOMPATIBLE_SUPPORT (= old 22C2h), and the flex_bg inode-alloc change all look correct. But the new goal path in extfsExtentAlloc is exploitable.

edi = goal derives a group index with no upper bound:

.use_goal:
        mov     eax, edi
        sub     eax, [firstGroupBlock]
        div     [blocksPerGroup]          ; group, never checked vs group count
.calc_group:
        ... ebx = descriptorTable + group*descSize
.test_block_group:
        cmp     [ebx+BGDESCR.blocksFree_lo], 0   ; OOB read if group is out of range

.next (1008-1019) bounds the group against [descriptorTable, descriptorTableEnd), but the first iteration bypasses it and dereferences the descriptor directly at 883. If that OOB read is nonzero, the completion path then does an OOB write:

        sub     [ebx+BGDESCR.blocksFree_lo], cx   ; OOB write to kernel heap
        call    extfsWriteDescriptor              ; and to disk

Source of the bad goal: extfsExtendFile (1538) takes the goal from extfsGetExtent, i.e. a physical block number straight out of the file's extent tree, with no check against the filesystem size. A crafted extent (block number e.g. 0xFFFFFFF0) yields a group index far past descriptorTableEnd.

Trigger: mount a crafted ext image, extend a file whose extent points outside the volume => OOB descriptor read, then a possible arbitrary block read (via the OOB blockBitmap_lo) and an OOB descriptor write. Same class as #570 - a new unvalidated input indexing an array.

Fix: after the div [blocksPerGroup], validate the group against the group count (or the goal against total blocks) and fall back to the inode-based group, or error, when out of range.

**🔴 OOB block-group descriptor access from an unvalidated goal block** The struct offset (logGroupsPerFlex at 0x174, checksumSeed preserved at 0x270), the symbolic `INCOMPATIBLE_SUPPORT` (= old 22C2h), and the flex_bg inode-alloc change all look correct. But the new goal path in `extfsExtentAlloc` is exploitable. `edi = goal` derives a group index with no upper bound: ```asm .use_goal: mov eax, edi sub eax, [firstGroupBlock] div [blocksPerGroup] ; group, never checked vs group count .calc_group: ... ebx = descriptorTable + group*descSize .test_block_group: cmp [ebx+BGDESCR.blocksFree_lo], 0 ; OOB read if group is out of range ``` `.next` (1008-1019) bounds the group against `[descriptorTable, descriptorTableEnd)`, but the **first** iteration bypasses it and dereferences the descriptor directly at 883. If that OOB read is nonzero, the completion path then does an OOB **write**: ```asm sub [ebx+BGDESCR.blocksFree_lo], cx ; OOB write to kernel heap call extfsWriteDescriptor ; and to disk ``` **Source of the bad goal:** `extfsExtendFile` (1538) takes the goal from `extfsGetExtent`, i.e. a physical block number straight out of the file's extent tree, with no check against the filesystem size. A crafted extent (block number e.g. 0xFFFFFFF0) yields a group index far past `descriptorTableEnd`. **Trigger:** mount a crafted ext image, extend a file whose extent points outside the volume => OOB descriptor read, then a possible arbitrary block read (via the OOB `blockBitmap_lo`) and an OOB descriptor write. Same class as #570 - a new unvalidated input indexing an array. **Fix:** after the `div [blocksPerGroup]`, validate the group against the group count (or the goal against total blocks) and fall back to the inode-based group, or error, when out of range.

Matou1306 referenced this pull request

2026-07-24 13:50:34 +00:00

fs/ext: Support INCOMPAT_EXTENTS, RO_COMPAT_DIR_NLINK, RO_COMPAT_HUGE_FILE. #603

Matou1306 force-pushed flex-bg-write from 512eaa4798 to 4e928da42c

2026-07-24 22:40:22 +00:00

Compare

Matou1306 force-pushed flex-bg-write from 4e928da42c to f9a353349e

2026-07-24 22:43:14 +00:00

Compare

Matou1306 commented

2026-07-25 20:32:41 +00:00

Hi @Burer.
Thanks, please check the latest commit.

Hi @Burer. Thanks, please check the latest commit.

👀 1

Burer requested changes

2026-08-08 15:25:52 +00:00

Dismissed

Burer left a comment

🔴 Cross-PR hazard with #570 (the hunks don't overlap, so git will merge both silently). This PR makes edi an input of extfsExtentAlloc and stops preserving it; ext_CreateSymlink.slow_symlink in #570 calls it with edi = filename pointer and relies on it surviving. If the pointer divides into a group index past the descriptor table, the new OOB check fails long-symlink creation with ERROR_FS_FAIL; but user space starts at 0, so the pointer is usually small, the derived group index is often plausible, the OOB check passes - the extent lands in an arbitrary group and the clobbered edi then reaches linkInode as the name pointer, producing a garbage directory entry. Whichever PR merges second needs, in #570: push edi / xor edi, edi / call extfsExtentAlloc / pop edi. Both halves matter: zero goal (the inode's group is right for a symlink) and explicit preservation - xor alone doesn't survive the call.

🟡 The goal lookup in extfsExtendFile passes the wrong register. extfsGetExtent takes the starting file block in ecx (per its header), but the new code loads ecx from [esp+8] - which at that point is the inode number (ExtendFile's input ecx) - and puts the last-block index into eax, which GetExtent ignores. The lookup fails past-EOF almost every time, silently falling back to edi = 0, so the goal-directed allocation this PR introduces mostly never engages; when the inode number happens to be smaller than the file's block count, it returns an unrelated block as the goal. Fix: lea ecx, [edx-1] and drop the eax load - edx survives GetExtent (both exit paths restore it).

🔵 Contract notes in extfsExtentAlloc's header: add that edi is clobbered on return (callers previously relied on it being preserved), and that on CF=1 ecx is no longer guaranteed 0 (the old .fail zeroed it; the new path returns the caller's blocks-max - none of the three current call sites reads it, but the guarantee is gone).

🔵 logGroupsPerFlex is read from disk unvalidated; shl masks the count to 5 bits, so a corrupted value > 31 yields a garbage flexGroupSize. Impact is limited to the inode-allocation heuristic - still would be nice to add a two-line cap hardens against broken images.

🔴 **Cross-PR hazard with #570** (the hunks don't overlap, so git will merge both silently). This PR makes `edi` an input of `extfsExtentAlloc` and stops preserving it; `ext_CreateSymlink.slow_symlink` in #570 calls it with `edi` = filename pointer and relies on it surviving. If the pointer divides into a group index past the descriptor table, the new OOB check fails long-symlink creation with `ERROR_FS_FAIL`; but user space starts at 0, so the pointer is usually small, the derived group index is often plausible, the OOB check passes - the extent lands in an arbitrary group and the clobbered `edi` then reaches `linkInode` as the name pointer, producing a garbage directory entry. Whichever PR merges second needs, in #570: `push edi` / `xor edi, edi` / `call extfsExtentAlloc` / `pop edi`. Both halves matter: zero goal (the inode's group is right for a symlink) *and* explicit preservation - `xor` alone doesn't survive the call. 🟡 **The goal lookup in `extfsExtendFile` passes the wrong register.** `extfsGetExtent` takes the starting file block in `ecx` (per its header), but the new code loads `ecx` from `[esp+8]` - which at that point is the *inode number* (ExtendFile's input `ecx`) - and puts the last-block index into `eax`, which GetExtent ignores. The lookup fails past-EOF almost every time, silently falling back to `edi = 0`, so the goal-directed allocation this PR introduces mostly never engages; when the inode number happens to be smaller than the file's block count, it returns an unrelated block as the goal. Fix: `lea ecx, [edx-1]` and drop the `eax` load - `edx` survives GetExtent (both exit paths restore it). 🔵 Contract notes in `extfsExtentAlloc`'s header: add that `edi` is clobbered on return (callers previously relied on it being preserved), and that on CF=1 `ecx` is no longer guaranteed 0 (the old `.fail` zeroed it; the new path returns the caller's blocks-max - none of the three current call sites reads it, but the guarantee is gone). 🔵 `logGroupsPerFlex` is read from disk unvalidated; `shl` masks the count to 5 bits, so a corrupted value > 31 yields a garbage `flexGroupSize`. Impact is limited to the inode-allocation heuristic - still would be nice to add a two-line cap hardens against broken images.

👍 1

Matou1306 added 3 commits 2026-08-10 05:48:24 +00:00

support flex-bg writing and update new blocks allocation selection cb1d1ac86a

Check OOB blocks 5a486fa76d

Corrupted logGroupsPerFlex check, fix register in extfsExtendFile, add extra comments to explain error behaviour in extfsExtentAlloc

Check kernel codestyle / Check kernel codestyle (pull_request) Successful in 18s

Details

Test PR / Build (es_ES) (pull_request) Successful in 2m17s

Details

Test PR / Build (en_US) (pull_request) Successful in 2m17s

Details

Test PR / Build (ru_RU) (pull_request) Successful in 2m20s

Details

1329a809ef

Matou1306 force-pushed flex-bg-write from f9a353349e to 1329a809ef

2026-08-10 05:48:24 +00:00

Compare

Doczom added the