Re: [PATCH 3/4] ocfs2: validate suballoc slot of xattr and dir index blocks

[Heming Zhao]

On Mon, Aug 31, 2026 at 02:28:47PM +0800, Joseph Qi wrote:
> ocfs2_validate_xattr_block() and ocfs2_validate_dx_root() do not
> validate xb_suballoc_slot and dr_suballoc_slot against the mounted
> filesystem's slot range.  Since xattr blocks and dir index root blocks
> are allocated from a per-slot suballocator at runtime, their suballoc
> slots must be within range.
> 
> Otherwise a corrupted image can carry an out-of-range slot.  When the
> xattr block or dir index is removed, ocfs2_xattr_block_remove() or
> ocfs2_dx_dir_remove_index() passes the unvalidated slot to
> ocfs2_get_system_file_inode() and get_local_system_inode() will either
> hit BUG_ON(slot == OCFS2_INVALID_SLOT) or compute an out-of-bounds
> index into the local_system_inodes array.
> 
> Reject out-of-range suballoc slots during validation.
> 
> Signed-off-by: Joseph Qi <joseph.qi@xxxxxxxxxxxxxxxxx>

LGTM, thanks for the contribution.
Reviewed-by: Heming Zhao <heming.zhao@xxxxxxxx>
> ---
>  fs/ocfs2/dir.c   | 14 ++++++++++++++
>  fs/ocfs2/xattr.c | 13 +++++++++++++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
> index 0075e1624310..eb9850610ba3 100644
> --- a/fs/ocfs2/dir.c
> +++ b/fs/ocfs2/dir.c
> @@ -605,6 +605,20 @@ static int ocfs2_validate_dx_root(struct super_block *sb,
>  		goto bail;
>  	}
>  
> +	/*
> +	 * Dir index root blocks are allocated from a per-slot suballocator,
> +	 * so the slot must be in range.  Otherwise removing the index passes
> +	 * it to get_local_system_inode(), which hits BUG_ON() for
> +	 * OCFS2_INVALID_SLOT or computes an out-of-bounds index otherwise.
> +	 */
> +	if ((u32)le16_to_cpu(dx_root->dr_suballoc_slot) >= OCFS2_SB(sb)->max_slots) {
> +		ret = ocfs2_error(sb,
> +				  "Dir Index Root # %llu has invalid dr_suballoc_slot %u\n",
> +				  (unsigned long long)le64_to_cpu(dx_root->dr_blkno),
> +				  le16_to_cpu(dx_root->dr_suballoc_slot));
> +		goto bail;
> +	}
> +
>  	if (!(dx_root->dr_flags & OCFS2_DX_FLAG_INLINE)) {
>  		struct ocfs2_extent_list *el = &dx_root->dr_list;
>  
> diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
> index 0062cbeb1e8b..5acc8091d06f 100644
> --- a/fs/ocfs2/xattr.c
> +++ b/fs/ocfs2/xattr.c
> @@ -517,6 +517,19 @@ static int ocfs2_validate_xattr_block(struct super_block *sb,
>  				   le32_to_cpu(xb->xb_fs_generation));
>  	}
>  
> +	/*
> +	 * Xattr blocks are allocated from a per-slot suballocator, so the
> +	 * slot must be in range.  Otherwise freeing the block passes it to
> +	 * get_local_system_inode(), which hits BUG_ON() for
> +	 * OCFS2_INVALID_SLOT or computes an out-of-bounds index otherwise.
> +	 */
> +	if ((u32)le16_to_cpu(xb->xb_suballoc_slot) >= OCFS2_SB(sb)->max_slots) {
> +		return ocfs2_error(sb,
> +				   "Extended attribute block #%llu has an invalid xb_suballoc_slot of %u\n",
> +				   (unsigned long long)bh->b_blocknr,
> +				   le16_to_cpu(xb->xb_suballoc_slot));
> +	}
> +
>  	if (!(le16_to_cpu(xb->xb_flags) & OCFS2_XATTR_INDEXED)) {
>  		size_t region_offset =
>  			offsetof(struct ocfs2_xattr_block, xb_attrs.xb_header);
> -- 
> 2.39.3
>