From 898f706695682b9954f280d95e49fa86ffa55d08 Mon Sep 17 00:00:00 2001 From: Dongliang Mu Date: Tue, 18 Oct 2022 08:48:07 -0500 Subject: fs: jfs: fix shift-out-of-bounds in dbAllocAG 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 L2MAXL2SIZE - L2MAXAG, thanks for the help of Dave Kleikamp. Note that, for maintenance, I reorganized error handling code of dbMount. Reported-by: syzbot+15342c1aa6a00fb7a438@syzkaller.appspotmail.com Signed-off-by: Dongliang Mu Signed-off-by: Dave Kleikamp --- fs/jfs/jfs_dmap.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) (limited to 'fs/jfs') diff --git a/fs/jfs/jfs_dmap.c b/fs/jfs/jfs_dmap.c index 6b838d3ae7c2..e1cbfbb60303 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 > L2MAXL2SIZE - 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; } -- cgit v1.2.1 From 73c6da327ff17a07a9260b0ff1f36e13665ac63d Mon Sep 17 00:00:00 2001 From: Jiangshan Yi Date: Thu, 14 Jul 2022 10:56:56 +0800 Subject: fs/jfs: replace ternary operator with min_t() Fix the following coccicheck warning: fs/jfs/super.c:748: WARNING opportunity for min(). fs/jfs/super.c:788: WARNING opportunity for min(). min_t() macro is defined in include/linux/minmax.h. It avoids multiple evaluations of the arguments when non-constant and performs strict type-checking. Signed-off-by: Jiangshan Yi Signed-off-by: Dave Kleikamp --- fs/jfs/super.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'fs/jfs') diff --git a/fs/jfs/super.c b/fs/jfs/super.c index 85d4f44f2ac4..d2f82cb7db1b 100644 --- a/fs/jfs/super.c +++ b/fs/jfs/super.c @@ -745,8 +745,7 @@ static ssize_t jfs_quota_read(struct super_block *sb, int type, char *data, len = i_size-off; toread = len; while (toread > 0) { - tocopy = sb->s_blocksize - offset < toread ? - sb->s_blocksize - offset : toread; + tocopy = min_t(size_t, sb->s_blocksize - offset, toread); tmp_bh.b_state = 0; tmp_bh.b_size = i_blocksize(inode); @@ -785,8 +784,7 @@ static ssize_t jfs_quota_write(struct super_block *sb, int type, inode_lock(inode); while (towrite > 0) { - tocopy = sb->s_blocksize - offset < towrite ? - sb->s_blocksize - offset : towrite; + tocopy = min_t(size_t, sb->s_blocksize - offset, towrite); tmp_bh.b_state = 0; tmp_bh.b_size = i_blocksize(inode); -- cgit v1.2.1 From b0a35efa0ebc50032ab81d60ed5a12de4324c5e2 Mon Sep 17 00:00:00 2001 From: Jiangshan Yi Date: Fri, 2 Sep 2022 16:53:38 +0800 Subject: fs/jfs/jfs_xattr.h: Fix spelling typo in comment Fix spelling typo in comment. Reported-by: k2ci Signed-off-by: Jiangshan Yi Signed-off-by: Dave Kleikamp --- fs/jfs/jfs_xattr.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/jfs') diff --git a/fs/jfs/jfs_xattr.h b/fs/jfs/jfs_xattr.h index c50167a7bc50..0d33816d251d 100644 --- a/fs/jfs/jfs_xattr.h +++ b/fs/jfs/jfs_xattr.h @@ -25,7 +25,7 @@ struct jfs_ea_list { struct jfs_ea ea[]; /* Variable length list */ }; -/* Macros for defining maxiumum number of bytes supported for EAs */ +/* Macros for defining maximum number of bytes supported for EAs */ #define MAXEASIZE 65535 #define MAXEALISTSIZE MAXEASIZE -- cgit v1.2.1 From 1ea66d71b17608b49ddb907f6155bc47c6e33506 Mon Sep 17 00:00:00 2001 From: Gaosheng Cui Date: Fri, 9 Sep 2022 14:50:55 +0800 Subject: jfs: remove unused declarations for jfs extRealloc(), xtRelocate(), xtDelete() and extFill() have been removed since commit e471e5942c00 ("fs/jfs: Remove dead code"), so remove them. Signed-off-by: Gaosheng Cui Signed-off-by: Dave Kleikamp --- fs/jfs/jfs_extent.h | 2 -- fs/jfs/jfs_xtree.h | 4 ---- 2 files changed, 6 deletions(-) (limited to 'fs/jfs') diff --git a/fs/jfs/jfs_extent.h b/fs/jfs/jfs_extent.h index 1c984214e95e..a0ee4ccea66e 100644 --- a/fs/jfs/jfs_extent.h +++ b/fs/jfs/jfs_extent.h @@ -10,9 +10,7 @@ (addressPXD(&(JFS_IP(ip)->ixpxd)) + lengthPXD(&(JFS_IP(ip)->ixpxd)) - 1) extern int extAlloc(struct inode *, s64, s64, xad_t *, bool); -extern int extFill(struct inode *, xad_t *); extern int extHint(struct inode *, s64, xad_t *); -extern int extRealloc(struct inode *, s64, xad_t *, bool); extern int extRecord(struct inode *, xad_t *); #endif /* _H_JFS_EXTENT */ diff --git a/fs/jfs/jfs_xtree.h b/fs/jfs/jfs_xtree.h index 142caafc73b1..ad7592191d76 100644 --- a/fs/jfs/jfs_xtree.h +++ b/fs/jfs/jfs_xtree.h @@ -96,12 +96,8 @@ extern int xtInsert(tid_t tid, struct inode *ip, extern int xtExtend(tid_t tid, struct inode *ip, s64 xoff, int xlen, int flag); extern int xtUpdate(tid_t tid, struct inode *ip, struct xad *nxad); -extern int xtDelete(tid_t tid, struct inode *ip, s64 xoff, int xlen, - int flag); extern s64 xtTruncate(tid_t tid, struct inode *ip, s64 newsize, int type); extern s64 xtTruncate_pmap(tid_t tid, struct inode *ip, s64 committed_size); -extern int xtRelocate(tid_t tid, struct inode *ip, - xad_t * oxad, s64 nxaddr, int xtype); extern int xtAppend(tid_t tid, struct inode *ip, int xflag, s64 xoff, int maxblocks, int *xlenp, s64 * xaddrp, int flag); -- cgit v1.2.1 From dee8744524096514109a9680a9c45a4259dae2a4 Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Tue, 18 Oct 2022 16:23:10 +0100 Subject: jfs: remove redundant assignments to ipaimap and ipaimap2 The pointers ipaimap and ipaimap2 are re-assigned with values a second time with the same values when they were initialized. The re-assignments are redundant and can be removed. Cleans up two clang scan build warnings: fs/jfs/jfs_umount.c:42:16: warning: Value stored to 'ipaimap' during its initialization is never read [deadcode.DeadStores] fs/jfs/jfs_umount.c:43:16: warning: Value stored to 'ipaimap2' during its initialization is never read [deadcode.DeadStores] Signed-off-by: Colin Ian King Signed-off-by: Dave Kleikamp --- fs/jfs/jfs_umount.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'fs/jfs') diff --git a/fs/jfs/jfs_umount.c b/fs/jfs/jfs_umount.c index 3e8b13e6aa01..95ebcd17ce75 100644 --- a/fs/jfs/jfs_umount.c +++ b/fs/jfs/jfs_umount.c @@ -68,7 +68,6 @@ int jfs_umount(struct super_block *sb) /* * close secondary aggregate inode allocation map */ - ipaimap2 = sbi->ipaimap2; if (ipaimap2) { diUnmount(ipaimap2, 0); diFreeSpecial(ipaimap2); @@ -78,7 +77,6 @@ int jfs_umount(struct super_block *sb) /* * close aggregate inode allocation map */ - ipaimap = sbi->ipaimap; diUnmount(ipaimap, 0); diFreeSpecial(ipaimap); sbi->ipaimap = NULL; -- cgit v1.2.1 From ebe060369f8d6e4588b115f252bebf5ba4d64350 Mon Sep 17 00:00:00 2001 From: "Dr. David Alan Gilbert" Date: Sat, 22 Oct 2022 21:39:14 +0100 Subject: jfs: Fix fortify moan in symlink JFS has in jfs_incore.h: /* _inline may overflow into _inline_ea when needed */ /* _inline_ea may overlay the last part of * file._xtroot if maxentry = XTROOTINITSLOT */ union { struct { /* 128: inline symlink */ unchar _inline[128]; /* 128: inline extended attr */ unchar _inline_ea[128]; }; unchar _inline_all[256]; and currently the symlink code copies into _inline; if this is larger than 128 bytes it triggers a fortify warning of the form: memcpy: detected field-spanning write (size 132) of single field "ip->i_link" at fs/jfs/namei.c:950 (size 18446744073709551615) when it's actually OK. Copy it into _inline_all instead. Reported-by: syzbot+5fc38b2ddbbca7f5c680@syzkaller.appspotmail.com Signed-off-by: Dr. David Alan Gilbert Reviewed-by: Kees Cook Signed-off-by: Dave Kleikamp --- fs/jfs/namei.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/jfs') diff --git a/fs/jfs/namei.c b/fs/jfs/namei.c index 9db4f5789c0e..4fbbf88435e6 100644 --- a/fs/jfs/namei.c +++ b/fs/jfs/namei.c @@ -946,7 +946,7 @@ static int jfs_symlink(struct user_namespace *mnt_userns, struct inode *dip, if (ssize <= IDATASIZE) { ip->i_op = &jfs_fast_symlink_inode_operations; - ip->i_link = JFS_IP(ip)->i_inline; + ip->i_link = JFS_IP(ip)->i_inline_all; memcpy(ip->i_link, name, ssize); ip->i_size = ssize - 1; -- cgit v1.2.1 From 25e70c6162f207828dd405b432d8f2a98dbf7082 Mon Sep 17 00:00:00 2001 From: Hoi Pok Wu Date: Tue, 25 Oct 2022 23:20:45 +0800 Subject: fs: jfs: fix shift-out-of-bounds in dbDiscardAG This should be applied to most URSAN bugs found recently by syzbot, by guarding the dbMount. As syzbot feeding rubbish into the bmap descriptor. Signed-off-by: Hoi Pok Wu Signed-off-by: Dave Kleikamp --- fs/jfs/jfs_dmap.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'fs/jfs') diff --git a/fs/jfs/jfs_dmap.c b/fs/jfs/jfs_dmap.c index e1cbfbb60303..765838578a72 100644 --- a/fs/jfs/jfs_dmap.c +++ b/fs/jfs/jfs_dmap.c @@ -198,6 +198,11 @@ int dbMount(struct inode *ipbmap) goto err_release_metapage; } + if (((bmp->db_mapsize - 1) >> bmp->db_agl2size) > MAXAG) { + 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); -- cgit v1.2.1 From d0e482c45c50117bfb568825a41f0693e5f33c0f Mon Sep 17 00:00:00 2001 From: Oleg Kanatov Date: Fri, 28 Oct 2022 15:16:39 +0300 Subject: jfs: Fix a typo in function jfs_umount When closing the block allocation map, an incorrect pointer was NULL'ed. This commit fixes that. Signed-off-by: Oleg Kanatov Signed-off-by: Dave Kleikamp --- fs/jfs/jfs_umount.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/jfs') diff --git a/fs/jfs/jfs_umount.c b/fs/jfs/jfs_umount.c index 95ebcd17ce75..8ec43f53f686 100644 --- a/fs/jfs/jfs_umount.c +++ b/fs/jfs/jfs_umount.c @@ -87,7 +87,7 @@ int jfs_umount(struct super_block *sb) dbUnmount(ipbmap, 0); diFreeSpecial(ipbmap); - sbi->ipimap = NULL; + sbi->ipbmap = NULL; /* * Make sure all metadata makes it to disk before we mark -- cgit v1.2.1 From a60dca73a1a8079d867b2c2e9549440346c1ba83 Mon Sep 17 00:00:00 2001 From: Oleg Kanatov Date: Fri, 28 Oct 2022 15:22:54 +0300 Subject: jfs: makes diUnmount/diMount in jfs_mount_rw atomic jfs_mount_rw can call diUnmount and then diMount. These calls change the imap pointer. Between these two calls there may be calls of function jfs_lookup(). The jfs_lookup() function calls jfs_iget(), which, in turn calls diRead(). The latter references the imap pointer. That may cause diRead() to refer to a pointer freed in diUnmount(). This commit makes the calls to diUnmount()/diMount() atomic so that nothing will read the imap pointer until the whole remount is completed. Signed-off-by: Oleg Kanatov Signed-off-by: Dave Kleikamp --- fs/jfs/jfs_imap.c | 2 +- fs/jfs/jfs_mount.c | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) (limited to 'fs/jfs') diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c index 799d3837e7c2..390cbfce391f 100644 --- a/fs/jfs/jfs_imap.c +++ b/fs/jfs/jfs_imap.c @@ -310,8 +310,8 @@ int diRead(struct inode *ip) iagno = INOTOIAG(ip->i_ino); /* read the iag */ - imap = JFS_IP(ipimap)->i_imap; IREAD_LOCK(ipimap, RDWRLOCK_IMAP); + imap = JFS_IP(ipimap)->i_imap; rc = diIAGRead(imap, iagno, &mp); IREAD_UNLOCK(ipimap); if (rc) { diff --git a/fs/jfs/jfs_mount.c b/fs/jfs/jfs_mount.c index 48d1f70f786c..b83aae56a1f2 100644 --- a/fs/jfs/jfs_mount.c +++ b/fs/jfs/jfs_mount.c @@ -234,11 +234,15 @@ int jfs_mount_rw(struct super_block *sb, int remount) truncate_inode_pages(sbi->ipimap->i_mapping, 0); truncate_inode_pages(sbi->ipbmap->i_mapping, 0); + + IWRITE_LOCK(sbi->ipimap, RDWRLOCK_IMAP); diUnmount(sbi->ipimap, 1); if ((rc = diMount(sbi->ipimap))) { + IWRITE_UNLOCK(sbi->ipimap); jfs_err("jfs_mount_rw: diMount failed!"); return rc; } + IWRITE_UNLOCK(sbi->ipimap); dbUnmount(sbi->ipbmap, 1); if ((rc = dbMount(sbi->ipbmap))) { -- cgit v1.2.1