[lvc-project] [PATCH] jfs: add extra superblock validation

[Dmitry Antipov]

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 at syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=5f0d7af0e45fae10edd1
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Dmitry Antipov <dmantipov at yandex.ru>
---
 fs/jfs/jfs_mount.c  | 28 +++++++++++++++++-----------
 fs/jfs/jfs_umount.c | 22 ++++++++++++----------
 2 files changed, 29 insertions(+), 21 deletions(-)

diff --git a/fs/jfs/jfs_mount.c b/fs/jfs/jfs_mount.c
index 98f9a432c336..c0a51defc5b7 100644
--- a/fs/jfs/jfs_mount.c
+++ b/fs/jfs/jfs_mount.c
@@ -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;
 }
 
 
diff --git a/fs/jfs/jfs_umount.c b/fs/jfs/jfs_umount.c
index 8ec43f53f686..5f01f767bc0a 100644
--- a/fs/jfs/jfs_umount.c
+++ b/fs/jfs/jfs_umount.c
@@ -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);
 }
-- 
2.47.0