* KASAN: use-after-free Read in ext4_xattr_set_entry (2)
@ 2018-11-02 15:39 syzbot
2019-12-13 18:21 ` syzbot
2019-12-15 1:27 ` syzbot
0 siblings, 2 replies; 6+ messages in thread
From: syzbot @ 2018-11-02 15:39 UTC (permalink / raw)
To: adilger.kernel, linux-ext4, linux-kernel, syzkaller-bugs, tytso
Hello,
syzbot found the following crash on:
HEAD commit: fb1c592cf4c9 Merge tag 'char-misc-4.19-rc7' of git://git.k..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=11431406400000
kernel config: https://syzkaller.appspot.com/x/.config?x=c0af03fe452b65fb
dashboard link: https://syzkaller.appspot.com/bug?extid=4a39a025912b265cacef
compiler: gcc (GCC) 8.0.1 20180413 (experimental)
Unfortunately, I don't have any reproducer for this crash yet.
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+4a39a025912b265cacef@syzkaller.appspotmail.com
==================================================================
BUG: KASAN: use-after-free in ext4_xattr_set_entry+0x2f6a/0x3d70
fs/ext4/xattr.c:1600
Read of size 4 at addr ffff8801a4db491b by task syz-executor0/14364
CPU: 1 PID: 14364 Comm: syz-executor0 Not tainted 4.19.0-rc6+ #51
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1c4/0x2b4 lib/dump_stack.c:113
print_address_description.cold.8+0x9/0x1ff mm/kasan/report.c:256
kasan_report_error mm/kasan/report.c:354 [inline]
kasan_report.cold.9+0x242/0x309 mm/kasan/report.c:412
__asan_report_load4_noabort+0x14/0x20 mm/kasan/report.c:432
ext4_xattr_set_entry+0x2f6a/0x3d70 fs/ext4/xattr.c:1600
ext4_xattr_ibody_set+0x8a/0x2c0 fs/ext4/xattr.c:2240
ext4_xattr_set_handle+0xb8f/0x1650 fs/ext4/xattr.c:2394
ext4_initxattrs+0xbd/0x120 fs/ext4/xattr_security.c:43
security_inode_init_security+0x1d1/0x3d0 security/security.c:502
ext4_init_security+0x34/0x40 fs/ext4/xattr_security.c:57
__ext4_new_inode+0x4a6a/0x65b0 fs/ext4/ialloc.c:1160
ext4_symlink+0x4b7/0x1130 fs/ext4/namei.c:3093
vfs_symlink+0x37a/0x5d0 fs/namei.c:4127
do_symlinkat+0x242/0x2d0 fs/namei.c:4154
__do_sys_symlink fs/namei.c:4173 [inline]
__se_sys_symlink fs/namei.c:4171 [inline]
__x64_sys_symlink+0x59/0x80 fs/namei.c:4171
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x4572a7
Code: 64 8b 5d 00 e9 14 fd ff ff 4c 8b 74 24 30 64 c7 45 00 22 00 00 00 bb
22 00 00 00 e9 fd fc ff ff 0f 1f 00 b8 58 00 00 00 0f 05 <48> 3d 01 f0 ff
ff 0f 83 bd b6 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007ffcaac01628 EFLAGS: 00000206 ORIG_RAX: 0000000000000058
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00000000004572a7
RDX: 00007ffcaac016a7 RSI: 00000000004bcfd0 RDI: 00007ffcaac01690
RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000017
R10: 0000000000000075 R11: 0000000000000206 R12: 0000000000000000
R13: 0000000000000001 R14: 0000000000000b01 R15: 0000000000000000
The buggy address belongs to the page:
page:ffffea0006936d00 count:0 mapcount:0 mapping:0000000000000000 index:0x1
flags: 0x2fffc0000000000()
raw: 02fffc0000000000 dead000000000100 dead000000000200 0000000000000000
raw: 0000000000000001 0000000000000000 00000000ffffffff 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff8801a4db4800: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ffff8801a4db4880: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> ffff8801a4db4900: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
^
ffff8801a4db4980: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ffff8801a4db4a00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
==================================================================
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
syzbot.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: KASAN: use-after-free Read in ext4_xattr_set_entry (2) 2018-11-02 15:39 KASAN: use-after-free Read in ext4_xattr_set_entry (2) syzbot @ 2019-12-13 18:21 ` syzbot 2019-12-15 1:27 ` syzbot 1 sibling, 0 replies; 6+ messages in thread From: syzbot @ 2019-12-13 18:21 UTC (permalink / raw) To: adilger.kernel, linux-ext4, linux-kernel, syzkaller-bugs, tytso syzbot has found a reproducer for the following crash on: HEAD commit: ae4b064e Merge tag 'afs-fixes-20191211' of git://git.kerne.. git tree: upstream console output: https://syzkaller.appspot.com/x/log.txt?x=137f6499e00000 kernel config: https://syzkaller.appspot.com/x/.config?x=79f79de2a27d3e3d dashboard link: https://syzkaller.appspot.com/bug?extid=4a39a025912b265cacef compiler: gcc (GCC) 9.0.0 20181231 (experimental) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=15ec1332e00000 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=163455dee00000 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+4a39a025912b265cacef@syzkaller.appspotmail.com EXT4-fs error (device sda1): ext4_xattr_set_entry:1583: inode #2339: comm syz-executor117: corrupted xattr entries ================================================================== BUG: KASAN: use-after-free in ext4_xattr_set_entry+0x35de/0x3770 fs/ext4/xattr.c:1580 Read of size 4 at addr ffff888093705283 by task syz-executor117/9665 CPU: 0 PID: 9665 Comm: syz-executor117 Not tainted 5.5.0-rc1-syzkaller #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x197/0x210 lib/dump_stack.c:118 print_address_description.constprop.0.cold+0xd4/0x30b mm/kasan/report.c:374 __kasan_report.cold+0x1b/0x41 mm/kasan/report.c:506 kasan_report+0x12/0x20 mm/kasan/common.c:639 __asan_report_load4_noabort+0x14/0x20 mm/kasan/generic_report.c:134 ext4_xattr_set_entry+0x35de/0x3770 fs/ext4/xattr.c:1580 ext4_xattr_ibody_set+0x80/0x2d0 fs/ext4/xattr.c:2216 ext4_xattr_set_handle+0x933/0x1200 fs/ext4/xattr.c:2372 ext4_initxattrs+0xc0/0x130 fs/ext4/xattr_security.c:43 security_inode_init_security security/security.c:996 [inline] security_inode_init_security+0x2c8/0x3b0 security/security.c:969 ext4_init_security+0x34/0x40 fs/ext4/xattr_security.c:57 __ext4_new_inode+0x4288/0x4f30 fs/ext4/ialloc.c:1155 ext4_mkdir+0x3d5/0xe20 fs/ext4/namei.c:2774 vfs_mkdir+0x42e/0x670 fs/namei.c:3819 do_mkdirat+0x234/0x2a0 fs/namei.c:3842 __do_sys_mkdir fs/namei.c:3858 [inline] __se_sys_mkdir fs/namei.c:3856 [inline] __x64_sys_mkdir+0x5c/0x80 fs/namei.c:3856 do_syscall_64+0xfa/0x790 arch/x86/entry/common.c:294 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x44d177 Code: 1f 40 00 b8 5a 00 00 00 0f 05 48 3d 01 f0 ff ff 0f 83 4d d6 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 b8 53 00 00 00 0f 05 <48> 3d 01 f0 ff ff 0f 83 2d d6 fb ff c3 66 2e 0f 1f 84 00 00 00 00 RSP: 002b:00007f5c9599fb18 EFLAGS: 00000246 ORIG_RAX: 0000000000000053 RAX: ffffffffffffffda RBX: 00000000006e69c8 RCX: 000000000044d177 RDX: 0000000000000003 RSI: 00000000000001ff RDI: 0000000020000000 RBP: 00000000006e69c0 R08: 0000000000000000 R09: 000000000000000a R10: 0000000000000075 R11: 0000000000000246 R12: 00000000006e69cc R13: 00007ffd790837cf R14: 00007f5c959a09c0 R15: 0000000000000000 The buggy address belongs to the page: page:ffffea00024dc140 refcount:0 mapcount:0 mapping:0000000000000000 index:0x1 raw: 00fffe0000000000 ffffea0002183608 ffffea00027ef508 0000000000000000 raw: 0000000000000001 0000000000000000 00000000ffffffff 0000000000000000 page dumped because: kasan: bad access detected Memory state around the buggy address: ffff888093705180: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ffff888093705200: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff > ffff888093705280: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ^ ffff888093705300: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ffff888093705380: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ================================================================== ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: KASAN: use-after-free Read in ext4_xattr_set_entry (2) 2018-11-02 15:39 KASAN: use-after-free Read in ext4_xattr_set_entry (2) syzbot 2019-12-13 18:21 ` syzbot @ 2019-12-15 1:27 ` syzbot 2019-12-15 6:30 ` Theodore Y. Ts'o 1 sibling, 1 reply; 6+ messages in thread From: syzbot @ 2019-12-15 1:27 UTC (permalink / raw) To: a, adilger.kernel, afd, b.a.t.m.a.n, chris, davem, linux-ext4, linux-kernel, mareklindner, netdev, pali.rohar, sre, sw, syzkaller-bugs, tytso syzbot has bisected this bug to: commit 8835cae5f2abd7f7a3143afe357f416aff5517a4 Author: Chris Lapa <chris@lapa.com.au> Date: Wed Jan 11 01:44:47 2017 +0000 power: supply: bq27xxx: adds specific support for bq27520-g4 revision. bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=16661f41e00000 start commit: ae4b064e Merge tag 'afs-fixes-20191211' of git://git.kerne.. git tree: upstream final crash: https://syzkaller.appspot.com/x/report.txt?x=15661f41e00000 console output: https://syzkaller.appspot.com/x/log.txt?x=11661f41e00000 kernel config: https://syzkaller.appspot.com/x/.config?x=79f79de2a27d3e3d dashboard link: https://syzkaller.appspot.com/bug?extid=4a39a025912b265cacef syz repro: https://syzkaller.appspot.com/x/repro.syz?x=15ec1332e00000 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=163455dee00000 Reported-by: syzbot+4a39a025912b265cacef@syzkaller.appspotmail.com Fixes: 8835cae5f2ab ("power: supply: bq27xxx: adds specific support for bq27520-g4 revision.") For information about bisection process see: https://goo.gl/tpsmEJ#bisection ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: KASAN: use-after-free Read in ext4_xattr_set_entry (2) 2019-12-15 1:27 ` syzbot @ 2019-12-15 6:30 ` Theodore Y. Ts'o 2019-12-15 6:52 ` syzbot 2019-12-16 8:09 ` Dmitry Vyukov 0 siblings, 2 replies; 6+ messages in thread From: Theodore Y. Ts'o @ 2019-12-15 6:30 UTC (permalink / raw) To: syzbot Cc: a, adilger.kernel, afd, b.a.t.m.a.n, chris, davem, linux-ext4, linux-kernel, mareklindner, netdev, pali.rohar, sre, sw, syzkaller-bugs On Sat, Dec 14, 2019 at 05:27:00PM -0800, syzbot wrote: > syzbot has bisected this bug to: > > commit 8835cae5f2abd7f7a3143afe357f416aff5517a4 > Author: Chris Lapa <chris@lapa.com.au> > Date: Wed Jan 11 01:44:47 2017 +0000 > > power: supply: bq27xxx: adds specific support for bq27520-g4 revision. This is pretty clearly nonsense. However let's try this fix: #syz test: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git master From 9c962de70a52e0b24fba00ee7b8707964d3d1e37 Mon Sep 17 00:00:00 2001 From: Theodore Ts'o <tytso@mit.edu> Date: Sun, 15 Dec 2019 01:09:03 -0500 Subject: [PATCH] ext4: validate the debug_want_extra_isize mount option at parse time Instead of setting s_want_extra_size and then making sure that it is a valid value afterwards, validate the field before we set it. This avoids races and other problems when remounting the file system. Signed-off-by: Theodore Ts'o <tytso@mit.edu> Reported-by: syzbot+4a39a025912b265cacef@syzkaller.appspotmail.com --- fs/ext4/super.c | 143 +++++++++++++++++++++++------------------------- 1 file changed, 69 insertions(+), 74 deletions(-) diff --git a/fs/ext4/super.c b/fs/ext4/super.c index b205112ca051..46b6d5b150ac 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -1886,6 +1886,13 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token, } sbi->s_commit_interval = HZ * arg; } else if (token == Opt_debug_want_extra_isize) { + if ((arg & 1) || + (arg < 4) || + (arg > (sbi->s_inode_size - EXT4_GOOD_OLD_INODE_SIZE))) { + ext4_msg(sb, KERN_ERR, + "Invalid want_extra_isize %d", arg); + return -1; + } sbi->s_want_extra_isize = arg; } else if (token == Opt_max_batch_time) { sbi->s_max_batch_time = arg; @@ -3540,40 +3547,6 @@ int ext4_calculate_overhead(struct super_block *sb) return 0; } -static void ext4_clamp_want_extra_isize(struct super_block *sb) -{ - struct ext4_sb_info *sbi = EXT4_SB(sb); - struct ext4_super_block *es = sbi->s_es; - unsigned def_extra_isize = sizeof(struct ext4_inode) - - EXT4_GOOD_OLD_INODE_SIZE; - - if (sbi->s_inode_size == EXT4_GOOD_OLD_INODE_SIZE) { - sbi->s_want_extra_isize = 0; - return; - } - if (sbi->s_want_extra_isize < 4) { - sbi->s_want_extra_isize = def_extra_isize; - if (ext4_has_feature_extra_isize(sb)) { - if (sbi->s_want_extra_isize < - le16_to_cpu(es->s_want_extra_isize)) - sbi->s_want_extra_isize = - le16_to_cpu(es->s_want_extra_isize); - if (sbi->s_want_extra_isize < - le16_to_cpu(es->s_min_extra_isize)) - sbi->s_want_extra_isize = - le16_to_cpu(es->s_min_extra_isize); - } - } - /* Check if enough inode space is available */ - if ((sbi->s_want_extra_isize > sbi->s_inode_size) || - (EXT4_GOOD_OLD_INODE_SIZE + sbi->s_want_extra_isize > - sbi->s_inode_size)) { - sbi->s_want_extra_isize = def_extra_isize; - ext4_msg(sb, KERN_INFO, - "required extra inode space not available"); - } -} - static void ext4_set_resv_clusters(struct super_block *sb) { ext4_fsblk_t resv_clusters; @@ -3781,6 +3754,68 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent) */ sbi->s_li_wait_mult = EXT4_DEF_LI_WAIT_MULT; + if (le32_to_cpu(es->s_rev_level) == EXT4_GOOD_OLD_REV) { + sbi->s_inode_size = EXT4_GOOD_OLD_INODE_SIZE; + sbi->s_first_ino = EXT4_GOOD_OLD_FIRST_INO; + } else { + sbi->s_inode_size = le16_to_cpu(es->s_inode_size); + sbi->s_first_ino = le32_to_cpu(es->s_first_ino); + if (sbi->s_first_ino < EXT4_GOOD_OLD_FIRST_INO) { + ext4_msg(sb, KERN_ERR, "invalid first ino: %u", + sbi->s_first_ino); + goto failed_mount; + } + if ((sbi->s_inode_size < EXT4_GOOD_OLD_INODE_SIZE) || + (!is_power_of_2(sbi->s_inode_size)) || + (sbi->s_inode_size > blocksize)) { + ext4_msg(sb, KERN_ERR, + "unsupported inode size: %d", + sbi->s_inode_size); + goto failed_mount; + } + /* + * i_atime_extra is the last extra field available for + * [acm]times in struct ext4_inode. Checking for that + * field should suffice to ensure we have extra space + * for all three. + */ + if (sbi->s_inode_size >= offsetof(struct ext4_inode, i_atime_extra) + + sizeof(((struct ext4_inode *)0)->i_atime_extra)) { + sb->s_time_gran = 1; + sb->s_time_max = EXT4_EXTRA_TIMESTAMP_MAX; + } else { + sb->s_time_gran = NSEC_PER_SEC; + sb->s_time_max = EXT4_NON_EXTRA_TIMESTAMP_MAX; + } + sb->s_time_min = EXT4_TIMESTAMP_MIN; + } + if (sbi->s_inode_size > EXT4_GOOD_OLD_INODE_SIZE) { + sbi->s_want_extra_isize = sizeof(struct ext4_inode) - + EXT4_GOOD_OLD_INODE_SIZE; + if (ext4_has_feature_extra_isize(sb)) { + unsigned v, max = (sbi->s_inode_size - + EXT4_GOOD_OLD_INODE_SIZE); + + v = le16_to_cpu(es->s_want_extra_isize); + if (v > max) { + ext4_msg(sb, KERN_ERR, + "bad s_want_extra_isize: %d", v); + goto failed_mount; + } + if (sbi->s_want_extra_isize < v) + sbi->s_want_extra_isize = v; + + v = le16_to_cpu(es->s_min_extra_isize); + if (v > max) { + ext4_msg(sb, KERN_ERR, + "bad s_min_extra_isize: %d", v); + goto failed_mount; + } + if (sbi->s_want_extra_isize < v) + sbi->s_want_extra_isize = v; + } + } + if (sbi->s_es->s_mount_opts[0]) { char *s_mount_opts = kstrndup(sbi->s_es->s_mount_opts, sizeof(sbi->s_es->s_mount_opts), @@ -4019,42 +4054,6 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent) has_huge_files); sb->s_maxbytes = ext4_max_size(sb->s_blocksize_bits, has_huge_files); - if (le32_to_cpu(es->s_rev_level) == EXT4_GOOD_OLD_REV) { - sbi->s_inode_size = EXT4_GOOD_OLD_INODE_SIZE; - sbi->s_first_ino = EXT4_GOOD_OLD_FIRST_INO; - } else { - sbi->s_inode_size = le16_to_cpu(es->s_inode_size); - sbi->s_first_ino = le32_to_cpu(es->s_first_ino); - if (sbi->s_first_ino < EXT4_GOOD_OLD_FIRST_INO) { - ext4_msg(sb, KERN_ERR, "invalid first ino: %u", - sbi->s_first_ino); - goto failed_mount; - } - if ((sbi->s_inode_size < EXT4_GOOD_OLD_INODE_SIZE) || - (!is_power_of_2(sbi->s_inode_size)) || - (sbi->s_inode_size > blocksize)) { - ext4_msg(sb, KERN_ERR, - "unsupported inode size: %d", - sbi->s_inode_size); - goto failed_mount; - } - /* - * i_atime_extra is the last extra field available for [acm]times in - * struct ext4_inode. Checking for that field should suffice to ensure - * we have extra space for all three. - */ - if (sbi->s_inode_size >= offsetof(struct ext4_inode, i_atime_extra) + - sizeof(((struct ext4_inode *)0)->i_atime_extra)) { - sb->s_time_gran = 1; - sb->s_time_max = EXT4_EXTRA_TIMESTAMP_MAX; - } else { - sb->s_time_gran = NSEC_PER_SEC; - sb->s_time_max = EXT4_NON_EXTRA_TIMESTAMP_MAX; - } - - sb->s_time_min = EXT4_TIMESTAMP_MIN; - } - sbi->s_desc_size = le16_to_cpu(es->s_desc_size); if (ext4_has_feature_64bit(sb)) { if (sbi->s_desc_size < EXT4_MIN_DESC_SIZE_64BIT || @@ -4503,8 +4502,6 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent) } else if (ret) goto failed_mount4a; - ext4_clamp_want_extra_isize(sb); - ext4_set_resv_clusters(sb); err = ext4_setup_system_zone(sb); @@ -5292,8 +5289,6 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data) goto restore_opts; } - ext4_clamp_want_extra_isize(sb); - if ((old_opts.s_mount_opt & EXT4_MOUNT_JOURNAL_CHECKSUM) ^ test_opt(sb, JOURNAL_CHECKSUM)) { ext4_msg(sb, KERN_ERR, "changing journal_checksum " -- 2.24.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: KASAN: use-after-free Read in ext4_xattr_set_entry (2) 2019-12-15 6:30 ` Theodore Y. Ts'o @ 2019-12-15 6:52 ` syzbot 2019-12-16 8:09 ` Dmitry Vyukov 1 sibling, 0 replies; 6+ messages in thread From: syzbot @ 2019-12-15 6:52 UTC (permalink / raw) To: a, adilger.kernel, afd, b.a.t.m.a.n, chris, davem, linux-ext4, linux-kernel, mareklindner, netdev, pali.rohar, sre, sw, syzkaller-bugs, tytso Hello, syzbot has tested the proposed patch and the reproducer did not trigger crash: Reported-and-tested-by: syzbot+4a39a025912b265cacef@syzkaller.appspotmail.com Tested on: commit: dfdeeb41 Merge branch 'tt/misc' into dev git tree: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git master kernel config: https://syzkaller.appspot.com/x/.config?x=be3b077056d26622 dashboard link: https://syzkaller.appspot.com/bug?extid=4a39a025912b265cacef compiler: gcc (GCC) 9.0.0 20181231 (experimental) patch: https://syzkaller.appspot.com/x/patch.diff?x=11b02546e00000 Note: testing is done by a robot and is best-effort only. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: KASAN: use-after-free Read in ext4_xattr_set_entry (2) 2019-12-15 6:30 ` Theodore Y. Ts'o 2019-12-15 6:52 ` syzbot @ 2019-12-16 8:09 ` Dmitry Vyukov 1 sibling, 0 replies; 6+ messages in thread From: Dmitry Vyukov @ 2019-12-16 8:09 UTC (permalink / raw) To: Theodore Y. Ts'o Cc: syzbot, a, Andreas Dilger, afd, b.a.t.m.a.n, chris, David Miller, linux-ext4, LKML, mareklindner, netdev, Pali Rohár, sre, sw, syzkaller-bugs On Sun, Dec 15, 2019 at 7:30 AM Theodore Y. Ts'o <tytso@mit.edu> wrote: > > On Sat, Dec 14, 2019 at 05:27:00PM -0800, syzbot wrote: > > syzbot has bisected this bug to: > > > > commit 8835cae5f2abd7f7a3143afe357f416aff5517a4 > > Author: Chris Lapa <chris@lapa.com.au> > > Date: Wed Jan 11 01:44:47 2017 +0000 > > > > power: supply: bq27xxx: adds specific support for bq27520-g4 revision. > > This is pretty clearly nonsense. Agree. FTR, it seems that bisection was first diverged by this kernel bug: testing commit 8835cae5f2abd7f7a3143afe357f416aff5517a4 with gcc (GCC) 5.5.0 run #0: crashed: WARNING in batadv_mcast_mla_update on top of this non-deterministic kernel build bug kicked in to prevent detection of "culprit does not affect build": culprit signature: 2aca06cd9a4175f124f866fe66467cfa96c0bf2a parent signature: 8a8dd9ca5726f129b6d36eb6e1f3b78cc7c18b31 > However let's try this fix: > > #syz test: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git master > > From 9c962de70a52e0b24fba00ee7b8707964d3d1e37 Mon Sep 17 00:00:00 2001 > From: Theodore Ts'o <tytso@mit.edu> > Date: Sun, 15 Dec 2019 01:09:03 -0500 > Subject: [PATCH] ext4: validate the debug_want_extra_isize mount option at parse time > > Instead of setting s_want_extra_size and then making sure that it is a > valid value afterwards, validate the field before we set it. This > avoids races and other problems when remounting the file system. > > Signed-off-by: Theodore Ts'o <tytso@mit.edu> > Reported-by: syzbot+4a39a025912b265cacef@syzkaller.appspotmail.com > --- > fs/ext4/super.c | 143 +++++++++++++++++++++++------------------------- > 1 file changed, 69 insertions(+), 74 deletions(-) > > diff --git a/fs/ext4/super.c b/fs/ext4/super.c > index b205112ca051..46b6d5b150ac 100644 > --- a/fs/ext4/super.c > +++ b/fs/ext4/super.c > @@ -1886,6 +1886,13 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token, > } > sbi->s_commit_interval = HZ * arg; > } else if (token == Opt_debug_want_extra_isize) { > + if ((arg & 1) || > + (arg < 4) || > + (arg > (sbi->s_inode_size - EXT4_GOOD_OLD_INODE_SIZE))) { > + ext4_msg(sb, KERN_ERR, > + "Invalid want_extra_isize %d", arg); > + return -1; > + } > sbi->s_want_extra_isize = arg; > } else if (token == Opt_max_batch_time) { > sbi->s_max_batch_time = arg; > @@ -3540,40 +3547,6 @@ int ext4_calculate_overhead(struct super_block *sb) > return 0; > } > > -static void ext4_clamp_want_extra_isize(struct super_block *sb) > -{ > - struct ext4_sb_info *sbi = EXT4_SB(sb); > - struct ext4_super_block *es = sbi->s_es; > - unsigned def_extra_isize = sizeof(struct ext4_inode) - > - EXT4_GOOD_OLD_INODE_SIZE; > - > - if (sbi->s_inode_size == EXT4_GOOD_OLD_INODE_SIZE) { > - sbi->s_want_extra_isize = 0; > - return; > - } > - if (sbi->s_want_extra_isize < 4) { > - sbi->s_want_extra_isize = def_extra_isize; > - if (ext4_has_feature_extra_isize(sb)) { > - if (sbi->s_want_extra_isize < > - le16_to_cpu(es->s_want_extra_isize)) > - sbi->s_want_extra_isize = > - le16_to_cpu(es->s_want_extra_isize); > - if (sbi->s_want_extra_isize < > - le16_to_cpu(es->s_min_extra_isize)) > - sbi->s_want_extra_isize = > - le16_to_cpu(es->s_min_extra_isize); > - } > - } > - /* Check if enough inode space is available */ > - if ((sbi->s_want_extra_isize > sbi->s_inode_size) || > - (EXT4_GOOD_OLD_INODE_SIZE + sbi->s_want_extra_isize > > - sbi->s_inode_size)) { > - sbi->s_want_extra_isize = def_extra_isize; > - ext4_msg(sb, KERN_INFO, > - "required extra inode space not available"); > - } > -} > - > static void ext4_set_resv_clusters(struct super_block *sb) > { > ext4_fsblk_t resv_clusters; > @@ -3781,6 +3754,68 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent) > */ > sbi->s_li_wait_mult = EXT4_DEF_LI_WAIT_MULT; > > + if (le32_to_cpu(es->s_rev_level) == EXT4_GOOD_OLD_REV) { > + sbi->s_inode_size = EXT4_GOOD_OLD_INODE_SIZE; > + sbi->s_first_ino = EXT4_GOOD_OLD_FIRST_INO; > + } else { > + sbi->s_inode_size = le16_to_cpu(es->s_inode_size); > + sbi->s_first_ino = le32_to_cpu(es->s_first_ino); > + if (sbi->s_first_ino < EXT4_GOOD_OLD_FIRST_INO) { > + ext4_msg(sb, KERN_ERR, "invalid first ino: %u", > + sbi->s_first_ino); > + goto failed_mount; > + } > + if ((sbi->s_inode_size < EXT4_GOOD_OLD_INODE_SIZE) || > + (!is_power_of_2(sbi->s_inode_size)) || > + (sbi->s_inode_size > blocksize)) { > + ext4_msg(sb, KERN_ERR, > + "unsupported inode size: %d", > + sbi->s_inode_size); > + goto failed_mount; > + } > + /* > + * i_atime_extra is the last extra field available for > + * [acm]times in struct ext4_inode. Checking for that > + * field should suffice to ensure we have extra space > + * for all three. > + */ > + if (sbi->s_inode_size >= offsetof(struct ext4_inode, i_atime_extra) + > + sizeof(((struct ext4_inode *)0)->i_atime_extra)) { > + sb->s_time_gran = 1; > + sb->s_time_max = EXT4_EXTRA_TIMESTAMP_MAX; > + } else { > + sb->s_time_gran = NSEC_PER_SEC; > + sb->s_time_max = EXT4_NON_EXTRA_TIMESTAMP_MAX; > + } > + sb->s_time_min = EXT4_TIMESTAMP_MIN; > + } > + if (sbi->s_inode_size > EXT4_GOOD_OLD_INODE_SIZE) { > + sbi->s_want_extra_isize = sizeof(struct ext4_inode) - > + EXT4_GOOD_OLD_INODE_SIZE; > + if (ext4_has_feature_extra_isize(sb)) { > + unsigned v, max = (sbi->s_inode_size - > + EXT4_GOOD_OLD_INODE_SIZE); > + > + v = le16_to_cpu(es->s_want_extra_isize); > + if (v > max) { > + ext4_msg(sb, KERN_ERR, > + "bad s_want_extra_isize: %d", v); > + goto failed_mount; > + } > + if (sbi->s_want_extra_isize < v) > + sbi->s_want_extra_isize = v; > + > + v = le16_to_cpu(es->s_min_extra_isize); > + if (v > max) { > + ext4_msg(sb, KERN_ERR, > + "bad s_min_extra_isize: %d", v); > + goto failed_mount; > + } > + if (sbi->s_want_extra_isize < v) > + sbi->s_want_extra_isize = v; > + } > + } > + > if (sbi->s_es->s_mount_opts[0]) { > char *s_mount_opts = kstrndup(sbi->s_es->s_mount_opts, > sizeof(sbi->s_es->s_mount_opts), > @@ -4019,42 +4054,6 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent) > has_huge_files); > sb->s_maxbytes = ext4_max_size(sb->s_blocksize_bits, has_huge_files); > > - if (le32_to_cpu(es->s_rev_level) == EXT4_GOOD_OLD_REV) { > - sbi->s_inode_size = EXT4_GOOD_OLD_INODE_SIZE; > - sbi->s_first_ino = EXT4_GOOD_OLD_FIRST_INO; > - } else { > - sbi->s_inode_size = le16_to_cpu(es->s_inode_size); > - sbi->s_first_ino = le32_to_cpu(es->s_first_ino); > - if (sbi->s_first_ino < EXT4_GOOD_OLD_FIRST_INO) { > - ext4_msg(sb, KERN_ERR, "invalid first ino: %u", > - sbi->s_first_ino); > - goto failed_mount; > - } > - if ((sbi->s_inode_size < EXT4_GOOD_OLD_INODE_SIZE) || > - (!is_power_of_2(sbi->s_inode_size)) || > - (sbi->s_inode_size > blocksize)) { > - ext4_msg(sb, KERN_ERR, > - "unsupported inode size: %d", > - sbi->s_inode_size); > - goto failed_mount; > - } > - /* > - * i_atime_extra is the last extra field available for [acm]times in > - * struct ext4_inode. Checking for that field should suffice to ensure > - * we have extra space for all three. > - */ > - if (sbi->s_inode_size >= offsetof(struct ext4_inode, i_atime_extra) + > - sizeof(((struct ext4_inode *)0)->i_atime_extra)) { > - sb->s_time_gran = 1; > - sb->s_time_max = EXT4_EXTRA_TIMESTAMP_MAX; > - } else { > - sb->s_time_gran = NSEC_PER_SEC; > - sb->s_time_max = EXT4_NON_EXTRA_TIMESTAMP_MAX; > - } > - > - sb->s_time_min = EXT4_TIMESTAMP_MIN; > - } > - > sbi->s_desc_size = le16_to_cpu(es->s_desc_size); > if (ext4_has_feature_64bit(sb)) { > if (sbi->s_desc_size < EXT4_MIN_DESC_SIZE_64BIT || > @@ -4503,8 +4502,6 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent) > } else if (ret) > goto failed_mount4a; > > - ext4_clamp_want_extra_isize(sb); > - > ext4_set_resv_clusters(sb); > > err = ext4_setup_system_zone(sb); > @@ -5292,8 +5289,6 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data) > goto restore_opts; > } > > - ext4_clamp_want_extra_isize(sb); > - > if ((old_opts.s_mount_opt & EXT4_MOUNT_JOURNAL_CHECKSUM) ^ > test_opt(sb, JOURNAL_CHECKSUM)) { > ext4_msg(sb, KERN_ERR, "changing journal_checksum " > -- > 2.24.1 > > -- > You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group. > To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/20191215063020.GA11512%40mit.edu. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-12-16 8:10 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-11-02 15:39 KASAN: use-after-free Read in ext4_xattr_set_entry (2) syzbot 2019-12-13 18:21 ` syzbot 2019-12-15 1:27 ` syzbot 2019-12-15 6:30 ` Theodore Y. Ts'o 2019-12-15 6:52 ` syzbot 2019-12-16 8:09 ` Dmitry Vyukov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).