All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] JFS: more checks for invalid superblock
@ 2020-12-18  5:19 Randy Dunlap
  2020-12-18  7:23 ` kernel test robot
  0 siblings, 1 reply; 3+ messages in thread
From: Randy Dunlap @ 2020-12-18  5:19 UTC (permalink / raw)
  To: linux-kernel
  Cc: Randy Dunlap, syzbot+36315852ece4132ec193, Dave Kleikamp, jfs-discussion

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
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Cc: Dave Kleikamp <shaggy@kernel.org>
Cc: jfs-discussion@lists.sourceforge.net
---
 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 ||
+	    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 */

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] JFS: more checks for invalid superblock
  2020-12-18  5:19 [PATCH] JFS: more checks for invalid superblock Randy Dunlap
@ 2020-12-18  7:23 ` kernel test robot
  2020-12-18 20:14   ` Randy Dunlap
  0 siblings, 1 reply; 3+ messages in thread
From: kernel test robot @ 2020-12-18  7:23 UTC (permalink / raw)
  To: Randy Dunlap, linux-kernel
  Cc: kbuild-all, Randy Dunlap, syzbot+36315852ece4132ec193,
	Dave Kleikamp, jfs-discussion

[-- Attachment #1: Type: text/plain, Size: 4530 bytes --]

Hi Randy,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on shaggy/jfs-next]
[also build test WARNING on linux/master linus/master v5.10 next-20201217]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Randy-Dunlap/JFS-more-checks-for-invalid-superblock/20201218-132143
base:   https://github.com/kleikamp/linux-shaggy jfs-next
config: i386-randconfig-s002-20201217 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.3-184-g1b896707-dirty
        # https://github.com/0day-ci/linux/commit/11cb0575aca69504da8b7984fc7f3e439b1a2331
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Randy-Dunlap/JFS-more-checks-for-invalid-superblock/20201218-132143
        git checkout 11cb0575aca69504da8b7984fc7f3e439b1a2331
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


"sparse warnings: (new ones prefixed by >>)"
>> fs/jfs/jfs_mount.c:373:17: sparse: sparse: restricted __le32 degrades to integer

vim +373 fs/jfs/jfs_mount.c

   324	
   325		jfs_info("superblock: flag:0x%08x state:0x%08x size:0x%Lx",
   326			 le32_to_cpu(j_sb->s_flag), le32_to_cpu(j_sb->s_state),
   327			 (unsigned long long) le64_to_cpu(j_sb->s_size));
   328	
   329		/* validate the descriptors for Secondary AIM and AIT */
   330		if ((j_sb->s_flag & cpu_to_le32(JFS_BAD_SAIT)) !=
   331		    cpu_to_le32(JFS_BAD_SAIT)) {
   332			expected_AIM_bytesize = 2 * PSIZE;
   333			AIM_bytesize = lengthPXD(&(j_sb->s_aim2)) * bsize;
   334			expected_AIT_bytesize = 4 * PSIZE;
   335			AIT_bytesize = lengthPXD(&(j_sb->s_ait2)) * bsize;
   336			AIM_byte_addr = addressPXD(&(j_sb->s_aim2)) * bsize;
   337			AIT_byte_addr = addressPXD(&(j_sb->s_ait2)) * bsize;
   338			byte_addr_diff0 = AIT_byte_addr - AIM_byte_addr;
   339			fsckwsp_addr = addressPXD(&(j_sb->s_fsckpxd)) * bsize;
   340			byte_addr_diff1 = fsckwsp_addr - AIT_byte_addr;
   341			if ((AIM_bytesize != expected_AIM_bytesize) ||
   342			    (AIT_bytesize != expected_AIT_bytesize) ||
   343			    (byte_addr_diff0 != AIM_bytesize) ||
   344			    (byte_addr_diff1 <= AIT_bytesize))
   345				j_sb->s_flag |= cpu_to_le32(JFS_BAD_SAIT);
   346		}
   347	
   348		if ((j_sb->s_flag & cpu_to_le32(JFS_GROUPCOMMIT)) !=
   349		    cpu_to_le32(JFS_GROUPCOMMIT))
   350			j_sb->s_flag |= cpu_to_le32(JFS_GROUPCOMMIT);
   351	
   352		/* validate fs state */
   353		if (j_sb->s_state != cpu_to_le32(FM_CLEAN) &&
   354		    !sb_rdonly(sb)) {
   355			jfs_err("jfs_mount: Mount Failure: File System Dirty.");
   356			rc = -EINVAL;
   357			goto out;
   358		}
   359	
   360		sbi->state = le32_to_cpu(j_sb->s_state);
   361		sbi->mntflag = le32_to_cpu(j_sb->s_flag);
   362	
   363		/*
   364		 * JFS always does I/O by 4K pages.  Don't tell the buffer cache
   365		 * that we use anything else (leave s_blocksize alone).
   366		 */
   367		sbi->bsize = bsize;
   368		sbi->l2bsize = le16_to_cpu(j_sb->s_l2bsize);
   369	
   370		/* check some fields for possible corruption */
   371		if (sbi->l2bsize != ilog2((u32)bsize) ||
   372		    j_sb->pad != 0 ||
 > 373		    j_sb->s_state > FM_STATE_MAX) {
   374			rc = -EINVAL;
   375			jfs_err("jfs_mount: Mount Failure: superblock is corrupt!");
   376			goto out;
   377		}
   378	
   379		/*
   380		 * For now, ignore s_pbsize, l2bfactor.  All I/O going through buffer
   381		 * cache.
   382		 */
   383		sbi->nbperpage = PSIZE >> sbi->l2bsize;
   384		sbi->l2nbperpage = L2PSIZE - sbi->l2bsize;
   385		sbi->l2niperblk = sbi->l2bsize - L2DISIZE;
   386		if (sbi->mntflag & JFS_INLINELOG)
   387			sbi->logpxd = j_sb->s_logpxd;
   388		else {
   389			sbi->logdev = new_decode_dev(le32_to_cpu(j_sb->s_logdev));
   390			uuid_copy(&sbi->uuid, &j_sb->s_uuid);
   391			uuid_copy(&sbi->loguuid, &j_sb->s_loguuid);
   392		}
   393		sbi->fsckpxd = j_sb->s_fsckpxd;
   394		sbi->ait2 = j_sb->s_ait2;
   395	
   396	      out:
   397		brelse(bh);
   398		return rc;
   399	}
   400	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 37675 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] JFS: more checks for invalid superblock
  2020-12-18  7:23 ` kernel test robot
@ 2020-12-18 20:14   ` Randy Dunlap
  0 siblings, 0 replies; 3+ messages in thread
From: Randy Dunlap @ 2020-12-18 20:14 UTC (permalink / raw)
  To: kernel test robot, linux-kernel
  Cc: kbuild-all, syzbot+36315852ece4132ec193, Dave Kleikamp, jfs-discussion

On 12/17/20 11:23 PM, kernel test robot wrote:
> Hi Randy,
> 
> Thank you for the patch! Perhaps something to improve:
> 
> [auto build test WARNING on shaggy/jfs-next]
> [also build test WARNING on linux/master linus/master v5.10 next-20201217]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:    https://github.com/0day-ci/linux/commits/Randy-Dunlap/JFS-more-checks-for-invalid-superblock/20201218-132143
> base:   https://github.com/kleikamp/linux-shaggy jfs-next
> config: i386-randconfig-s002-20201217 (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
> reproduce:
>         # apt-get install sparse
>         # sparse version: v0.6.3-184-g1b896707-dirty
>         # https://github.com/0day-ci/linux/commit/11cb0575aca69504da8b7984fc7f3e439b1a2331
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Randy-Dunlap/JFS-more-checks-for-invalid-superblock/20201218-132143
>         git checkout 11cb0575aca69504da8b7984fc7f3e439b1a2331
>         # save the attached .config to linux build tree
>         make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=i386 
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> 
> "sparse warnings: (new ones prefixed by >>)"
>>> fs/jfs/jfs_mount.c:373:17: sparse: sparse: restricted __le32 degrades to integer
> 

Thank you. I have fixed that.

-- 
~Randy


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-12-18 20:15 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-18  5:19 [PATCH] JFS: more checks for invalid superblock Randy Dunlap
2020-12-18  7:23 ` kernel test robot
2020-12-18 20:14   ` Randy Dunlap

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.