Re: [PATCH 1/4] ocfs2: validate dx_root extent list fields during block read

Heming Zhao <[email protected]>
Newsgroups dev.linux.lists.ocfs2-devel,org.kernel.vger.linux-kernel
Message-ID <7xxyxgd6lomzalhigpp255b25rvpewf2dhwqvt4yr65mshunjn@sbxp4juiw6qp>
On Fri, Apr 03, 2026 at 05:08:00PM +0800, Joseph Qi wrote:
> Move the extent list l_count validation from ocfs2_dx_dir_lookup_rec()
> into ocfs2_validate_dx_root(), so that corrupted on-disk fields are
> caught early at block read time rather than during directory lookups.
> 
> Additionally, add a l_next_free_rec <= l_count check to prevent
> out-of-bounds access when iterating over extent records.
> 
> Both checks are skipped for inline dx roots (OCFS2_DX_FLAG_INLINE),
> which use dr_entries instead of dr_list.
> 
> Signed-off-by: Joseph Qi <[email protected]>

LGTM
Reviewed-by: Heming Zhao <[email protected]>
> ---
>  fs/ocfs2/dir.c | 34 +++++++++++++++++++++++++---------
>  1 file changed, 25 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
> index 1c8abf2c592c..82e720c8ba32 100644
> --- a/fs/ocfs2/dir.c
> +++ b/fs/ocfs2/dir.c
> @@ -593,7 +593,7 @@ static int ocfs2_validate_dx_root(struct super_block *sb,
>  		mlog(ML_ERROR,
>  		     "Checksum failed for dir index root block %llu\n",
>  		     (unsigned long long)bh->b_blocknr);
> -		return ret;
> +		goto bail;
>  	}
>  
>  	if (!OCFS2_IS_VALID_DX_ROOT(dx_root)) {
> @@ -601,8 +601,32 @@ static int ocfs2_validate_dx_root(struct super_block *sb,
>  				  "Dir Index Root # %llu has bad signature %.*s\n",
>  				  (unsigned long long)le64_to_cpu(dx_root->dr_blkno),
>  				  7, dx_root->dr_signature);
> +		goto bail;
> +	}
> +
> +	if (!(dx_root->dr_flags & OCFS2_DX_FLAG_INLINE)) {
> +		struct ocfs2_extent_list *el = &dx_root->dr_list;
> +
> +		if (le16_to_cpu(el->l_count) != ocfs2_extent_recs_per_dx_root(sb)) {
> +			ret = ocfs2_error(sb,
> +					  "Dir Index Root # %llu has invalid l_count %u (expected %u)\n",
> +					  (unsigned long long)le64_to_cpu(dx_root->dr_blkno),
> +					  le16_to_cpu(el->l_count),
> +					  ocfs2_extent_recs_per_dx_root(sb));
> +			goto bail;
> +		}
> +
> +		if (le16_to_cpu(el->l_next_free_rec) > le16_to_cpu(el->l_count)) {
> +			ret = ocfs2_error(sb,
> +					  "Dir Index Root # %llu has invalid l_next_free_rec %u (l_count %u)\n",
> +					  (unsigned long long)le64_to_cpu(dx_root->dr_blkno),
> +					  le16_to_cpu(el->l_next_free_rec),
> +					  le16_to_cpu(el->l_count));
> +			goto bail;
> +		}
>  	}
>  
> +bail:
>  	return ret;
>  }
>  
> @@ -791,14 +815,6 @@ static int ocfs2_dx_dir_lookup_rec(struct inode *inode,
>  	struct ocfs2_extent_block *eb;
>  	struct ocfs2_extent_rec *rec = NULL;
>  
> -	if (le16_to_cpu(el->l_count) !=
> -	    ocfs2_extent_recs_per_dx_root(inode->i_sb)) {
> -		ret = ocfs2_error(inode->i_sb,
> -				  "Inode %lu has invalid extent list length %u\n",
> -				  inode->i_ino, le16_to_cpu(el->l_count));
> -		goto out;
> -	}
> -
>  	if (el->l_tree_depth) {
>  		ret = ocfs2_find_leaf(INODE_CACHE(inode), el, major_hash,
>  				      &eb_bh);
> -- 
> 2.39.3
>
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.