Re: [PATCH 05/27] ext4: add KUnit tests for fs/ext4/namei.c

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [High] ext4: out-of-bounds read due to missing dx_entry validation in
  ext4_htree_next_block() [1]
--

commit 2ae6fd1b46635dda6b69ee78cd382ff9c0d0450a
Author: Theodore Ts'o <tytso@mit.edu>

ext4: add KUnit tests for fs/ext4/namei.c

This commit adds a KUnit test suite for directory lookup, HTree indexing,
and namespace operations in fs/ext4/namei.c, and exports the internal
HTree definitions and helper wrappers needed by the tests.

> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index a6386c1d237f1..63d01a9711a27 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c

[ ... ]

> @@ -4249,3 +4103,159 @@ const struct inode_operations ext4_special_inode_operations = {
>  	.get_inode_acl	= ext4_get_acl,
>  	.set_acl	= ext4_set_acl,
>  };
> +EXPORT_SYMBOL_FOR_EXT4_TEST(ext4_special_inode_operations);
> +
> +#if IS_ENABLED(CONFIG_EXT4_KUNIT_TESTS)

[ ... ]

> +int ext4_htree_next_block_test(struct inode *dir, __u32 hash,
> +			       struct dx_frame *frame,
> +			       struct dx_frame *frames, __u32 *start_hash)
> +{
> +	return ext4_htree_next_block(dir, hash, frame, frames, start_hash);
> +}
> +EXPORT_SYMBOL_FOR_EXT4_TEST(ext4_htree_next_block_test);

[Severity: High]
This problem wasn't introduced by this patch, but can
ext4_htree_next_block() read out of bounds past bh->b_data [1] when
loading a sibling interior dx_node block?

fs/ext4/namei.c:ext4_htree_next_block() {
	...
	while (1) {
		if (++(p->at) < p->entries + dx_get_count(p->entries))
			break;
		if (p == frames)
			return 0;
		num_frames++;
		p--;
	}
	...
	while (num_frames--) {
		bh = ext4_read_dirblock(dir, dx_get_block(p->at), INDEX);
		if (IS_ERR(bh))
			return PTR_ERR(bh);
		p++;
		brelse(p->bh);
		p->bh = bh;
		p->at = p->entries = ((struct dx_node *) bh->b_data)->entries;
	}
	return 1;
}

Unlike dx_probe(), which checks dx_get_limit() against dx_node_limit() and
verifies that dx_get_count() is non-zero and within dx_get_limit(),
ext4_htree_next_block() assigns p->entries directly from bh->b_data without
validating the new node's count or limit.  If a corrupted interior node has
an out-of-bounds count (for example, when metadata_csum is disabled or
buffer_verified(bh) is already set), later calls to ext4_htree_next_block()
advance p->at up to p->entries + dx_get_count(p->entries) and dereference
it via dx_get_hash() and dx_get_block(), reading past the end of bh->b_data.

[1] https://sashiko.dev/#/bug/linux-c6639d9a-25ec-49eb-a6a4-468135552f9a

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112040638.1576836-1-tytso@mit.edu?part=5