Skip to contentSkip to conversation

ufs: validate cylinder group free bitmap offset

PATCH In review Cen Zhang (Microsoft Security FORGE Labs) 2 participants 2 messages Sep 1 – Sep 2

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ufs: validate cylinder group free bitmap offset

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.

---
 fs/ufs/cylinder.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)
fs/ufs/cylinder.c +16 -2
index a2813270c303..fec6c9dfb055 100644
@@ -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

Signature · 1 line
2.55.0
maintainer
#1 lore raw
Quoted 12 lines · 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
Quoted 3 lines

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