Messages in this thread | | | Date | Mon, 17 Oct 2022 15:02:02 -0500 | Subject | Re: [PATCH v2] fs: jfs: fix shift-out-of-bounds in dbAllocAG | From | Dave Kleikamp <> |
| |
Looks good. Pushed to jfs-next.
Thanks, Shaggy
On 10/15/22 8:25PM, 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 64 and trigger the shift-out-of-bounds. > > Fix this bug by adding a check of bmp->db_agl2size in dbMount since this > field is used in many following functions. The upper bound for this > field is MAXMAPSIZE - L2MAXAG, thanks for the help of Dave Kleikamp. > Notethat, for maintainance, I reorganzie error handling code of dbMount. > > Reported-by: syzbot+15342c1aa6a00fb7a438@syzkaller.appspotmail.com > Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com> > --- > v1->2: fix the size of bmp->db_agl2size; revise the correct upper bound > fs/jfs/jfs_dmap.c | 22 ++++++++++++++++------ > 1 file changed, 16 insertions(+), 6 deletions(-) > > diff --git a/fs/jfs/jfs_dmap.c b/fs/jfs/jfs_dmap.c > index 6b838d3ae7c2..962c855631cb 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,11 @@ 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 > 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 +218,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; > } > >
| |