Messages in this thread Patch in this message | | | Date | Thu, 13 Oct 2022 15:55:29 -0500 | Subject | Re: [PATCH] fs: jfs: fix shift-out-of-bounds in dbAllocAG | From | Dave Kleikamp <> |
| |
On 9/29/22 12:44AM, Dongliang Mu wrote: > From: Dongliang Mu <mudongliangabcd@gmail.com> > > Syzbot found a crash : UBSAN: shift-out-of-bounds in dbAllocAG. The > underlying bug is the missing check of bmp->db_agl2size. The field can > be greater than 32 and trigger the shift-out-of-bounds.
The shift is done to a 64-byte value, not 32.
The volume can contain up to 2^43 blocks, which are divided up into 128 (2^7) aggregate groups, so each AG can have 2^36 blocks, which makes 36 a valid value for db_agl2size.
> > Fix this bug by adding a check of bmp->db_agl2size in dbMount since this > field is used in many following functions. Note that, for maintainance, > I reorganzie the error handling code of dbMount. > > Reported-by: syzbot+15342c1aa6a00fb7a438@syzkaller.appspotmail.com > Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com> > --- > fs/jfs/jfs_dmap.c | 21 +++++++++++++++------ > 1 file changed, 15 insertions(+), 6 deletions(-) > > diff --git a/fs/jfs/jfs_dmap.c b/fs/jfs/jfs_dmap.c > index 6b838d3ae7c2..4c717f245920 100644 > --- a/fs/jfs/jfs_dmap.c > +++ b/fs/jfs/jfs_dmap.c > @@ -155,7 +155,7 @@ int dbMount(struct inode *ipbmap) > struct bmap *bmp; > struct dbmap_disk *dbmp_le; > struct metapage *mp; > - int i; > + int i, err; > > /* > * allocate/initialize the in-memory bmap descriptor > @@ -170,8 +170,8 @@ int dbMount(struct inode *ipbmap) > BMAPBLKNO << JFS_SBI(ipbmap->i_sb)->l2nbperpage, > PSIZE, 0); > if (mp == NULL) { > - kfree(bmp); > - return -EIO; > + err = -EIO; > + goto err_kfree_bmp; > } > > /* copy the on-disk bmap descriptor to its in-memory version. */ > @@ -181,9 +181,8 @@ int dbMount(struct inode *ipbmap) > bmp->db_l2nbperpage = le32_to_cpu(dbmp_le->dn_l2nbperpage); > bmp->db_numag = le32_to_cpu(dbmp_le->dn_numag); > if (!bmp->db_numag) { > - release_metapage(mp); > - kfree(bmp); > - return -EINVAL; > + err = -EINVAL; > + goto err_release_metapage; > } > > bmp->db_maxlevel = le32_to_cpu(dbmp_le->dn_maxlevel); > @@ -194,6 +193,10 @@ int dbMount(struct inode *ipbmap) > bmp->db_agwidth = le32_to_cpu(dbmp_le->dn_agwidth); > bmp->db_agstart = le32_to_cpu(dbmp_le->dn_agstart); > bmp->db_agl2size = le32_to_cpu(dbmp_le->dn_agl2size); > + if (bmp->db_agl2size >= 32) {
I think the right number here is MAXMAPSIZE - L2MAXAG, so
if (bmp->db_agl2size > (MAXMAPSIZE - L2MAXAG)) {
> + err = -EINVAL; > + goto err_release_metapage; > + } > for (i = 0; i < MAXAG; i++) > bmp->db_agfree[i] = le64_to_cpu(dbmp_le->dn_agfree[i]); > bmp->db_agsize = le64_to_cpu(dbmp_le->dn_agsize); > @@ -214,6 +217,12 @@ int dbMount(struct inode *ipbmap) > BMAP_LOCK_INIT(bmp); > > return (0); > + > +err_release_metapage: > + release_metapage(mp); > +err_kfree_bmp: > + kfree(bmp); > + return err; > } > >
| |