[PATCH] ufs: validate cylinder group free bitmap offset

[PATCH] ufs: validate cylinder group free bitmap offset

[Cen Zhang (Microsoft Security FORGE Labs)]

ufs_read_cylinder() copies cg_freeoff from the on-disk cylinder group
without checking that the free-fragment bitmap fits in the loaded
cylinder group buffers. A crafted image can place cg_freeoff past those
buffers so that allocation later indexes ubh->bh[] out of range.

UBSAN: array-index-out-of-bounds in fs/ufs/balloc.c:752:15
  ubh_scanc()              fs/ufs/balloc.c:752
  ufs_bitmap_search()      fs/ufs/balloc.c
  ufs_alloccg_block()      fs/ufs/balloc.c
  ufs_alloc_fragments()    fs/ufs/balloc.c
  ufs_new_fragments()      fs/ufs/balloc.c

Reject the cylinder group unless the full BITS_TO_BYTES(s_fpg)
free-fragment bitmap starting at c_freeoff fits in the bytes actually
loaded for that cylinder group.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: Xiang Mei (Microsoft) <xmei5@asu.edu>
Cc: AutonomousCodeSecurity@microsoft.com
Cc: stable@vger.kernel.org
Signed-off-by: Cen Zhang (Microsoft Security FORGE Labs) <cenzhang@linux.microsoft.com>
---
 fs/ufs/cylinder.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/fs/ufs/cylinder.c b/fs/ufs/cylinder.c
index a2813270c303..fec6c9dfb055 100644
--- a/fs/ufs/cylinder.c
+++ b/fs/ufs/cylinder.c
@@ -33,6 +33,8 @@ static bool ufs_read_cylinder(struct super_block *sb,
 	struct ufs_sb_private_info * uspi;
 	struct ufs_cg_private_info * ucpi;
 	struct ufs_cylinder_group * ucg;
+	u64 cg_bytes, free_bitmap_bytes;
+	const char *err;
 	unsigned i, j;
 
 	UFSD("ENTER, cgno %u, bitmap_nr %u\n", cgno, bitmap_nr);
@@ -48,8 +50,10 @@ static bool ufs_read_cylinder(struct super_block *sb,
 	UCPI_UBH(ucpi)->bh[0] = sbi->s_ucg[cgno];
 	for (i = 1; i < UCPI_UBH(ucpi)->count; i++) {
 		UCPI_UBH(ucpi)->bh[i] = sb_bread(sb, UCPI_UBH(ucpi)->fragment + i);
-		if (!UCPI_UBH(ucpi)->bh[i])
+		if (!UCPI_UBH(ucpi)->bh[i]) {
+			err = "can't read cylinder group block %u";
 			goto failed;
+		}
 	}
 	sbi->s_cgno[bitmap_nr] = cgno;
 			
@@ -68,6 +72,16 @@ static bool ufs_read_cylinder(struct super_block *sb,
 	ucpi->c_clustersumoff = fs32_to_cpu(sb, ucg->cg_u.cg_44.cg_clustersumoff);
 	ucpi->c_clusteroff = fs32_to_cpu(sb, ucg->cg_u.cg_44.cg_clusteroff);
 	ucpi->c_nclusterblks = fs32_to_cpu(sb, ucg->cg_u.cg_44.cg_nclusterblks);
+
+	cg_bytes = UCPI_UBH(ucpi)->count * sb->s_blocksize;
+	free_bitmap_bytes = BITS_TO_BYTES((u64)uspi->s_fpg);
+	/* The full free bitmap must fit in the loaded CG buffer. */
+	if (!free_bitmap_bytes ||
+	    (u64)ucpi->c_freeoff + free_bitmap_bytes > cg_bytes) {
+		err = "cylinder group %u has invalid free bitmap";
+		goto failed;
+	}
+
 	UFSD("EXIT\n");
 	return true;
 	
@@ -75,7 +89,7 @@ static bool ufs_read_cylinder(struct super_block *sb,
 	for (j = 1; j < i; j++)
 		brelse(UCPI_UBH(ucpi)->bh[j]);
 	sbi->s_cgno[bitmap_nr] = UFS_CGNO_EMPTY;
-	ufs_error (sb, "ufs_read_cylinder", "can't read cylinder group block %u", cgno);
+	ufs_error(sb, "ufs_read_cylinder", err, cgno);
 	return false;
 }
 

base-commit: 786262be6048deab760f68c8acc2c85607165894
-- 
2.55.0

Re: [PATCH] ufs: validate cylinder group free bitmap offset

[Greg KH]

On Tue, Sep 01, 2026 at 02:06:00PM -0400, Cen Zhang (Microsoft Security FORGE Labs) wrote:
> ufs_read_cylinder() copies cg_freeoff from the on-disk cylinder group
> without checking that the free-fragment bitmap fits in the loaded
> cylinder group buffers. A crafted image can place cg_freeoff past those
> buffers so that allocation later indexes ubh->bh[] out of range.
> 
> UBSAN: array-index-out-of-bounds in fs/ufs/balloc.c:752:15
>   ubh_scanc()              fs/ufs/balloc.c:752
>   ufs_bitmap_search()      fs/ufs/balloc.c
>   ufs_alloccg_block()      fs/ufs/balloc.c
>   ufs_alloc_fragments()    fs/ufs/balloc.c
>   ufs_new_fragments()      fs/ufs/balloc.c
> 
> Reject the cylinder group unless the full BITS_TO_BYTES(s_fpg)
> free-fragment bitmap starting at c_freeoff fits in the bytes actually
> loaded for that cylinder group.

Why not fix the userspace fsck tool to fix this instead of working
around it in the kernel?

thanks,

greg k-h