Re: [PATCH v6] ext4: fix slab-use-after-free in do_split()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ext4: incomplete bh2 alias check in ext4_dx_add_entry() index split
--

commit 24061820362e4d33d4d9d2657a26970aa2569b92
Author: Shuai Liu <847807867@qq.com>

ext4: fix slab-use-after-free in do_split()

This commit adds buffer_head aliasing checks in make_indexed_dir(),
ext4_dx_add_entry(), and do_split() to prevent directory and htree index
blocks from aliasing each other when allocating or reading blocks on a
filesystem with a corrupted block bitmap.

> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index 231a274b814ec..79f7f0073d9da 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c

[ ... ]

> @@ -2549,6 +2597,18 @@ static int ext4_dx_add_entry(handle_t *handle, struct ext4_filename *fname,
>  			err = PTR_ERR(bh2);
>  			goto cleanup;
>  		}
> +		for (f = frames; f <= frame; f++) {
> +			if (bh2 == f->bh)
> +				break;
> +		}
> +		if (f <= frame || bh2 == bh) {
> +			ext4_error_inode_block(dir, bh2->b_blocknr,
> +					       EFSCORRUPTED,
> +					       "cannot split aliased index block");
> +			brelse(bh2);
> +			err = -EFSCORRUPTED;
> +			goto cleanup;
> +		}

[Severity: High]
As reported by Sashiko on v5 [1], in ext4_dx_add_entry() in fs/ext4/namei.c,
when the leaf index block is full and an ancestor index block is also full,
the preceding loop walks up the htree and decrements frame:

	if (dx_get_count(entries) == dx_get_limit(entries)) {
		ext4_lblk_t newblock;
		int levels = frame - frames + 1;
		...
		while (frame > frames) {
			if (dx_get_count((frame - 1)->entries) <
			    dx_get_limit((frame - 1)->entries)) {
				add_level = 0;
				break;
			}
			frame--; /* split higher index block */
			at = frame->at;
			entries = frame->entries;
			restart = 1;
		}

By the time ext4_append() allocates bh2, frame may point to frames[0] (or an
intermediate frame) while the lower-level index blocks in
frames[(frame - frames) + 1 .. levels - 1] remain active in the htree and held
in frames[] until dx_release(frames) runs at cleanup.

Because the alias check loop bounds f by frame rather than
frames + levels - 1 (and testing f < frames + levels), can bh2 alias one of
those skipped lower-level index frames (such as frames[1].bh when splitting
the root index)? If so, wouldn't the subsequent memset and memcpy into
node2 overwrite the active lower index buffer in place and create a cycle in
the htree before jumping to again?

[1] https://lore.kernel.org/r/tencent_4EAEF2024D7BB83D62E41306EED8D2112C08@qq.com

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/tencent_75A7137EEE37D2E72C50F2284D95B85A6208@qq.com?part=1