Re: [PATCH 6.12] ext4: validate readdir offset before accessing dirent

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] ext4: unvalidated rescan rec_len in ext4_readdir() bypasses tail check
--

commit f6671320411a20d7f17d0b2fabcaf53f7a52ec83
Author: Yao Kai <yaokai34@huawei.com>

ext4: validate readdir offset before accessing dirent

This commit bounds the directory entry rescan loop in ext4_readdir() using the
minimum directory entry record length and validates that offset does not land
in a truncated block tail before entering the main entry-processing loop. If
offset falls into the trailing bytes at the end of the block, it logs a
directory error and advances ctx->pos to the next block.

Link: https://patch.msgid.link/20260706041313.708346-1-yaokai34@huawei.com

> diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
> index b278b5703c197..265e544cc3482 100644
> --- a/fs/ext4/dir.c
> +++ b/fs/ext4/dir.c

[ ... ]

> @@ -234,7 +235,10 @@ static int ext4_readdir(struct file *file, struct dir_context *ctx)
>  		 * dirent right now.  Scan from the start of the block
>  		 * to make sure. */
>  		if (!inode_eq_iversion(inode, info->cookie)) {
> -			for (i = 0; i < sb->s_blocksize && i < offset; ) {
> +			for (i = 0;
> +			     i <= sb->s_blocksize -
> +				  ext4_dir_rec_len(1, has_csum ? NULL : inode) &&
> +			     i < offset;) {
>  				de = (struct ext4_dir_entry_2 *)
>  					(bh->b_data + i);
>  				/* It's too expensive to do a full
> @@ -256,6 +260,17 @@ static int ext4_readdir(struct file *file, struct dir_context *ctx)
>  			info->cookie = inode_query_iversion(inode);
>  		}
>  
> +		if (unlikely(offset < sb->s_blocksize &&
> +			     offset > sb->s_blocksize -
> +			     ext4_dir_rec_len(1, has_csum ? NULL : inode))) {
> +			EXT4_ERROR_FILE(file, bh->b_blocknr,
> +					"bad entry in directory: %s - offset=%u, size=%lu",
> +					"directory entry too close to block end",
> +					offset, sb->s_blocksize);
> +			ctx->pos = round_up(ctx->pos, sb->s_blocksize);
> +			goto next_block;
> +		}

[Severity: Medium]
What happens in ext4_readdir() if a corrupted de->rec_len in the rescan loop
advances i past sb->s_blocksize?

In fs/ext4/dir.c:ext4_readdir(), when !inode_eq_iversion(inode, info->cookie)
triggers a rescan (for instance after ext4_dir_llseek()), the loop only checks
the lower bound of de->rec_len before adding it to i and updating ctx->pos:

		if (!inode_eq_iversion(inode, info->cookie)) {
			for (i = 0;
			     i <= sb->s_blocksize -
				  ext4_dir_rec_len(1, has_csum ? NULL : inode) &&
			     i < offset;) {
				...
				if (ext4_rec_len_from_disk(de->rec_len,
					sb->s_blocksize) < ext4_dir_rec_len(1,
									inode))
					break;
				i += ext4_rec_len_from_disk(de->rec_len,
							    sb->s_blocksize);
			}
			offset = i;
			ctx->pos = (ctx->pos & ~(sb->s_blocksize - 1))
				| offset;
			info->cookie = inode_query_iversion(inode);
		}

Because de->rec_len is a 16-bit on-disk value, a corrupted entry can set
offset > sb->s_blocksize. That value is bitwise-ORed into ctx->pos, which
corrupts the upper block-number bits of ctx->pos, and info->cookie is updated
to match inode->i_version.

Then, because the new tail check requires offset < sb->s_blocksize:

		if (unlikely(offset < sb->s_blocksize &&
			     offset > sb->s_blocksize -
			     ext4_dir_rec_len(1, has_csum ? NULL : inode))) {

and the subsequent main loop also checks offset < sb->s_blocksize, an
oversized offset bypasses both EXT4_ERROR_FILE() and ext4_check_dir_entry() on
the corrupted block. On the next iteration of the outer loop, ext4_readdir()
will read the block corresponding to the corrupted ctx->pos at an arbitrary
mid-block offset = ctx->pos & (sb->s_blocksize - 1) without rescanning, since
info->cookie now matches inode->i_version.

Should the tail check (or the rescan loop) also catch offset > sb->s_blocksize
before bitwise-ORing offset into ctx->pos and validating info->cookie?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261028003746.31943-1-ebiggers@kernel.org?part=1