Re: [PATCH 1/4] ocfs2: restrict OCFS2_INVALID_SLOT suballoc slot to system inodes

[Heming Zhao]

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@xxxxxxxxxxxxxxx
> >> Signed-off-by: Joseph Qi <joseph.qi@xxxxxxxxxxxxxxxxx>
> > 
> > LGTM.
> > Reviewed-by: Heming Zhao <heming.zhao@xxxxxxxx>
> 
> 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@xxxxxxxxxxxxxxx, 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
> >>
>