Re:Re: [PATCH] ocfs2: validate filecheck inode slots

[jiale yao]

At 2026-09-28 17:58:03, "Joseph Qi" <joseph.qi@xxxxxxxxxxxxxxxxx> wrote:
>
>
>On 9/26/26 9:53 PM, Jiale Yao wrote:
>> The online filecheck path reads inodes with
>> ocfs2_filecheck_validate_inode_block() instead of
>> ocfs2_validate_inode_block().  It does not check the slot fields that
>> are used as indices into arrays sized by osb->max_slots.
>> 
>> A corrupted dinode can set OCFS2_ORPHANED_FL or
>> OCFS2_DIO_ORPHANED_FL while carrying an out-of-range orphan slot.  An
>> out-of-range i_suballoc_slot can also bypass the normal validator
>> through the filecheck path.  These values can later be used to index
>> the slot-local system inode array.
>> 
>> Mirror the normal validator's slot checks in the filecheck validator.
>> Reject invalid slots in the repair path as well, since the correct
>> slot cannot be recovered.
>> 
>> Fixes: bb88131c9831 ("ocfs2: validate DIO orphan slot during inode read")
>
>Don't see why blames this commit.
>
>> Signed-off-by: Jiale Yao <yaojiale02@xxxxxxx>
>> ---
>>  fs/ocfs2/inode.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 45 insertions(+)
>> 
>> diff --git a/fs/ocfs2/inode.c b/fs/ocfs2/inode.c
>> index 180107a11046..ab82cf9146ef 100644
>> --- a/fs/ocfs2/inode.c
>> +++ b/fs/ocfs2/inode.c
>> @@ -1754,6 +1754,44 @@ int ocfs2_validate_inode_block(struct super_block *sb,
>>  	return rc;
>>  }
>>  
>> +/*
>> + * Validate the slot fields used as indices into arrays sized by
>> + * osb->max_slots.  The filecheck path uses this validator instead of
>> + * ocfs2_validate_inode_block(), so it must enforce the same bounds.
>> + */
>> +static int ocfs2_filecheck_validate_slots(struct super_block *sb,
>> +					  struct ocfs2_dinode *di,
>> +					  unsigned long long blkno)
>> +{
>> +	struct ocfs2_super *osb = OCFS2_SB(sb);
>> +
>> +	if (le16_to_cpu(di->i_suballoc_slot) != (u16)OCFS2_INVALID_SLOT &&
>> +	    (u32)le16_to_cpu(di->i_suballoc_slot) > osb->max_slots - 1) {
>> +		mlog(ML_ERROR,
>> +		     "Filecheck: invalid dinode #%llu: suballoc slot %u\n",
>> +		     blkno, le16_to_cpu(di->i_suballoc_slot));
>> +		return -OCFS2_FILECHECK_ERR_INVALIDINO;
>> +	}
>> +
>> +	if ((le32_to_cpu(di->i_flags) & OCFS2_ORPHANED_FL) &&
>> +	    le16_to_cpu(di->i_orphaned_slot) >= osb->max_slots) {
>> +		mlog(ML_ERROR,
>> +		     "Filecheck: invalid dinode #%llu: orphaned slot %u\n",
>> +		     blkno, le16_to_cpu(di->i_orphaned_slot));
>> +		return -OCFS2_FILECHECK_ERR_INVALIDINO;
>> +	}
>> +
>> +	if ((le32_to_cpu(di->i_flags) & OCFS2_DIO_ORPHANED_FL) &&
>> +	    le16_to_cpu(di->i_dio_orphaned_slot) >= osb->max_slots) {
>> +		mlog(ML_ERROR,
>> +		     "Filecheck: invalid dinode #%llu: DIO orphaned slot %u\n",
>> +		     blkno, le16_to_cpu(di->i_dio_orphaned_slot));
>> +		return -OCFS2_FILECHECK_ERR_INVALIDINO;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>
>The same comments with extent list filecheck patch.
>Please abtract common helpers for this.

How about this one,
https://lore.kernel.org/all/20261004070621.1514598-1-yaojiale02@xxxxxxx/

>
>Thanks,
>Joseph
>
>>  static int ocfs2_filecheck_validate_inode_block(struct super_block *sb,
>>  						struct buffer_head *bh)
>>  {
>> @@ -1835,6 +1873,10 @@ static int ocfs2_filecheck_validate_inode_block(struct super_block *sb,
>>  		goto bail;
>>  	}
>>  
>> +	rc = ocfs2_filecheck_validate_slots(sb, di, bh->b_blocknr);
>> +	if (rc)
>> +		goto bail;
>> +
>>  	if (ocfs2_dinode_has_size_without_clusters(sb, di)) {
>>  		if (S_ISDIR(le16_to_cpu(di->i_mode)))
>>  			mlog(ML_ERROR,
>> @@ -1893,6 +1935,9 @@ static int ocfs2_filecheck_repair_inode_block(struct super_block *sb,
>>  		return -OCFS2_FILECHECK_ERR_VALIDFLAG;
>>  	}
>>  
>> +	if (ocfs2_filecheck_validate_slots(sb, di, bh->b_blocknr))
>> +		return -OCFS2_FILECHECK_ERR_INVALIDINO;
>> +
>>  	if (le64_to_cpu(di->i_blkno) != bh->b_blocknr) {
>>  		di->i_blkno = cpu_to_le64(bh->b_blocknr);
>>  		changed = 1;