@@ -303,12 +303,6 @@ static int chkSuper(struct super_block *sb)
/*
* validate superblock
*/
- /* validate fs signature */
- if (strncmp(j_sb->s_magic, JFS_MAGIC, 4) ||
- le32_to_cpu(j_sb->s_version) > JFS_VERSION) {
- rc = -EINVAL;
- goto out;
- }
bsize = le32_to_cpu(j_sb->s_bsize);
if (bsize != PSIZE) {
@@ -449,6 +443,18 @@ int updateSuper(struct super_block *sb, uint state)
return 0;
}
+static int validateSuper(struct buffer_head *bh)
+{
+ struct jfs_superblock *j_sb;
+
+ if (!bh)
+ return -EIO;
+
+ j_sb = (struct jfs_superblock *)bh->b_data;
+
+ return (strncmp(j_sb->s_magic, JFS_MAGIC, 4) == 0 &&
+ le32_to_cpu(j_sb->s_version) <= JFS_VERSION) ? 0 : -EINVAL;
+}
/*
* readSuper()
@@ -457,17 +463,17 @@ int updateSuper(struct super_block *sb, uint state)
*/
int readSuper(struct super_block *sb, struct buffer_head **bpp)
{
- /* read in primary superblock */
+ /* read in and validate primary superblock */
*bpp = sb_bread(sb, SUPER1_OFF >> sb->s_blocksize_bits);
- if (*bpp)
+ if (!validateSuper(*bpp))
return 0;
- /* read in secondary/replicated superblock */
+ /* read in and validate secondary/replicated superblock */
*bpp = sb_bread(sb, SUPER2_OFF >> sb->s_blocksize_bits);
- if (*bpp)
+ if (!validateSuper(*bpp))
return 0;
- return -EIO;
+ return -EINVAL;
}
@@ -104,14 +104,15 @@ int jfs_umount(struct super_block *sb)
* list (to signify skip logredo()).
*/
if (log) { /* log = NULL if read-only mount */
- updateSuper(sb, FM_CLEAN);
-
- /*
- * close log:
- *
- * remove file system from log active file system list.
- */
- rc = lmLogClose(sb);
+ rc = updateSuper(sb, FM_CLEAN);
+ if (!rc) {
+ /*
+ * close log:
+ *
+ * remove file system from log active file system list.
+ */
+ rc = lmLogClose(sb);
+ }
}
jfs_info("UnMount JFS Complete: rc = %d", rc);
return rc;
@@ -122,6 +123,7 @@ int jfs_umount_rw(struct super_block *sb)
{
struct jfs_sb_info *sbi = JFS_SBI(sb);
struct jfs_log *log = sbi->log;
+ int rc;
if (!log)
return 0;
@@ -147,7 +149,7 @@ int jfs_umount_rw(struct super_block *sb)
*/
filemap_write_and_wait(sbi->direct_inode->i_mapping);
- updateSuper(sb, FM_CLEAN);
+ rc = updateSuper(sb, FM_CLEAN);
- return lmLogClose(sb);
+ return rc ? rc : lmLogClose(sb);
}
Syzbot has reported the following crash: ERROR: (device loop0): txBegin: read-only filesystem ERROR: (device loop0): remounting filesystem as read-only ERROR: (device loop0): dbFindCtl: Corrupt dmapctl page jfs_create: dtInsert returned -EIO ERROR: (device (efault)): txAbort: Oops: general protection fault, probably for non-canonical address <...> KASAN: null-ptr-deref in range [0x0000000000000050-0x0000000000000057] ... Call Trace: <TASK> ? die_addr.cold+0x8/0xd ? exc_general_protection+0x147/0x240 ? asm_exc_general_protection+0x26/0x30 ? vprintk+0x86/0xa0 ? jfs_error+0x10a/0x2a0 ? dtInsert+0x72f/0xb10 ? __pfx_jfs_error+0x10/0x10 ? vprintk+0x86/0xa0 ? _printk+0xd0/0x108 ? __pfx__printk+0x10/0x10 ? txAbort+0x332/0x540 txAbort+0x382/0x540 jfs_create+0x48e/0xb00 ? __pfx_jfs_create+0x10/0x10 ? generic_permission+0x230/0x690 ? bpf_lsm_inode_permission+0x9/0x10 ? security_inode_permission+0xbf/0x250 ? inode_permission+0xba/0x5d0 vfs_create+0x4e6/0x7a0 do_mknodat+0x2f7/0x5d0 ? __pfx_do_mknodat+0x10/0x10 ? getname_flags.part.0+0x1c5/0x550 __x64_sys_mknod+0x118/0x170 do_syscall_64+0xc7/0x250 entry_SYSCALL_64_after_hwframe+0x77/0x7f ... </TASK> This happens during the mid-air condition when superblock is artificially damaged before remount issued after superblock touch via 'fspick()' and 'fsconfig()'. So add basic superblock validation in 'readSuper()' and do not go further if 'updateSuper()' returns non-zero error code. Reported-by: syzbot+5f0d7af0e45fae10edd1@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=5f0d7af0e45fae10edd1 Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru> --- fs/jfs/jfs_mount.c | 28 +++++++++++++++++----------- fs/jfs/jfs_umount.c | 22 ++++++++++++---------- 2 files changed, 29 insertions(+), 21 deletions(-)