mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/4] ocfs2: validate suballoc slot and bit of metadata blocks
@ 2026-08-31  6:28 Joseph Qi
  2026-08-31  6:28 ` [PATCH 1/4] ocfs2: restrict OCFS2_INVALID_SLOT suballoc slot to system inodes Joseph Qi
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Joseph Qi @ 2026-08-31  6:28 UTC (permalink / raw)
  To: Andrew Morton, Heming Zhao
  Cc: Mark Fasheh, Joel Becker, ocfs2-devel, linux-kernel

ocfs2 metadata validators trust the on-disk suballoc slot (and the
dinode suballoc bit) without checking them against the mounted
filesystem's slot range.  A corrupted image can carry
OCFS2_INVALID_SLOT or another out-of-range value; when the
corresponding inode, extent block, xattr block, or dir index root is
later freed, the unvalidated slot/bit is passed to
ocfs2_get_system_file_inode() or _ocfs2_free_suballoc_bits() and hits
BUG_ON() or indexes local_system_inodes[] out of bounds.

Mounting ocfs2 requires privilege (ocfs2 cannot be mounted from a
user namespace), so this needs a privileged mount of a crafted or
corrupted image -- e.g. an administrator mounting untrusted media or a
disk image supplied to a virtual machine.  Once such an image is
mounted, an unprivileged user who can delete the corrupted object
triggers the crash.

This series rejects such values at read time, in the existing
validators, so corrupted objects fail with -EROFS (and a read-only
remount) instead of crashing:

  patch 1 restricts OCFS2_INVALID_SLOT dinodes to system inodes,
        completing fe7a283b3916 ("ocfs2: add suballoc slot check in
        ocfs2_validate_inode_block()");
  patch 2 rejects oversized dinode suballoc bits;
  patch 3 validates the suballoc slot of xattr and dir index blocks;
  patch 4 validates the suballoc slot of extent blocks.

All checks match what the kernel itself writes (always a valid slot
from meta_ac->ac_alloc_slot) and what mkfs.ocfs2/libocfs2 write
(system inodes carry OCFS2_INVALID_SLOT with OCFS2_SYSTEM_FL, extent
blocks carry slot 0), so legitimate filesystems are unaffected.

Tested with negative corruption testing under QEMU: for each new
check a test image was generated with the field overwritten by an
out-of-range value; with the series applied the access fails cleanly
with -EROFS and the filesystem remounts read-only instead of hitting
BUG_ON().

Joseph Qi (4):
  ocfs2: restrict OCFS2_INVALID_SLOT suballoc slot to system inodes
  ocfs2: validate suballoc bit during inode read
  ocfs2: validate suballoc slot of xattr and dir index blocks
  ocfs2: validate suballoc slot of extent blocks

 fs/ocfs2/alloc.c | 14 ++++++++++++++
 fs/ocfs2/dir.c   | 14 ++++++++++++++
 fs/ocfs2/inode.c | 33 +++++++++++++++++++++++++++++++--
 fs/ocfs2/xattr.c | 13 +++++++++++++
 4 files changed, 72 insertions(+), 2 deletions(-)

-- 
2.39.3


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/4] ocfs2: restrict OCFS2_INVALID_SLOT suballoc slot to system inodes
  2026-08-31  6:28 [PATCH 0/4] ocfs2: validate suballoc slot and bit of metadata blocks Joseph Qi
@ 2026-08-31  6:28 ` Joseph Qi
  2026-08-31 14:02   ` Heming Zhao
  2026-08-31  6:28 ` [PATCH 2/4] ocfs2: validate suballoc bit during inode read Joseph Qi
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Joseph Qi @ 2026-08-31  6:28 UTC (permalink / raw)
  To: Andrew Morton, Heming Zhao
  Cc: Mark Fasheh, Joel Becker, ocfs2-devel, linux-kernel

ocfs2_validate_inode_block() currently permits i_suballoc_slot to be
OCFS2_INVALID_SLOT for any dinode.  Only system inodes created by
mkfs.ocfs2 are allocated from the global allocator and thus
legitimately carry this value; regular inodes are always allocated
from a per-slot suballocator and hence must have a valid slot.

If a corrupted regular inode with OCFS2_INVALID_SLOT is accepted,
ocfs2_remove_inode() will pass the slot to ocfs2_get_system_file_inode()
and get_local_system_inode() will hit BUG_ON(slot == OCFS2_INVALID_SLOT)
when the inode is deleted.  This can be triggered by an unprivileged
user unlinking such a corrupted file.

Reject OCFS2_INVALID_SLOT for non-system dinodes during validation,
while still accepting it for system inodes.

Fixes: fe7a283b3916 ("ocfs2: add suballoc slot check in ocfs2_validate_inode_block()")
Cc: stable@vger.kernel.org
Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
---
 fs/ocfs2/inode.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/fs/ocfs2/inode.c b/fs/ocfs2/inode.c
index 180107a11046..eda50f13ffb5 100644
--- a/fs/ocfs2/inode.c
+++ b/fs/ocfs2/inode.c
@@ -1520,8 +1520,23 @@ int ocfs2_validate_inode_block(struct super_block *sb,
 		goto bail;
 	}
 
-	if (le16_to_cpu(di->i_suballoc_slot) != (u16)OCFS2_INVALID_SLOT &&
-	    (u32)le16_to_cpu(di->i_suballoc_slot) > OCFS2_SB(sb)->max_slots - 1) {
+	/*
+	 * Only system inodes created by mkfs.ocfs2 are allocated from the
+	 * global allocator and thus legitimately carry OCFS2_INVALID_SLOT.
+	 * Regular inodes are always allocated from a per-slot suballocator.
+	 * If a regular inode with OCFS2_INVALID_SLOT was accepted here,
+	 * deleting it would pass the slot to get_local_system_inode() via
+	 * ocfs2_remove_inode() and trigger BUG_ON(slot == OCFS2_INVALID_SLOT).
+	 */
+	if (le16_to_cpu(di->i_suballoc_slot) == (u16)OCFS2_INVALID_SLOT) {
+		if (!(le32_to_cpu(di->i_flags) & OCFS2_SYSTEM_FL)) {
+			rc = ocfs2_error(sb,
+					 "Invalid dinode %llu: suballoc slot %u for non-system inode\n",
+					 (unsigned long long)bh->b_blocknr,
+					 le16_to_cpu(di->i_suballoc_slot));
+			goto bail;
+		}
+	} else if ((u32)le16_to_cpu(di->i_suballoc_slot) > OCFS2_SB(sb)->max_slots - 1) {
 		rc = ocfs2_error(sb, "Invalid dinode %llu: suballoc slot %u\n",
 				 (unsigned long long)bh->b_blocknr,
 				 le16_to_cpu(di->i_suballoc_slot));
-- 
2.39.3


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/4] ocfs2: validate suballoc bit during inode read
  2026-08-31  6:28 [PATCH 0/4] ocfs2: validate suballoc slot and bit of metadata blocks Joseph Qi
  2026-08-31  6:28 ` [PATCH 1/4] ocfs2: restrict OCFS2_INVALID_SLOT suballoc slot to system inodes Joseph Qi
@ 2026-08-31  6:28 ` Joseph Qi
  2026-08-31 14:03   ` Heming Zhao
  2026-08-31  6:28 ` [PATCH 3/4] ocfs2: validate suballoc slot of xattr and dir index blocks Joseph Qi
  2026-08-31  6:28 ` [PATCH 4/4] ocfs2: validate suballoc slot of extent blocks Joseph Qi
  3 siblings, 1 reply; 12+ messages in thread
From: Joseph Qi @ 2026-08-31  6:28 UTC (permalink / raw)
  To: Andrew Morton, Heming Zhao
  Cc: Mark Fasheh, Joel Becker, ocfs2-devel, linux-kernel

i_suballoc_bit of a dinode is currently not validated at all.  A
corrupted dinode can carry an abnormally large i_suballoc_bit, which
bypasses ocfs2_validate_inode_block().  When the inode is deleted,
ocfs2_remove_inode() calls ocfs2_free_dinode(), which passes the
unvalidated bit to _ocfs2_free_suballoc_bits() and triggers
BUG_ON((count + start_bit) > ocfs2_bits_per_group(cl)).

Since suballocator block group bitmaps are contained in a single
block, a valid suballoc bit must be smaller than the number of bits
per block.  Reject oversized i_suballoc_bit values during dinode
validation.

Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
---
 fs/ocfs2/inode.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/fs/ocfs2/inode.c b/fs/ocfs2/inode.c
index eda50f13ffb5..a982c99a9678 100644
--- a/fs/ocfs2/inode.c
+++ b/fs/ocfs2/inode.c
@@ -1543,6 +1543,20 @@ int ocfs2_validate_inode_block(struct super_block *sb,
 		goto bail;
 	}
 
+	/*
+	 * A suballocator block group bitmap is contained in a single block,
+	 * so a valid suballoc bit can never exceed the number of bits per
+	 * block.  Otherwise deleting the inode will pass the oversized bit
+	 * to _ocfs2_free_suballoc_bits() via ocfs2_free_dinode() and trigger
+	 * BUG_ON((count + start_bit) > ocfs2_bits_per_group(cl)).
+	 */
+	if (le16_to_cpu(di->i_suballoc_bit) >= sb->s_blocksize * 8) {
+		rc = ocfs2_error(sb, "Invalid dinode %llu: suballoc bit %u\n",
+				 (unsigned long long)bh->b_blocknr,
+				 le16_to_cpu(di->i_suballoc_bit));
+		goto bail;
+	}
+
 	if ((le32_to_cpu(di->i_flags) & OCFS2_ORPHANED_FL) &&
 	    le16_to_cpu(di->i_orphaned_slot) >= OCFS2_SB(sb)->max_slots) {
 		rc = ocfs2_error(sb, "Invalid dinode %llu: orphaned slot %u\n",
-- 
2.39.3


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 3/4] ocfs2: validate suballoc slot of xattr and dir index blocks
  2026-08-31  6:28 [PATCH 0/4] ocfs2: validate suballoc slot and bit of metadata blocks Joseph Qi
  2026-08-31  6:28 ` [PATCH 1/4] ocfs2: restrict OCFS2_INVALID_SLOT suballoc slot to system inodes Joseph Qi
  2026-08-31  6:28 ` [PATCH 2/4] ocfs2: validate suballoc bit during inode read Joseph Qi
@ 2026-08-31  6:28 ` Joseph Qi
  2026-08-31 14:04   ` Heming Zhao
  2026-08-31  6:28 ` [PATCH 4/4] ocfs2: validate suballoc slot of extent blocks Joseph Qi
  3 siblings, 1 reply; 12+ messages in thread
From: Joseph Qi @ 2026-08-31  6:28 UTC (permalink / raw)
  To: Andrew Morton, Heming Zhao
  Cc: Mark Fasheh, Joel Becker, ocfs2-devel, linux-kernel

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@linux.alibaba.com>
---
 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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 4/4] ocfs2: validate suballoc slot of extent blocks
  2026-08-31  6:28 [PATCH 0/4] ocfs2: validate suballoc slot and bit of metadata blocks Joseph Qi
                   ` (2 preceding siblings ...)
  2026-08-31  6:28 ` [PATCH 3/4] ocfs2: validate suballoc slot of xattr and dir index blocks Joseph Qi
@ 2026-08-31  6:28 ` Joseph Qi
  2026-08-31 14:04   ` Heming Zhao
  3 siblings, 1 reply; 12+ messages in thread
From: Joseph Qi @ 2026-08-31  6:28 UTC (permalink / raw)
  To: Andrew Morton, Heming Zhao
  Cc: Mark Fasheh, Joel Becker, ocfs2-devel, linux-kernel

ocfs2_validate_extent_block() does not validate h_suballoc_slot against
the mounted filesystem's slot range.  Since extent blocks are allocated
from a per-slot suballocator at runtime, their suballoc slot must be
within range.

Otherwise a corrupted image can carry an out-of-range slot.  When the
extent block is freed, ocfs2_cache_extent_block_free() caches it and
ocfs2_free_cached_blocks() later passes the unvalidated slot to
ocfs2_get_system_file_inode(), so 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@linux.alibaba.com>
---
 fs/ocfs2/alloc.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c
index be09e766ac1f..2abd242a438f 100644
--- a/fs/ocfs2/alloc.c
+++ b/fs/ocfs2/alloc.c
@@ -925,6 +925,20 @@ static int ocfs2_validate_extent_block(struct super_block *sb,
 		goto bail;
 	}
 
+	/*
+	 * Extent 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(eb->h_suballoc_slot) >= OCFS2_SB(sb)->max_slots) {
+		rc = ocfs2_error(sb,
+				 "Extent block #%llu has an invalid h_suballoc_slot of %u\n",
+				 (unsigned long long)bh->b_blocknr,
+				 le16_to_cpu(eb->h_suballoc_slot));
+		goto bail;
+	}
+
 	if (le16_to_cpu(eb->h_list.l_count) != ocfs2_extent_recs_per_eb(sb)) {
 		rc = ocfs2_error(sb,
 				 "Extent block #%llu has invalid l_count %u (expected %u)\n",
-- 
2.39.3


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/4] ocfs2: restrict OCFS2_INVALID_SLOT suballoc slot to system inodes
  2026-08-31  6:28 ` [PATCH 1/4] ocfs2: restrict OCFS2_INVALID_SLOT suballoc slot to system inodes Joseph Qi
@ 2026-08-31 14:02   ` Heming Zhao
  2026-09-01  1:08     ` Joseph Qi
  0 siblings, 1 reply; 12+ messages in thread
From: Heming Zhao @ 2026-08-31 14:02 UTC (permalink / raw)
  To: Joseph Qi
  Cc: Andrew Morton, Mark Fasheh, Joel Becker, ocfs2-devel, linux-kernel

On Mon, Aug 31, 2026 at 02:28:45PM +0800, Joseph Qi wrote:
> ocfs2_validate_inode_block() currently permits i_suballoc_slot to be
> OCFS2_INVALID_SLOT for any dinode.  Only system inodes created by
> mkfs.ocfs2 are allocated from the global allocator and thus
> legitimately carry this value; regular inodes are always allocated
> from a per-slot suballocator and hence must have a valid slot.
> 
> If a corrupted regular inode with OCFS2_INVALID_SLOT is accepted,
> ocfs2_remove_inode() will pass the slot to ocfs2_get_system_file_inode()
> and get_local_system_inode() will hit BUG_ON(slot == OCFS2_INVALID_SLOT)
> when the inode is deleted.  This can be triggered by an unprivileged
> user unlinking such a corrupted file.
> 
> Reject OCFS2_INVALID_SLOT for non-system dinodes during validation,
> while still accepting it for system inodes.
> 
> Fixes: fe7a283b3916 ("ocfs2: add suballoc slot check in ocfs2_validate_inode_block()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>

LGTM.
Reviewed-by: Heming Zhao <heming.zhao@suse.com>
> ---
>  fs/ocfs2/inode.c | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ocfs2/inode.c b/fs/ocfs2/inode.c
> index 180107a11046..eda50f13ffb5 100644
> --- a/fs/ocfs2/inode.c
> +++ b/fs/ocfs2/inode.c
> @@ -1520,8 +1520,23 @@ int ocfs2_validate_inode_block(struct super_block *sb,
>  		goto bail;
>  	}
>  
> -	if (le16_to_cpu(di->i_suballoc_slot) != (u16)OCFS2_INVALID_SLOT &&
> -	    (u32)le16_to_cpu(di->i_suballoc_slot) > OCFS2_SB(sb)->max_slots - 1) {
> +	/*
> +	 * Only system inodes created by mkfs.ocfs2 are allocated from the
> +	 * global allocator and thus legitimately carry OCFS2_INVALID_SLOT.
> +	 * Regular inodes are always allocated from a per-slot suballocator.
> +	 * If a regular inode with OCFS2_INVALID_SLOT was accepted here,
> +	 * deleting it would pass the slot to get_local_system_inode() via
> +	 * ocfs2_remove_inode() and trigger BUG_ON(slot == OCFS2_INVALID_SLOT).
> +	 */
> +	if (le16_to_cpu(di->i_suballoc_slot) == (u16)OCFS2_INVALID_SLOT) {
> +		if (!(le32_to_cpu(di->i_flags) & OCFS2_SYSTEM_FL)) {
> +			rc = ocfs2_error(sb,
> +					 "Invalid dinode %llu: suballoc slot %u for non-system inode\n",
> +					 (unsigned long long)bh->b_blocknr,
> +					 le16_to_cpu(di->i_suballoc_slot));
> +			goto bail;
> +		}
> +	} else if ((u32)le16_to_cpu(di->i_suballoc_slot) > OCFS2_SB(sb)->max_slots - 1) {
>  		rc = ocfs2_error(sb, "Invalid dinode %llu: suballoc slot %u\n",
>  				 (unsigned long long)bh->b_blocknr,
>  				 le16_to_cpu(di->i_suballoc_slot));
> -- 
> 2.39.3
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/4] ocfs2: validate suballoc bit during inode read
  2026-08-31  6:28 ` [PATCH 2/4] ocfs2: validate suballoc bit during inode read Joseph Qi
@ 2026-08-31 14:03   ` Heming Zhao
  0 siblings, 0 replies; 12+ messages in thread
From: Heming Zhao @ 2026-08-31 14:03 UTC (permalink / raw)
  To: Joseph Qi
  Cc: Andrew Morton, Mark Fasheh, Joel Becker, ocfs2-devel, linux-kernel

On Mon, Aug 31, 2026 at 02:28:46PM +0800, Joseph Qi wrote:
> i_suballoc_bit of a dinode is currently not validated at all.  A
> corrupted dinode can carry an abnormally large i_suballoc_bit, which
> bypasses ocfs2_validate_inode_block().  When the inode is deleted,
> ocfs2_remove_inode() calls ocfs2_free_dinode(), which passes the
> unvalidated bit to _ocfs2_free_suballoc_bits() and triggers
> BUG_ON((count + start_bit) > ocfs2_bits_per_group(cl)).
> 
> Since suballocator block group bitmaps are contained in a single
> block, a valid suballoc bit must be smaller than the number of bits
> per block.  Reject oversized i_suballoc_bit values during dinode
> validation.
> 
> Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>

LGTM.
Reviewed-by: Heming Zhao <heming.zhao@suse.com>
> ---
>  fs/ocfs2/inode.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/fs/ocfs2/inode.c b/fs/ocfs2/inode.c
> index eda50f13ffb5..a982c99a9678 100644
> --- a/fs/ocfs2/inode.c
> +++ b/fs/ocfs2/inode.c
> @@ -1543,6 +1543,20 @@ int ocfs2_validate_inode_block(struct super_block *sb,
>  		goto bail;
>  	}
>  
> +	/*
> +	 * A suballocator block group bitmap is contained in a single block,
> +	 * so a valid suballoc bit can never exceed the number of bits per
> +	 * block.  Otherwise deleting the inode will pass the oversized bit
> +	 * to _ocfs2_free_suballoc_bits() via ocfs2_free_dinode() and trigger
> +	 * BUG_ON((count + start_bit) > ocfs2_bits_per_group(cl)).
> +	 */
> +	if (le16_to_cpu(di->i_suballoc_bit) >= sb->s_blocksize * 8) {
> +		rc = ocfs2_error(sb, "Invalid dinode %llu: suballoc bit %u\n",
> +				 (unsigned long long)bh->b_blocknr,
> +				 le16_to_cpu(di->i_suballoc_bit));
> +		goto bail;
> +	}
> +
>  	if ((le32_to_cpu(di->i_flags) & OCFS2_ORPHANED_FL) &&
>  	    le16_to_cpu(di->i_orphaned_slot) >= OCFS2_SB(sb)->max_slots) {
>  		rc = ocfs2_error(sb, "Invalid dinode %llu: orphaned slot %u\n",
> -- 
> 2.39.3
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/4] ocfs2: validate suballoc slot of xattr and dir index blocks
  2026-08-31  6:28 ` [PATCH 3/4] ocfs2: validate suballoc slot of xattr and dir index blocks Joseph Qi
@ 2026-08-31 14:04   ` Heming Zhao
  0 siblings, 0 replies; 12+ messages in thread
From: Heming Zhao @ 2026-08-31 14:04 UTC (permalink / raw)
  To: Joseph Qi
  Cc: Andrew Morton, Mark Fasheh, Joel Becker, ocfs2-devel, linux-kernel

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@linux.alibaba.com>

LGTM, thanks for the contribution.
Reviewed-by: Heming Zhao <heming.zhao@suse.com>
> ---
>  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
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/4] ocfs2: validate suballoc slot of extent blocks
  2026-08-31  6:28 ` [PATCH 4/4] ocfs2: validate suballoc slot of extent blocks Joseph Qi
@ 2026-08-31 14:04   ` Heming Zhao
  0 siblings, 0 replies; 12+ messages in thread
From: Heming Zhao @ 2026-08-31 14:04 UTC (permalink / raw)
  To: Joseph Qi
  Cc: Andrew Morton, Mark Fasheh, Joel Becker, ocfs2-devel, linux-kernel

On Mon, Aug 31, 2026 at 02:28:48PM +0800, Joseph Qi wrote:
> ocfs2_validate_extent_block() does not validate h_suballoc_slot against
> the mounted filesystem's slot range.  Since extent blocks are allocated
> from a per-slot suballocator at runtime, their suballoc slot must be
> within range.
> 
> Otherwise a corrupted image can carry an out-of-range slot.  When the
> extent block is freed, ocfs2_cache_extent_block_free() caches it and
> ocfs2_free_cached_blocks() later passes the unvalidated slot to
> ocfs2_get_system_file_inode(), so 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@linux.alibaba.com>

LGTM.
Reviewed-by: Heming Zhao <heming.zhao@suse.com>
> ---
>  fs/ocfs2/alloc.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c
> index be09e766ac1f..2abd242a438f 100644
> --- a/fs/ocfs2/alloc.c
> +++ b/fs/ocfs2/alloc.c
> @@ -925,6 +925,20 @@ static int ocfs2_validate_extent_block(struct super_block *sb,
>  		goto bail;
>  	}
>  
> +	/*
> +	 * Extent 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(eb->h_suballoc_slot) >= OCFS2_SB(sb)->max_slots) {
> +		rc = ocfs2_error(sb,
> +				 "Extent block #%llu has an invalid h_suballoc_slot of %u\n",
> +				 (unsigned long long)bh->b_blocknr,
> +				 le16_to_cpu(eb->h_suballoc_slot));
> +		goto bail;
> +	}
> +
>  	if (le16_to_cpu(eb->h_list.l_count) != ocfs2_extent_recs_per_eb(sb)) {
>  		rc = ocfs2_error(sb,
>  				 "Extent block #%llu has invalid l_count %u (expected %u)\n",
> -- 
> 2.39.3
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/4] ocfs2: restrict OCFS2_INVALID_SLOT suballoc slot to system inodes
  2026-08-31 14:02   ` Heming Zhao
@ 2026-09-01  1:08     ` Joseph Qi
  2026-09-01  1:16       ` Heming Zhao
  0 siblings, 1 reply; 12+ messages in thread
From: Joseph Qi @ 2026-09-01  1:08 UTC (permalink / raw)
  To: Heming Zhao
  Cc: Andrew Morton, Mark Fasheh, Joel Becker, ocfs2-devel, linux-kernel



On 8/31/26 10:02 PM, Heming Zhao wrote:
> On Mon, Aug 31, 2026 at 02:28:45PM +0800, Joseph Qi wrote:
>> ocfs2_validate_inode_block() currently permits i_suballoc_slot to be
>> OCFS2_INVALID_SLOT for any dinode.  Only system inodes created by
>> mkfs.ocfs2 are allocated from the global allocator and thus
>> legitimately carry this value; regular inodes are always allocated
>> from a per-slot suballocator and hence must have a valid slot.
>>
>> If a corrupted regular inode with OCFS2_INVALID_SLOT is accepted,
>> ocfs2_remove_inode() will pass the slot to ocfs2_get_system_file_inode()
>> and get_local_system_inode() will hit BUG_ON(slot == OCFS2_INVALID_SLOT)
>> when the inode is deleted.  This can be triggered by an unprivileged
>> user unlinking such a corrupted file.
>>
>> Reject OCFS2_INVALID_SLOT for non-system dinodes during validation,
>> while still accepting it for system inodes.
>>
>> Fixes: fe7a283b3916 ("ocfs2: add suballoc slot check in ocfs2_validate_inode_block()")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> 
> LGTM.
> Reviewed-by: Heming Zhao <heming.zhao@suse.com>

Thanks, sashiko has some review comments, I'll fix them and send v2 later.

Joseph

>> ---
>>  fs/ocfs2/inode.c | 19 +++++++++++++++++--
>>  1 file changed, 17 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/ocfs2/inode.c b/fs/ocfs2/inode.c
>> index 180107a11046..eda50f13ffb5 100644
>> --- a/fs/ocfs2/inode.c
>> +++ b/fs/ocfs2/inode.c
>> @@ -1520,8 +1520,23 @@ int ocfs2_validate_inode_block(struct super_block *sb,
>>  		goto bail;
>>  	}
>>  
>> -	if (le16_to_cpu(di->i_suballoc_slot) != (u16)OCFS2_INVALID_SLOT &&
>> -	    (u32)le16_to_cpu(di->i_suballoc_slot) > OCFS2_SB(sb)->max_slots - 1) {
>> +	/*
>> +	 * Only system inodes created by mkfs.ocfs2 are allocated from the
>> +	 * global allocator and thus legitimately carry OCFS2_INVALID_SLOT.
>> +	 * Regular inodes are always allocated from a per-slot suballocator.
>> +	 * If a regular inode with OCFS2_INVALID_SLOT was accepted here,
>> +	 * deleting it would pass the slot to get_local_system_inode() via
>> +	 * ocfs2_remove_inode() and trigger BUG_ON(slot == OCFS2_INVALID_SLOT).
>> +	 */
>> +	if (le16_to_cpu(di->i_suballoc_slot) == (u16)OCFS2_INVALID_SLOT) {
>> +		if (!(le32_to_cpu(di->i_flags) & OCFS2_SYSTEM_FL)) {
>> +			rc = ocfs2_error(sb,
>> +					 "Invalid dinode %llu: suballoc slot %u for non-system inode\n",
>> +					 (unsigned long long)bh->b_blocknr,
>> +					 le16_to_cpu(di->i_suballoc_slot));
>> +			goto bail;
>> +		}
>> +	} else if ((u32)le16_to_cpu(di->i_suballoc_slot) > OCFS2_SB(sb)->max_slots - 1) {
>>  		rc = ocfs2_error(sb, "Invalid dinode %llu: suballoc slot %u\n",
>>  				 (unsigned long long)bh->b_blocknr,
>>  				 le16_to_cpu(di->i_suballoc_slot));
>> -- 
>> 2.39.3
>>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/4] ocfs2: restrict OCFS2_INVALID_SLOT suballoc slot to system inodes
  2026-09-01  1:08     ` Joseph Qi
@ 2026-09-01  1:16       ` Heming Zhao
  2026-09-01  2:08         ` Joseph Qi
  0 siblings, 1 reply; 12+ messages in thread
From: Heming Zhao @ 2026-09-01  1:16 UTC (permalink / raw)
  To: Joseph Qi
  Cc: Andrew Morton, Mark Fasheh, Joel Becker, ocfs2-devel, linux-kernel

On Tue, Sep 01, 2026 at 09:08:26AM +0800, Joseph Qi wrote:
> 
> 
> On 8/31/26 10:02 PM, Heming Zhao wrote:
> > On Mon, Aug 31, 2026 at 02:28:45PM +0800, Joseph Qi wrote:
> >> ocfs2_validate_inode_block() currently permits i_suballoc_slot to be
> >> OCFS2_INVALID_SLOT for any dinode.  Only system inodes created by
> >> mkfs.ocfs2 are allocated from the global allocator and thus
> >> legitimately carry this value; regular inodes are always allocated
> >> from a per-slot suballocator and hence must have a valid slot.
> >>
> >> If a corrupted regular inode with OCFS2_INVALID_SLOT is accepted,
> >> ocfs2_remove_inode() will pass the slot to ocfs2_get_system_file_inode()
> >> and get_local_system_inode() will hit BUG_ON(slot == OCFS2_INVALID_SLOT)
> >> when the inode is deleted.  This can be triggered by an unprivileged
> >> user unlinking such a corrupted file.
> >>
> >> Reject OCFS2_INVALID_SLOT for non-system dinodes during validation,
> >> while still accepting it for system inodes.
> >>
> >> Fixes: fe7a283b3916 ("ocfs2: add suballoc slot check in ocfs2_validate_inode_block()")
> >> Cc: stable@vger.kernel.org
> >> Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> > 
> > LGTM.
> > Reviewed-by: Heming Zhao <heming.zhao@suse.com>
> 
> Thanks, sashiko has some review comments, I'll fix them and send v2 later.
> 
> Joseph

It seems sashiko review comments only go to the patch submitter/author.
In my view, they're also sent to ocfs2-devel@lists.linux.dev, which is nice as 
it lets others on the list see them."

- Heming
> 
> >> ---
> >>  fs/ocfs2/inode.c | 19 +++++++++++++++++--
> >>  1 file changed, 17 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/fs/ocfs2/inode.c b/fs/ocfs2/inode.c
> >> index 180107a11046..eda50f13ffb5 100644
> >> --- a/fs/ocfs2/inode.c
> >> +++ b/fs/ocfs2/inode.c
> >> @@ -1520,8 +1520,23 @@ int ocfs2_validate_inode_block(struct super_block *sb,
> >>  		goto bail;
> >>  	}
> >>  
> >> -	if (le16_to_cpu(di->i_suballoc_slot) != (u16)OCFS2_INVALID_SLOT &&
> >> -	    (u32)le16_to_cpu(di->i_suballoc_slot) > OCFS2_SB(sb)->max_slots - 1) {
> >> +	/*
> >> +	 * Only system inodes created by mkfs.ocfs2 are allocated from the
> >> +	 * global allocator and thus legitimately carry OCFS2_INVALID_SLOT.
> >> +	 * Regular inodes are always allocated from a per-slot suballocator.
> >> +	 * If a regular inode with OCFS2_INVALID_SLOT was accepted here,
> >> +	 * deleting it would pass the slot to get_local_system_inode() via
> >> +	 * ocfs2_remove_inode() and trigger BUG_ON(slot == OCFS2_INVALID_SLOT).
> >> +	 */
> >> +	if (le16_to_cpu(di->i_suballoc_slot) == (u16)OCFS2_INVALID_SLOT) {
> >> +		if (!(le32_to_cpu(di->i_flags) & OCFS2_SYSTEM_FL)) {
> >> +			rc = ocfs2_error(sb,
> >> +					 "Invalid dinode %llu: suballoc slot %u for non-system inode\n",
> >> +					 (unsigned long long)bh->b_blocknr,
> >> +					 le16_to_cpu(di->i_suballoc_slot));
> >> +			goto bail;
> >> +		}
> >> +	} else if ((u32)le16_to_cpu(di->i_suballoc_slot) > OCFS2_SB(sb)->max_slots - 1) {
> >>  		rc = ocfs2_error(sb, "Invalid dinode %llu: suballoc slot %u\n",
> >>  				 (unsigned long long)bh->b_blocknr,
> >>  				 le16_to_cpu(di->i_suballoc_slot));
> >> -- 
> >> 2.39.3
> >>
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/4] ocfs2: restrict OCFS2_INVALID_SLOT suballoc slot to system inodes
  2026-09-01  1:16       ` Heming Zhao
@ 2026-09-01  2:08         ` Joseph Qi
  0 siblings, 0 replies; 12+ messages in thread
From: Joseph Qi @ 2026-09-01  2:08 UTC (permalink / raw)
  To: Heming Zhao
  Cc: Andrew Morton, Mark Fasheh, Joel Becker, ocfs2-devel, linux-kernel



On 9/1/26 9:16 AM, Heming Zhao wrote:
> On Tue, Sep 01, 2026 at 09:08:26AM +0800, Joseph Qi wrote:
>>
>>
>> On 8/31/26 10:02 PM, Heming Zhao wrote:
>>> On Mon, Aug 31, 2026 at 02:28:45PM +0800, Joseph Qi wrote:
>>>> ocfs2_validate_inode_block() currently permits i_suballoc_slot to be
>>>> OCFS2_INVALID_SLOT for any dinode.  Only system inodes created by
>>>> mkfs.ocfs2 are allocated from the global allocator and thus
>>>> legitimately carry this value; regular inodes are always allocated
>>>> from a per-slot suballocator and hence must have a valid slot.
>>>>
>>>> If a corrupted regular inode with OCFS2_INVALID_SLOT is accepted,
>>>> ocfs2_remove_inode() will pass the slot to ocfs2_get_system_file_inode()
>>>> and get_local_system_inode() will hit BUG_ON(slot == OCFS2_INVALID_SLOT)
>>>> when the inode is deleted.  This can be triggered by an unprivileged
>>>> user unlinking such a corrupted file.
>>>>
>>>> Reject OCFS2_INVALID_SLOT for non-system dinodes during validation,
>>>> while still accepting it for system inodes.
>>>>
>>>> Fixes: fe7a283b3916 ("ocfs2: add suballoc slot check in ocfs2_validate_inode_block()")
>>>> Cc: stable@vger.kernel.org
>>>> Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
>>>
>>> LGTM.
>>> Reviewed-by: Heming Zhao <heming.zhao@suse.com>
>>
>> Thanks, sashiko has some review comments, I'll fix them and send v2 later.
>>
>> Joseph
> 
> It seems sashiko review comments only go to the patch submitter/author.
> In my view, they're also sent to ocfs2-devel@lists.linux.dev, which is nice as 
> it lets others on the list see them."
> 

Yes, now it only sends to author and maintainers.
I'll send a patch to cc list as well.

Thanks,
Joseph

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2026-09-01  2:08 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-31  6:28 [PATCH 0/4] ocfs2: validate suballoc slot and bit of metadata blocks Joseph Qi
2026-08-31  6:28 ` [PATCH 1/4] ocfs2: restrict OCFS2_INVALID_SLOT suballoc slot to system inodes Joseph Qi
2026-08-31 14:02   ` Heming Zhao
2026-09-01  1:08     ` Joseph Qi
2026-09-01  1:16       ` Heming Zhao
2026-09-01  2:08         ` Joseph Qi
2026-08-31  6:28 ` [PATCH 2/4] ocfs2: validate suballoc bit during inode read Joseph Qi
2026-08-31 14:03   ` Heming Zhao
2026-08-31  6:28 ` [PATCH 3/4] ocfs2: validate suballoc slot of xattr and dir index blocks Joseph Qi
2026-08-31 14:04   ` Heming Zhao
2026-08-31  6:28 ` [PATCH 4/4] ocfs2: validate suballoc slot of extent blocks Joseph Qi
2026-08-31 14:04   ` Heming Zhao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®