ufs: validate cylinder group free bitmap offset
Subsystems (2)
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
@@ -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
Greg KH
maintainerQuoted 12 lines · Cen Zhang (Microsoft Security FORGE Labs)
ufs_read_cylinder()copiescg_freeofffrom the on-disk cylinder group without checking that the free-fragment bitmap fits in the loaded cylinder group buffers. A crafted image can placecg_freeoffpast 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 atc_freeofffits 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