The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH] ocfs2: validate rl_used against rl_count in refcount block validator
@ 2026-07-09 13:26 Ibrahim Hashimov
  2026-07-10 11:57 ` Joseph Qi
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Ibrahim Hashimov @ 2026-07-09 13:26 UTC (permalink / raw)
  To: Mark Fasheh, Joel Becker, Joseph Qi; +Cc: ocfs2-devel, linux-kernel, stable

ocfs2_find_refcount_rec_in_rl() walks the on-disk refcount record
array with:

	for (; i < le16_to_cpu(rb->rf_records.rl_used); i++) {
		rec = &rb->rf_records.rl_recs[i];
		...

rl_recs[] lives in a single metadata block (4096 bytes on the common
configuration), so its real capacity is fixed by
ocfs2_refcount_recs_per_rb(sb) (247 records for a 4K block with the
16-byte ocfs2_refcount_rec). rl_used and rl_count are both read
directly off disk by ocfs2_validate_refcount_block() and are never
checked against that capacity, nor against each other, before any
refcount/reflink/CoW operation walks the array.

A crafted (or corrupted) refcount block with rl_used == 0xffff makes
the loop above walk far past the end of the block, dereferencing
rl_recs[i] for i up to 65534. The resulting index is then handed to
the sibling ocfs2_insert_refcount_rec(), whose insert-shift does:

	if (index < le16_to_cpu(rf_list->rl_used))
		memmove(&rf_list->rl_recs[index + 1],
			&rf_list->rl_recs[index],
			(le16_to_cpu(rf_list->rl_used) - index) *
			 sizeof(struct ocfs2_refcount_rec));

i.e. a memmove() of up to (0xffff - index) * 16 bytes (~1 MiB) from an
offset already past the block. This is reachable from an ordinary
reflink (FICLONE) against a crafted/corrupted ocfs2 image: attaching
an extent whose cpos sorts past every real record in the leaf forces
the lookup to run off the end instead of returning early on a match.
The attacker model is local: CAP_SYS_ADMIN mounting a crafted or
corrupted ocfs2 image, or a raw write to the block device backing an
already-mounted ocfs2 filesystem.

ocfs2_validate_refcount_block() already validates the block's ECC,
signature, rf_blkno and rf_fs_generation, but never rl_count/rl_used
against the block's actual on-disk capacity. This is the same class
of gap that ocfs2_validate_extent_block() (fs/ocfs2/alloc.c) already
closes for the sibling extent-list header, which checks both the
record capacity and the "used" bound before any code walks
h_list.l_recs[]:

	if (le16_to_cpu(eb->h_list.l_count) != ocfs2_extent_recs_per_eb(sb)) {
		rc = ocfs2_error(...);
		goto bail;
	}

	if (le16_to_cpu(eb->h_list.l_next_free_rec) >
	    le16_to_cpu(eb->h_list.l_count)) {
		rc = ocfs2_error(...);
		goto bail;
	}

Add the equivalent pair of checks to ocfs2_validate_refcount_block():
reject a refcount block whose rl_count does not match the fixed
per-block capacity returned by ocfs2_refcount_recs_per_rb(), and
reject rl_used > rl_count. Both checks are skipped when
OCFS2_REFCOUNT_TREE_FL is set, because in that case the same union
bytes hold an ocfs2_extent_list (rf_list), not the refcount record
list (rf_records) -- that layout is already validated separately by
ocfs2_validate_extent_block() when the referenced extent block is
read. This mirrors the existing
"!(rb->rf_flags & OCFS2_REFCOUNT_TREE_FL)" guard used elsewhere in
this file (e.g. ocfs2_get_refcount_rec()) to decide whether
rf_records or rf_list is the live member of the union.

With this in place, a forged rl_used/rl_count is caught at block
validation time (ocfs2_error()), consistent with every other
corruption check in this function, instead of driving an
out-of-bounds read in ocfs2_find_refcount_rec_in_rl() and a
subsequent out-of-bounds memmove() in ocfs2_insert_refcount_rec().

Verified against a crafted image on a v6.19 KASAN (KASAN_GENERIC)
build: replaying the same reflink (FICLONE) reliably hit a KASAN
report in __ocfs2_increase_refcount()/ocfs2_insert_refcount_rec()
before this patch, and triggers no report once
ocfs2_validate_refcount_block() rejects the forged rl_used/rl_count.

Fixes: f2c870e3b12e ("ocfs2: Add ocfs2_read_refcount_block.")
Cc: stable@vger.kernel.org
Signed-off-by: Ibrahim Hashimov <security@auditcode.ai>
Assisted-by: AuditCode-AI:2026.07
---
 fs/ocfs2/refcounttree.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/fs/ocfs2/refcounttree.c b/fs/ocfs2/refcounttree.c
index 7323bde70caa..63d6cb326e30 100644
--- a/fs/ocfs2/refcounttree.c
+++ b/fs/ocfs2/refcounttree.c
@@ -116,6 +116,33 @@ static int ocfs2_validate_refcount_block(struct super_block *sb,
 				 le32_to_cpu(rb->rf_fs_generation));
 		goto out;
 	}
+
+	/*
+	 * rf_records (rl_count/rl_used/rl_recs[]) is only meaningful when
+	 * this block is not an interior tree block (OCFS2_REFCOUNT_TREE_FL);
+	 * in that case the same union bytes hold an extent list (rf_list)
+	 * instead, which is validated by ocfs2_validate_extent_block().
+	 */
+	if (!(le32_to_cpu(rb->rf_flags) & OCFS2_REFCOUNT_TREE_FL)) {
+		if (le16_to_cpu(rb->rf_records.rl_count) !=
+		    ocfs2_refcount_recs_per_rb(sb)) {
+			rc = ocfs2_error(sb,
+					 "Refcount block #%llu has an invalid rl_count of %u\n",
+					 (unsigned long long)bh->b_blocknr,
+					 le16_to_cpu(rb->rf_records.rl_count));
+			goto out;
+		}
+
+		if (le16_to_cpu(rb->rf_records.rl_used) >
+		    le16_to_cpu(rb->rf_records.rl_count)) {
+			rc = ocfs2_error(sb,
+					 "Refcount block #%llu has an invalid rl_used of %u (rl_count %u)\n",
+					 (unsigned long long)bh->b_blocknr,
+					 le16_to_cpu(rb->rf_records.rl_used),
+					 le16_to_cpu(rb->rf_records.rl_count));
+			goto out;
+		}
+	}
 out:
 	return rc;
 }
-- 
2.50.1 (Apple Git-155)


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

* Re: [PATCH] ocfs2: validate rl_used against rl_count in refcount block validator
  2026-07-09 13:26 [PATCH] ocfs2: validate rl_used against rl_count in refcount block validator Ibrahim Hashimov
@ 2026-07-10 11:57 ` Joseph Qi
  2026-07-10 13:18 ` Joseph Qi
  2026-07-10 20:33 ` Andrew Morton
  2 siblings, 0 replies; 4+ messages in thread
From: Joseph Qi @ 2026-07-10 11:57 UTC (permalink / raw)
  To: Ibrahim Hashimov
  Cc: Mark Fasheh, Joel Becker, ocfs2-devel, linux-kernel, stable



On 7/9/26 9:26 PM, Ibrahim Hashimov wrote:
> ocfs2_find_refcount_rec_in_rl() walks the on-disk refcount record
> array with:
> 
> 	for (; i < le16_to_cpu(rb->rf_records.rl_used); i++) {
> 		rec = &rb->rf_records.rl_recs[i];
> 		...
> 
> rl_recs[] lives in a single metadata block (4096 bytes on the common
> configuration), so its real capacity is fixed by
> ocfs2_refcount_recs_per_rb(sb) (247 records for a 4K block with the
> 16-byte ocfs2_refcount_rec). rl_used and rl_count are both read
> directly off disk by ocfs2_validate_refcount_block() and are never
> checked against that capacity, nor against each other, before any
> refcount/reflink/CoW operation walks the array.
> 
> A crafted (or corrupted) refcount block with rl_used == 0xffff makes
> the loop above walk far past the end of the block, dereferencing
> rl_recs[i] for i up to 65534. The resulting index is then handed to
> the sibling ocfs2_insert_refcount_rec(), whose insert-shift does:
> 
> 	if (index < le16_to_cpu(rf_list->rl_used))
> 		memmove(&rf_list->rl_recs[index + 1],
> 			&rf_list->rl_recs[index],
> 			(le16_to_cpu(rf_list->rl_used) - index) *
> 			 sizeof(struct ocfs2_refcount_rec));
> 
> i.e. a memmove() of up to (0xffff - index) * 16 bytes (~1 MiB) from an
> offset already past the block. This is reachable from an ordinary
> reflink (FICLONE) against a crafted/corrupted ocfs2 image: attaching
> an extent whose cpos sorts past every real record in the leaf forces
> the lookup to run off the end instead of returning early on a match.
> The attacker model is local: CAP_SYS_ADMIN mounting a crafted or
> corrupted ocfs2 image, or a raw write to the block device backing an
> already-mounted ocfs2 filesystem.
> 
> ocfs2_validate_refcount_block() already validates the block's ECC,
> signature, rf_blkno and rf_fs_generation, but never rl_count/rl_used
> against the block's actual on-disk capacity. This is the same class
> of gap that ocfs2_validate_extent_block() (fs/ocfs2/alloc.c) already
> closes for the sibling extent-list header, which checks both the
> record capacity and the "used" bound before any code walks
> h_list.l_recs[]:
> 
> 	if (le16_to_cpu(eb->h_list.l_count) != ocfs2_extent_recs_per_eb(sb)) {
> 		rc = ocfs2_error(...);
> 		goto bail;
> 	}
> 
> 	if (le16_to_cpu(eb->h_list.l_next_free_rec) >
> 	    le16_to_cpu(eb->h_list.l_count)) {
> 		rc = ocfs2_error(...);
> 		goto bail;
> 	}
> 
> Add the equivalent pair of checks to ocfs2_validate_refcount_block():
> reject a refcount block whose rl_count does not match the fixed
> per-block capacity returned by ocfs2_refcount_recs_per_rb(), and
> reject rl_used > rl_count. Both checks are skipped when
> OCFS2_REFCOUNT_TREE_FL is set, because in that case the same union
> bytes hold an ocfs2_extent_list (rf_list), not the refcount record
> list (rf_records) -- that layout is already validated separately by
> ocfs2_validate_extent_block() when the referenced extent block is
> read. This mirrors the existing
> "!(rb->rf_flags & OCFS2_REFCOUNT_TREE_FL)" guard used elsewhere in
> this file (e.g. ocfs2_get_refcount_rec()) to decide whether
> rf_records or rf_list is the live member of the union.
> 
> With this in place, a forged rl_used/rl_count is caught at block
> validation time (ocfs2_error()), consistent with every other
> corruption check in this function, instead of driving an
> out-of-bounds read in ocfs2_find_refcount_rec_in_rl() and a
> subsequent out-of-bounds memmove() in ocfs2_insert_refcount_rec().
> 
> Verified against a crafted image on a v6.19 KASAN (KASAN_GENERIC)
> build: replaying the same reflink (FICLONE) reliably hit a KASAN
> report in __ocfs2_increase_refcount()/ocfs2_insert_refcount_rec()
> before this patch, and triggers no report once
> ocfs2_validate_refcount_block() rejects the forged rl_used/rl_count.
> 
> Fixes: f2c870e3b12e ("ocfs2: Add ocfs2_read_refcount_block.")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ibrahim Hashimov <security@auditcode.ai>
> Assisted-by: AuditCode-AI:2026.07

Looks good.
Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>

> ---
>  fs/ocfs2/refcounttree.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/fs/ocfs2/refcounttree.c b/fs/ocfs2/refcounttree.c
> index 7323bde70caa..63d6cb326e30 100644
> --- a/fs/ocfs2/refcounttree.c
> +++ b/fs/ocfs2/refcounttree.c
> @@ -116,6 +116,33 @@ static int ocfs2_validate_refcount_block(struct super_block *sb,
>  				 le32_to_cpu(rb->rf_fs_generation));
>  		goto out;
>  	}
> +
> +	/*
> +	 * rf_records (rl_count/rl_used/rl_recs[]) is only meaningful when
> +	 * this block is not an interior tree block (OCFS2_REFCOUNT_TREE_FL);
> +	 * in that case the same union bytes hold an extent list (rf_list)
> +	 * instead, which is validated by ocfs2_validate_extent_block().
> +	 */
> +	if (!(le32_to_cpu(rb->rf_flags) & OCFS2_REFCOUNT_TREE_FL)) {
> +		if (le16_to_cpu(rb->rf_records.rl_count) !=
> +		    ocfs2_refcount_recs_per_rb(sb)) {
> +			rc = ocfs2_error(sb,
> +					 "Refcount block #%llu has an invalid rl_count of %u\n",
> +					 (unsigned long long)bh->b_blocknr,
> +					 le16_to_cpu(rb->rf_records.rl_count));
> +			goto out;
> +		}
> +
> +		if (le16_to_cpu(rb->rf_records.rl_used) >
> +		    le16_to_cpu(rb->rf_records.rl_count)) {
> +			rc = ocfs2_error(sb,
> +					 "Refcount block #%llu has an invalid rl_used of %u (rl_count %u)\n",
> +					 (unsigned long long)bh->b_blocknr,
> +					 le16_to_cpu(rb->rf_records.rl_used),
> +					 le16_to_cpu(rb->rf_records.rl_count));
> +			goto out;
> +		}
> +	}
>  out:
>  	return rc;
>  }


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

* Re: [PATCH] ocfs2: validate rl_used against rl_count in refcount block validator
  2026-07-09 13:26 [PATCH] ocfs2: validate rl_used against rl_count in refcount block validator Ibrahim Hashimov
  2026-07-10 11:57 ` Joseph Qi
@ 2026-07-10 13:18 ` Joseph Qi
  2026-07-10 20:33 ` Andrew Morton
  2 siblings, 0 replies; 4+ messages in thread
From: Joseph Qi @ 2026-07-10 13:18 UTC (permalink / raw)
  To: Ibrahim Hashimov, akpm
  Cc: Mark Fasheh, Joel Becker, ocfs2-devel, linux-kernel



On 7/9/26 9:26 PM, Ibrahim Hashimov wrote:
> ocfs2_find_refcount_rec_in_rl() walks the on-disk refcount record
> array with:
> 
> 	for (; i < le16_to_cpu(rb->rf_records.rl_used); i++) {
> 		rec = &rb->rf_records.rl_recs[i];
> 		...
> 
> rl_recs[] lives in a single metadata block (4096 bytes on the common
> configuration), so its real capacity is fixed by
> ocfs2_refcount_recs_per_rb(sb) (247 records for a 4K block with the
> 16-byte ocfs2_refcount_rec). rl_used and rl_count are both read
> directly off disk by ocfs2_validate_refcount_block() and are never
> checked against that capacity, nor against each other, before any
> refcount/reflink/CoW operation walks the array.
> 
> A crafted (or corrupted) refcount block with rl_used == 0xffff makes
> the loop above walk far past the end of the block, dereferencing
> rl_recs[i] for i up to 65534. The resulting index is then handed to
> the sibling ocfs2_insert_refcount_rec(), whose insert-shift does:
> 
> 	if (index < le16_to_cpu(rf_list->rl_used))
> 		memmove(&rf_list->rl_recs[index + 1],
> 			&rf_list->rl_recs[index],
> 			(le16_to_cpu(rf_list->rl_used) - index) *
> 			 sizeof(struct ocfs2_refcount_rec));
> 
> i.e. a memmove() of up to (0xffff - index) * 16 bytes (~1 MiB) from an
> offset already past the block. This is reachable from an ordinary
> reflink (FICLONE) against a crafted/corrupted ocfs2 image: attaching
> an extent whose cpos sorts past every real record in the leaf forces
> the lookup to run off the end instead of returning early on a match.
> The attacker model is local: CAP_SYS_ADMIN mounting a crafted or
> corrupted ocfs2 image, or a raw write to the block device backing an
> already-mounted ocfs2 filesystem.
> 
> ocfs2_validate_refcount_block() already validates the block's ECC,
> signature, rf_blkno and rf_fs_generation, but never rl_count/rl_used
> against the block's actual on-disk capacity. This is the same class
> of gap that ocfs2_validate_extent_block() (fs/ocfs2/alloc.c) already
> closes for the sibling extent-list header, which checks both the
> record capacity and the "used" bound before any code walks
> h_list.l_recs[]:
> 
> 	if (le16_to_cpu(eb->h_list.l_count) != ocfs2_extent_recs_per_eb(sb)) {
> 		rc = ocfs2_error(...);
> 		goto bail;
> 	}
> 
> 	if (le16_to_cpu(eb->h_list.l_next_free_rec) >
> 	    le16_to_cpu(eb->h_list.l_count)) {
> 		rc = ocfs2_error(...);
> 		goto bail;
> 	}
> 
> Add the equivalent pair of checks to ocfs2_validate_refcount_block():
> reject a refcount block whose rl_count does not match the fixed
> per-block capacity returned by ocfs2_refcount_recs_per_rb(), and
> reject rl_used > rl_count. Both checks are skipped when
> OCFS2_REFCOUNT_TREE_FL is set, because in that case the same union
> bytes hold an ocfs2_extent_list (rf_list), not the refcount record
> list (rf_records) -- that layout is already validated separately by
> ocfs2_validate_extent_block() when the referenced extent block is
> read. This mirrors the existing
> "!(rb->rf_flags & OCFS2_REFCOUNT_TREE_FL)" guard used elsewhere in
> this file (e.g. ocfs2_get_refcount_rec()) to decide whether
> rf_records or rf_list is the live member of the union.
> 
> With this in place, a forged rl_used/rl_count is caught at block
> validation time (ocfs2_error()), consistent with every other
> corruption check in this function, instead of driving an
> out-of-bounds read in ocfs2_find_refcount_rec_in_rl() and a
> subsequent out-of-bounds memmove() in ocfs2_insert_refcount_rec().
> 
> Verified against a crafted image on a v6.19 KASAN (KASAN_GENERIC)
> build: replaying the same reflink (FICLONE) reliably hit a KASAN
> report in __ocfs2_increase_refcount()/ocfs2_insert_refcount_rec()
> before this patch, and triggers no report once
> ocfs2_validate_refcount_block() rejects the forged rl_used/rl_count.
> 
> Fixes: f2c870e3b12e ("ocfs2: Add ocfs2_read_refcount_block.")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ibrahim Hashimov <security@auditcode.ai>
> Assisted-by: AuditCode-AI:2026.07

Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> ---
>  fs/ocfs2/refcounttree.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/fs/ocfs2/refcounttree.c b/fs/ocfs2/refcounttree.c
> index 7323bde70caa..63d6cb326e30 100644
> --- a/fs/ocfs2/refcounttree.c
> +++ b/fs/ocfs2/refcounttree.c
> @@ -116,6 +116,33 @@ static int ocfs2_validate_refcount_block(struct super_block *sb,
>  				 le32_to_cpu(rb->rf_fs_generation));
>  		goto out;
>  	}
> +
> +	/*
> +	 * rf_records (rl_count/rl_used/rl_recs[]) is only meaningful when
> +	 * this block is not an interior tree block (OCFS2_REFCOUNT_TREE_FL);
> +	 * in that case the same union bytes hold an extent list (rf_list)
> +	 * instead, which is validated by ocfs2_validate_extent_block().
> +	 */
> +	if (!(le32_to_cpu(rb->rf_flags) & OCFS2_REFCOUNT_TREE_FL)) {
> +		if (le16_to_cpu(rb->rf_records.rl_count) !=
> +		    ocfs2_refcount_recs_per_rb(sb)) {
> +			rc = ocfs2_error(sb,
> +					 "Refcount block #%llu has an invalid rl_count of %u\n",
> +					 (unsigned long long)bh->b_blocknr,
> +					 le16_to_cpu(rb->rf_records.rl_count));
> +			goto out;
> +		}
> +
> +		if (le16_to_cpu(rb->rf_records.rl_used) >
> +		    le16_to_cpu(rb->rf_records.rl_count)) {
> +			rc = ocfs2_error(sb,
> +					 "Refcount block #%llu has an invalid rl_used of %u (rl_count %u)\n",
> +					 (unsigned long long)bh->b_blocknr,
> +					 le16_to_cpu(rb->rf_records.rl_used),
> +					 le16_to_cpu(rb->rf_records.rl_count));
> +			goto out;
> +		}
> +	}
>  out:
>  	return rc;
>  }


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

* Re: [PATCH] ocfs2: validate rl_used against rl_count in refcount block validator
  2026-07-09 13:26 [PATCH] ocfs2: validate rl_used against rl_count in refcount block validator Ibrahim Hashimov
  2026-07-10 11:57 ` Joseph Qi
  2026-07-10 13:18 ` Joseph Qi
@ 2026-07-10 20:33 ` Andrew Morton
  2 siblings, 0 replies; 4+ messages in thread
From: Andrew Morton @ 2026-07-10 20:33 UTC (permalink / raw)
  To: Ibrahim Hashimov
  Cc: Mark Fasheh, Joel Becker, Joseph Qi, ocfs2-devel, linux-kernel,
	stable

On Thu,  9 Jul 2026 15:26:09 +0200 Ibrahim Hashimov <security@auditcode.ai> wrote:

> ocfs2_find_refcount_rec_in_rl() walks the on-disk refcount record
> array with:
> 
> 	for (; i < le16_to_cpu(rb->rf_records.rl_used); i++) {
> 		rec = &rb->rf_records.rl_recs[i];
> 		...
> 
> rl_recs[] lives in a single metadata block (4096 bytes on the common
> configuration), so its real capacity is fixed by
> ocfs2_refcount_recs_per_rb(sb) (247 records for a 4K block with the
> 16-byte ocfs2_refcount_rec). rl_used and rl_count are both read
> directly off disk by ocfs2_validate_refcount_block() and are never
> checked against that capacity, nor against each other, before any
> refcount/reflink/CoW operation walks the array.
> 
> A crafted (or corrupted) refcount block with rl_used == 0xffff makes
> the loop above walk far past the end of the block, dereferencing
> rl_recs[i] for i up to 65534. The resulting index is then handed to
> the sibling ocfs2_insert_refcount_rec(), whose insert-shift does:
> 
> ...
>
> Add the equivalent pair of checks to ocfs2_validate_refcount_block():
>

Thanks.  AI review might have found another bug in there:
	https://sashiko.dev/#/patchset/20260709132609.44233-1-security@auditcode.ai



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

end of thread, other threads:[~2026-07-10 20:33 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-09 13:26 [PATCH] ocfs2: validate rl_used against rl_count in refcount block validator Ibrahim Hashimov
2026-07-10 11:57 ` Joseph Qi
2026-07-10 13:18 ` Joseph Qi
2026-07-10 20:33 ` Andrew Morton

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