Re: [PATCH v7] 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: missing index-to-index alias check in dx_probe() and ext4_dx_add_entry()
--

commit 5e5de45a3996807d23fd6ebf3ff43ca17d889c45
Author: Shuai Liu <847807867@qq.com>

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

This commit adds buffer_head alias checks in do_split(), make_indexed_dir(),
and ext4_dx_add_entry() to prevent directory leaf blocks and newly allocated
blocks from aliasing each other or active htree index frames on corrupted
filesystems.

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

[ ... ]

> @@ -2496,6 +2535,15 @@ static int ext4_dx_add_entry(handle_t *handle, struct ext4_filename *fname,
>  		bh = NULL;
>  		goto cleanup;
>  	}
> +	for (f = frames; f <= frame; f++) {
> +		if (bh == f->bh) {
> +			ext4_error_inode_block(dir, bh->b_blocknr,
> +					       EFSCORRUPTED,
> +					       "leaf block aliased with index block");
> +			err = -EFSCORRUPTED;
> +			goto cleanup;
> +		}
> +	}
>  
>  	BUFFER_TRACE(bh, "get_write_access");
>  	err = ext4_journal_get_write_access(handle, sb, bh, EXT4_JTR_NONE);
> @@ -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 < frames + levels; f++) {
> +			if (bh2 == f->bh)
> +				break;
> +		}
> +		if (f < frames + levels || bh2 == bh) {
> +			ext4_error_inode_block(dir, bh2->b_blocknr,
> +					       EFSCORRUPTED,
> +					       "cannot split aliased index block");
> +			brelse(bh2);
> +			err = -EFSCORRUPTED;
> +			goto cleanup;
> +		}

[Severity: High]
What happens if two active htree index frames in frames[] already alias the
same physical block when dx_probe() returns?

In dx_probe(), the cycle check only compares logical block numbers in
blocks[]:

	block = dx_get_block(at);
	for (i = 0; i <= level; i++) {
		if (blocks[i] == block) {
			ext4_warning_inode(dir,
				"dx entry: tree cycle block %u points back to block %u",
				blocks[level], block);
			goto fail;
		}
	}
	if (++level > indirect)
		return frame;
	blocks[level] = block;
	frame++;
	frame->bh = ext4_read_dirblock(dir, block, INDEX);

On a corrupted filesystem where two distinct logical index blocks (such as
logical block 0 for frames[0] and logical block 1 for frames[1]) map to the
same physical disk block, dx_probe() can populate frames[] with identical
buffer_head pointers (frames[0].bh == frames[1].bh) becauseframes[0].entries
is read at offset 32 (after dx_root_info) while the child dx_node entries
array is read at offset 8, allowing a single physical block to pass the
limit, count, and checksum checks for both levels.

The new checks in ext4_dx_add_entry() and do_split() verify that the leaf
block bh and newly appended block bh2 do not alias f->bh, but nothing checks
whether the active index frames in frames[0..frame] alias each other.

When do_split() or the interior index split in ext4_dx_add_entry() calls
dx_insert_block() on a child frame whose bh aliases frames[0].bh, shifting or
writing dx_node entries starting at offset 8 overwrites frames[0]'s
dx_root_info (including info.indirect_levels) and root dx_countlimit in
place. When dx_release() subsequently runs:

	info = &((struct dx_root *)frames[0].bh->b_data)->info;
	/* save local copy, "info" may be freed after brelse() */
	indirect_levels = info->indirect_levels;
	for (i = 0; i <= indirect_levels; i++) {
		if (frames[i].bh == NULL)
			break;
		brelse(frames[i].bh);
		frames[i].bh = NULL;
	}

Can this corrupt info->indirect_levels and cause dx_release() to either leak
lower-level frame buffer_heads or read past the end of frames[EXT4_HTREE_LEVEL]
when all frames are active? Would checking that frame->bh does not alias any
ancestor frames[0..level-1].bh in dx_probe() prevent index-to-index aliasing
as well?

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