* [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; 5+ 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] 5+ 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
0 siblings, 0 replies; 5+ 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] 5+ messages in thread
* Re: [PATCH] JFS: more checks for invalid superblock
@ 2020-12-18 7:23 ` kernel test robot
0 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2020-12-18 7:23 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 4646 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(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 37675 bytes --]
^ permalink raw reply [flat|nested] 5+ 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
-1 siblings, 0 replies; 5+ 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] 5+ messages in thread
end of thread, other threads:[~2020-12-18 20:15 UTC | newest]
Thread overview: 5+ 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 7:23 ` kernel test robot
2020-12-18 20:14 ` Randy Dunlap
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.