Re: [PATCH v2 2/2] ocfs2: bound-check dir entries in the inline-data re-validation scan

[Joseph Qi]

On 8/11/26 10:43 AM, Zhan Xusheng wrote:
> ocfs2_dir_foreach_blk_id() re-scans the inline data area the same way
> ocfs2_dir_foreach_blk_el() re-scans a directory block, and is missing the
> same two bounds:
> 
> 	for (i = 0; i < i_size_read(inode) && i < offset; ) {
> 		de = (struct ocfs2_dir_entry *)(data->id_data + i);
> 		if (le16_to_cpu(de->rec_len) < OCFS2_DIR_REC_LEN(1))
> 			break;
> 		i += le16_to_cpu(de->rec_len);
> 	}
> 
> ocfs2_validate_inode_block() keeps i_size inside the inline area:
> 
> 	if (le16_to_cpu(data->id_count) >
> 	    ocfs2_max_inline_data_with_xattr(sb, di))
> 	if (le64_to_cpu(di->i_size) > le16_to_cpu(data->id_count))
> 
> and that area runs to the end of the inode block, so for a full inline
> directory data->id_data + i_size is the end of di_bh->b_data.  A bogus
> rec_len leaves i in the last OCFS2_DIR_REC_LEN(1) - 1 bytes of it, and
> de->rec_len, at byte offset 8 within the entry, is then read past the
> block.
> 
> The emit loop below hands i_size_read(inode) to ocfs2_check_dir_entry(),
> which refuses both an entry that close to the end and one whose rec_len
> runs past it.  Apply the same two bounds to the re-validation scan, reading
> i_size once into a local as ocfs2_check_dir_entry() takes it as @size.
> 
> Unlike the extent case there is no mask to corrupt here: an unbounded i
> only sets ctx->pos past i_size, which ends the readdir early rather
> than moving it to the wrong place.
> 
> Cc: Mark Fasheh <mark@xxxxxxxxxx>
> Cc: Joel Becker <jlbec@xxxxxxxxxxxx>
> Cc: Junxiao Bi <junxiao.bi@xxxxxxxxxx>
> Cc: Changwei Ge <gechangwei@xxxxxxx>
> Cc: Jun Piao <piaojun@xxxxxxxxxx>
> Cc: Heming Zhao <heming.zhao@xxxxxxxx>
> Signed-off-by: Zhan Xusheng <zhanxusheng@xxxxxxxxxx>

Looks fine.
Reviewed-by: Joseph Qi <joseph.qi@xxxxxxxxxxxxxxxxx>

> ---
>  fs/ocfs2/dir.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
> index 39ce60874e17..863c69dec7da 100644
> --- a/fs/ocfs2/dir.c
> +++ b/fs/ocfs2/dir.c
> @@ -1812,7 +1812,12 @@ static int ocfs2_dir_foreach_blk_id(struct inode *inode,
>  		 * dirent right now.  Scan from the start of the block
>  		 * to make sure. */
>  		if (!inode_eq_iversion(inode, *f_version)) {
> -			for (i = 0; i < i_size_read(inode) && i < offset; ) {
> +			loff_t size = i_size_read(inode);
> +
> +			for (i = 0; i + OCFS2_DIR_REC_LEN(1) <= size &&
> +			     i < offset;) {
> +				unsigned int rec_len;
> +
>  				de = (struct ocfs2_dir_entry *)
>  					(data->id_data + i);
>  				/* It's too expensive to do a full
> @@ -1821,10 +1826,11 @@ static int ocfs2_dir_foreach_blk_id(struct inode *inode,
>  				 * least that it is non-zero.  A
>  				 * failure will be detected in the
>  				 * dirent test below. */
> -				if (le16_to_cpu(de->rec_len) <
> -				    OCFS2_DIR_REC_LEN(1))
> +				rec_len = le16_to_cpu(de->rec_len);
> +				if (rec_len < OCFS2_DIR_REC_LEN(1) ||
> +				    i + rec_len > size)
>  					break;
> -				i += le16_to_cpu(de->rec_len);
> +				i += rec_len;
>  			}
>  			ctx->pos = offset = i;
>  			*f_version = inode_query_iversion(inode);