[PATCH] ocfs2: validate filecheck inode slots

[PATCH] ocfs2: validate filecheck inode slots

[Jiale Yao]

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")
Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 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;
+}
+
 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;
-- 
2.34.1

Re:[PATCH] ocfs2: validate filecheck inode slots

[jiale yao]

At 2026-09-26 21:53:53, "Jiale Yao" <yaojiale02@163.com> 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.

Reproduced on 7.3-rc4 (x86_64 QEMU, KASAN):

[   45.942783] BUG: KASAN: slab-out-of-bounds in ocfs2_evict_inode+0x28f7/0x3ac0
[   45.942818] Read of size 4 at addr ffff8881
[   45.942827] Call Trace:
[   45.942829]  dump_stack_lvl+0x2d/0x70
[   45.942833]  print_report+0x175/0x7d0
[   45.942836]  kasan_report+0x139/0x170
[   45.942839]  ocfs2_evict_inode+0x28f7/0x3ac0
[   45.942840]  evict+0x344/0x6e0
[   45.942842]  ? iput+0x45d/0x730
[   45.942843]  ocfs2_filecheck_attr_store+0x96a/0xd30
[   45.942846]  ocfs2_filecheck_store+0x5d/0x90
[   45.942847]  kernfs_fop_write_iter+0x262/0x370
[   45.942849]  vfs_write+0x933/0xcb0
[   45.942852]  ksys_write+0xf2/0x1b0
[   45.942853]  do_syscall_64+0x15b/0x420
[   45.942856]  entry_SYSCALL_64_after_hwframe
...
[   45.942867] Allocated by task 81:
[   45.942870]  __kasan_kmalloc+0x72/0x90
[   45.942871]  __kmalloc_noprof+0x1d2/0x470
[   45.942875]  ocfs2_fill_super+0x195b/0x59f0s, ...)
[   45.942885] The buggy address is located 4 bytes to the right of
[   45.942885]  allocated 4-byte region [ffff888105f04a40, ffff888105f04a44)
[   45.942910] Oops: general protection fault, probably for non-canonical
           address ... (the pointer read past inode)

>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")
>Signed-off-by: Jiale Yao <yaojiale02@163.com>
>---
> 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;
>+}
>+
> 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;
>-- 
>2.34.1

Re: [PATCH] ocfs2: validate filecheck inode slots

[Joseph Qi]

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@163.com>
> ---
>  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.

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;