[lvc-project] [PATCH] ocfs2: consistently validate extent lists in __ocfs2_find_path()

[Heming Zhao]

On Tue, Dec 16, 2025 at 02:19:07PM +0300, Dmitry Antipov wrote:
> Introduce 'ocfs2_validate_extent_list()' to validate all of the extent
> lists processed by '__ocfs2_find_path()' and reject ones of an invalid
> length, whatever it comes from a regular inode or from an extent block.
> 
> Reported-by: syzbot+151afab124dfbc5f15e6 at syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=151afab124dfbc5f15e6
> Signed-off-by: Dmitry Antipov <dmantipov at yandex.ru>

Hi,

As I mentioned in another thread [1], all __ocfs2_find_path() operations occur
in memory, Please don't introduce I/O into this function.
btw, I have also provided a fix for this syzbot issue in the same thread [1].

[1]:
https://lore.kernel.org/ocfs2-devel/693fd627.a70a0220.33cd7b.00f4.GAE@google.com/T/#t

Thanks,
Heming
> ---
>  fs/ocfs2/alloc.c | 86 +++++++++++++++++++++++++++++++++---------------
>  1 file changed, 59 insertions(+), 27 deletions(-)
> 
> diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c
> index 58bf58b68955..22e2de4fb3b0 100644
> --- a/fs/ocfs2/alloc.c
> +++ b/fs/ocfs2/alloc.c
> @@ -1784,6 +1784,53 @@ int ocfs2_find_subtree_root(struct ocfs2_extent_tree *et,
>  
>  typedef void (path_insert_t)(void *, struct buffer_head *);
>  
> +/*
> + * Validate an extent list read from the regular inode or from an extent block.
> + */
> +static int ocfs2_validate_extent_list(struct ocfs2_caching_info *ci,
> +				      struct super_block *sb,
> +				      struct ocfs2_extent_list *el,
> +				      struct buffer_head *bh)
> +{
> +	unsigned long long owner = ocfs2_metadata_cache_owner(ci);
> +	u8 signature[8];
> +	int count;
> +
> +	if (le16_to_cpu(el->l_tree_depth) >= OCFS2_MAX_PATH_DEPTH) {
> +		ocfs2_error(sb, "Owner %llu has an extent list with invalid tree depth %u\n",
> +			    owner, le16_to_cpu(el->l_tree_depth));
> +		return 1;
> +	}
> +
> +	if (OCFS2_IS_VALID_DINODE((struct ocfs2_dinode *)bh->b_data))
> +		count = ocfs2_extent_recs_per_inode_with_xattr
> +			(sb, (struct ocfs2_dinode *)bh->b_data);
> +	else if (OCFS2_IS_VALID_EXTENT_BLOCK((struct ocfs2_extent_block *)bh->b_data))
> +		count = ocfs2_extent_recs_per_eb(sb);
> +	else {
> +		/* Should be NUL terminated to print it safely. */
> +		strscpy(signature, bh->b_data);
> +		ocfs2_error(sb, "Owner %llu has unexpected signature '%s'\n",
> +			    owner, signature);
> +		return 1;
> +	}
> +
> +	if (le16_to_cpu(el->l_count) != count) {
> +		ocfs2_error(sb, "Owner %llu has an extent list with invalid length %u\n",
> +			    owner, le16_to_cpu(el->l_count));
> +		return 1;
> +	}
> +
> +	if (le16_to_cpu(el->l_next_free_rec) == 0 ||
> +	    (le16_to_cpu(el->l_next_free_rec) > count)) {
> +		ocfs2_error(sb, "Owner %llu has an extent list with invalid unused slot %u\n",
> +			    owner, le16_to_cpu(el->l_next_free_rec));
> +		return 1;
> +	}
> +
> +	return 0;
> +}
> +
>  /*
>   * Traverse a btree path in search of cpos, starting at root_el.
>   *
> @@ -1797,31 +1844,23 @@ static int __ocfs2_find_path(struct ocfs2_caching_info *ci,
>  	int i, ret = 0;
>  	u32 range;
>  	u64 blkno;
> -	struct buffer_head *bh = NULL;
>  	struct ocfs2_extent_block *eb;
>  	struct ocfs2_extent_list *el;
>  	struct ocfs2_extent_rec *rec;
> +	struct super_block *sb = ocfs2_metadata_cache_get_super(ci);
> +	struct buffer_head *bh = sb_getblk(sb, ocfs2_metadata_cache_owner(ci));
>  
> +	if (!bh) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
>  	el = root_el;
> -	while (el->l_tree_depth) {
> -		if (unlikely(le16_to_cpu(el->l_tree_depth) >= OCFS2_MAX_PATH_DEPTH)) {
> -			ocfs2_error(ocfs2_metadata_cache_get_super(ci),
> -				    "Owner %llu has invalid tree depth %u in extent list\n",
> -				    (unsigned long long)ocfs2_metadata_cache_owner(ci),
> -				    le16_to_cpu(el->l_tree_depth));
> -			ret = -EROFS;
> -			goto out;
> -		}
> -		if (le16_to_cpu(el->l_next_free_rec) == 0) {
> -			ocfs2_error(ocfs2_metadata_cache_get_super(ci),
> -				    "Owner %llu has empty extent list at depth %u\n",
> -				    (unsigned long long)ocfs2_metadata_cache_owner(ci),
> -				    le16_to_cpu(el->l_tree_depth));
> -			ret = -EROFS;
> -			goto out;
> -
> -		}
> +	if (ocfs2_validate_extent_list(ci, sb, el, bh)) {
> +		ret = -EROFS;
> +		goto out;
> +	}
>  
> +	while (el->l_tree_depth) {
>  		for(i = 0; i < le16_to_cpu(el->l_next_free_rec) - 1; i++) {
>  			rec = &el->l_recs[i];
>  
> @@ -1857,14 +1896,7 @@ static int __ocfs2_find_path(struct ocfs2_caching_info *ci,
>  		eb = (struct ocfs2_extent_block *) bh->b_data;
>  		el = &eb->h_list;
>  
> -		if (le16_to_cpu(el->l_next_free_rec) >
> -		    le16_to_cpu(el->l_count)) {
> -			ocfs2_error(ocfs2_metadata_cache_get_super(ci),
> -				    "Owner %llu has bad count in extent list at block %llu (next free=%u, count=%u)\n",
> -				    (unsigned long long)ocfs2_metadata_cache_owner(ci),
> -				    (unsigned long long)bh->b_blocknr,
> -				    le16_to_cpu(el->l_next_free_rec),
> -				    le16_to_cpu(el->l_count));
> +		if (ocfs2_validate_extent_list(ci, sb, el, bh)) {
>  			ret = -EROFS;
>  			goto out;
>  		}
> -- 
> 2.52.0
>