lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 18 Dec 2020 14:31:50 -0600 From: Dave Kleikamp <dave.kleikamp@...cle.com> To: Randy Dunlap <rdunlap@...radead.org>, linux-kernel@...r.kernel.org Cc: syzbot+36315852ece4132ec193@...kaller.appspotmail.com, kernel test robot <lkp@...el.com>, Dave Kleikamp <shaggy@...nel.org>, jfs-discussion@...ts.sourceforge.net Subject: Re: [PATCH v2] JFS: more checks for invalid superblock Thanks! This looks good and reasonable. I'll try to get it pushed out to -next in the next few days. Shaggy On 12/18/20 2:17 PM, Randy Dunlap wrote: > 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@...kaller.appspotmail.com > Reported-by: kernel test robot <lkp@...el.com> # v2 > Signed-off-by: Randy Dunlap <rdunlap@...radead.org> > Cc: Dave Kleikamp <shaggy@...nel.org> > Cc: jfs-discussion@...ts.sourceforge.net > --- > v2: fix sparse __le32 warning (lkp robot) > > fs/jfs/jfs_filsys.h | 1 + > fs/jfs/jfs_mount.c | 10 ++++++++++ > 2 files changed, 11 insertions(+) > > --- lnx-510.orig/fs/jfs/jfs_mount.c > +++ lnx-510/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. > --- lnx-510.orig/fs/jfs/jfs_filsys.h > +++ lnx-510/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 */ >
Powered by blists - more mailing lists