Re: [PATCH] ocfs2: validate inline dir size in ocfs2_dir_foreach_blk_id

Joseph Qi <[email protected]>
Newsgroups dev.linux.lists.ocfs2-devel,org.kernel.vger.linux-kernel
Message-ID <[email protected]>

On 4/10/26 2:22 PM, ZhengYuan Huang wrote:
> [BUG]
> A crafted inline-data directory can set i_size larger than id_count.
> readdir then walks past data->id_data and KASAN reports:
> 
> BUG: KASAN: use-after-free in ocfs2_check_dir_entry.isra.0+0x31f/0x370 fs/ocfs2/dir.c:305
> Read of size 2 at addr ffff8880088f0008 by task syz.0.1936/4656
> Call Trace:
>  ...
>  ocfs2_check_dir_entry.isra.0+0x31f/0x370 fs/ocfs2/dir.c:305
>  ocfs2_dir_foreach_blk_id+0x203/0xa70 fs/ocfs2/dir.c:1805
>  ocfs2_dir_foreach_blk fs/ocfs2/dir.c:1933 [inline]
>  ocfs2_readdir+0x4ba/0x520 fs/ocfs2/dir.c:1977
>  wrap_directory_iterator+0x9c/0xe0 fs/readdir.c:65
>  shared_ocfs2_readdir+0x29/0x40 fs/ocfs2/file.c:2822
>  iterate_dir+0x276/0x9e0 fs/readdir.c:108
>  __do_sys_getdents64 fs/readdir.c:410 [inline]
>  __se_sys_getdents64 fs/readdir.c:396 [inline]
>  __x64_sys_getdents64+0x143/0x2a0 fs/readdir.c:396
>  ...
> 
> [CAUSE]
> ocfs2_dir_foreach_blk_id() uses i_size_read(inode) as the loop bound
> after reading the inode block. Inline directories are only valid while
> i_size <= le16_to_cpu(data->id_count), but that invariant is never
> checked on read. Once ctx->pos reaches id_count, data->id_data +
> ctx->pos points past the inode block and can land in a freed neighbor
> page.
> 

ocfs2_read_inode_block() has already validated dinode->i_size.
So how it happens for the corrupted in-memory i_size?

Thanks,
Joseph

> [FIX]
> Validate i_size_read(inode) against data->id_count immediately after
> ocfs2_read_inode_block(). If the inline directory size exceeds its
> on-disk capacity, return -EFSCORRUPTED before constructing any dirent
> pointer. Keep the change local to ocfs2_dir_foreach_blk_id() so the
> patch stays scoped to the readdir bug.
> 
> Fixes: 23193e513d1c ("ocfs2: Read support for directories with inline data")
> Signed-off-by: ZhengYuan Huang <[email protected]>
> ---
>  fs/ocfs2/dir.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
> index 8c9c4825f984..fa537505d1a9 100644
> --- a/fs/ocfs2/dir.c
> +++ b/fs/ocfs2/dir.c
> @@ -1761,6 +1761,7 @@ static int ocfs2_dir_foreach_blk_id(struct inode *inode,
>  				    struct dir_context *ctx)
>  {
>  	int ret, i;
> +	int error = 0;
>  	unsigned long offset = ctx->pos;
>  	struct buffer_head *di_bh = NULL;
>  	struct ocfs2_dinode *di;
> @@ -1777,6 +1778,12 @@ static int ocfs2_dir_foreach_blk_id(struct inode *inode,
>  	di = (struct ocfs2_dinode *)di_bh->b_data;
>  	data = &di->id2.i_data;
>  
> +	if (unlikely(i_size_read(inode) > le16_to_cpu(data->id_count))) {
> +		error = -EFSCORRUPTED;
> +		mlog_errno(error);
> +		goto out;
> +	}
> +
>  	while (ctx->pos < i_size_read(inode)) {
>  		/* If the dir block has changed since the last call to
>  		 * readdir(2), then we might be pointing to an invalid
> @@ -1819,7 +1826,7 @@ static int ocfs2_dir_foreach_blk_id(struct inode *inode,
>  	}
>  out:
>  	brelse(di_bh);
> -	return 0;
> +	return error;
>  }
>  
>  /*
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.