| From 3bef198f1b17d1bb89260bad947ef084c0a2d1a6 Mon Sep 17 00:00:00 2001 |
| From: Randy Dunlap <rdunlap@infradead.org> |
| Date: Fri, 18 Dec 2020 12:17:16 -0800 |
| Subject: JFS: more checks for invalid superblock |
| |
| From: Randy Dunlap <rdunlap@infradead.org> |
| |
| commit 3bef198f1b17d1bb89260bad947ef084c0a2d1a6 upstream. |
| |
| syzbot is feeding invalid superblock data to JFS for mount testing. |
| JFS does not check several of the fields -- just assumes that they |
| are good since the JFS_MAGIC and version fields are good. |
| |
| In this case (syzbot reproducer), we have s_l2bsize == 0xda0c, |
| pad == 0xf045, and s_state == 0x50, all of which are invalid IMO. |
| Having s_l2bsize == 0xda0c causes this UBSAN warning: |
| UBSAN: shift-out-of-bounds in fs/jfs/jfs_mount.c:373:25 |
| shift exponent -9716 is negative |
| |
| s_l2bsize can be tested for correctness. pad can be tested for non-0 |
| and punted. s_state can be tested for its valid values and punted. |
| |
| Do those 3 tests and if any of them fails, report the superblock as |
| invalid/corrupt and let fsck handle it. |
| |
| With this patch, chkSuper() says this when JFS_DEBUG is enabled: |
| jfs_mount: Mount Failure: superblock is corrupt! |
| Mount JFS Failure: -22 |
| jfs_mount failed w/return code = -22 |
| |
| The obvious problem with this method is that next week there could |
| be another syzbot test that uses different fields for invalid values, |
| this making this like a game of whack-a-mole. |
| |
| syzkaller link: https://syzkaller.appspot.com/bug?extid=36315852ece4132ec193 |
| |
| Reported-by: syzbot+36315852ece4132ec193@syzkaller.appspotmail.com |
| Reported-by: kernel test robot <lkp@intel.com> # v2 |
| Signed-off-by: Randy Dunlap <rdunlap@infradead.org> |
| Signed-off-by: Dave Kleikamp <dave.kleikamp@oracle.com> |
| Cc: jfs-discussion@lists.sourceforge.net |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| fs/jfs/jfs_filsys.h | 1 + |
| fs/jfs/jfs_mount.c | 10 ++++++++++ |
| 2 files changed, 11 insertions(+) |
| |
| --- a/fs/jfs/jfs_filsys.h |
| +++ b/fs/jfs/jfs_filsys.h |
| @@ -268,5 +268,6 @@ |
| * fsck() must be run to repair |
| */ |
| #define FM_EXTENDFS 0x00000008 /* file system extendfs() in progress */ |
| +#define FM_STATE_MAX 0x0000000f /* max value of s_state */ |
| |
| #endif /* _H_JFS_FILSYS */ |
| --- a/fs/jfs/jfs_mount.c |
| +++ b/fs/jfs/jfs_mount.c |
| @@ -37,6 +37,7 @@ |
| #include <linux/fs.h> |
| #include <linux/buffer_head.h> |
| #include <linux/blkdev.h> |
| +#include <linux/log2.h> |
| |
| #include "jfs_incore.h" |
| #include "jfs_filsys.h" |
| @@ -366,6 +367,15 @@ static int chkSuper(struct super_block * |
| sbi->bsize = bsize; |
| sbi->l2bsize = le16_to_cpu(j_sb->s_l2bsize); |
| |
| + /* check some fields for possible corruption */ |
| + if (sbi->l2bsize != ilog2((u32)bsize) || |
| + j_sb->pad != 0 || |
| + le32_to_cpu(j_sb->s_state) > FM_STATE_MAX) { |
| + rc = -EINVAL; |
| + jfs_err("jfs_mount: Mount Failure: superblock is corrupt!"); |
| + goto out; |
| + } |
| + |
| /* |
| * For now, ignore s_pbsize, l2bfactor. All I/O going through buffer |
| * cache. |