* [syzbot] UBSAN: shift-out-of-bounds in ntfs_fill_super
@ 2022-04-21 8:16 syzbot
2022-04-21 8:37 ` syzbot
` (2 more replies)
0 siblings, 3 replies; 17+ messages in thread
From: syzbot @ 2022-04-21 8:16 UTC (permalink / raw)
To: almaz.alexandrovich, linux-kernel, llvm, nathan, ndesaulniers,
ntfs3, syzkaller-bugs, trix
Hello,
syzbot found the following issue on:
HEAD commit: b253435746d9 Merge tag 'xtensa-20220416' of https://github..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=144dd7f4f00000
kernel config: https://syzkaller.appspot.com/x/.config?x=ff9f8140cbb3af7
dashboard link: https://syzkaller.appspot.com/bug?extid=1631f09646bc214d2e76
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
Unfortunately, I don't have any reproducer for this issue yet.
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+1631f09646bc214d2e76@syzkaller.appspotmail.com
loop3: detected capacity change from 0 to 8189
================================================================================
UBSAN: shift-out-of-bounds in fs/ntfs3/super.c:673:16
shift exponent -192 is negative
CPU: 0 PID: 12945 Comm: syz-executor.3 Not tainted 5.18.0-rc3-syzkaller-00016-gb253435746d9 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106
ubsan_epilogue+0xb/0x50 lib/ubsan.c:151
__ubsan_handle_shift_out_of_bounds.cold+0xb1/0x187 lib/ubsan.c:322
ntfs_fill_super.cold+0x2bf/0x549 fs/ntfs/bitmap.c:84
get_tree_bdev+0x440/0x760 fs/super.c:1292
vfs_get_tree+0x89/0x2f0 fs/super.c:1497
do_new_mount fs/namespace.c:3040 [inline]
path_mount+0x1320/0x1fa0 fs/namespace.c:3370
do_mount fs/namespace.c:3383 [inline]
__do_sys_mount fs/namespace.c:3591 [inline]
__se_sys_mount fs/namespace.c:3568 [inline]
__x64_sys_mount+0x27f/0x300 fs/namespace.c:3568
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x44/0xae
RIP: 0033:0x7f768c28a61a
Code: 48 c7 c2 b8 ff ff ff f7 d8 64 89 02 b8 ff ff ff ff eb d2 e8 b8 04 00 00 0f 1f 84 00 00 00 00 00 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007f768d3dff88 EFLAGS: 00000206 ORIG_RAX: 00000000000000a5
RAX: ffffffffffffffda RBX: 0000000020000200 RCX: 00007f768c28a61a
RDX: 0000000020000000 RSI: 0000000020000100 RDI: 00007f768d3dffe0
RBP: 00007f768d3e0020 R08: 00007f768d3e0020 R09: 0000000020000000
R10: 0000000000000000 R11: 0000000000000206 R12: 0000000020000000
R13: 0000000020000100 R14: 00007f768d3dffe0 R15: 000000002007aa80
</TASK>
================================================================================
---
This report 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 issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [syzbot] UBSAN: shift-out-of-bounds in ntfs_fill_super 2022-04-21 8:16 [syzbot] UBSAN: shift-out-of-bounds in ntfs_fill_super syzbot @ 2022-04-21 8:37 ` syzbot 2022-04-21 12:29 ` [f2fs-dev] " syzbot 2022-09-20 15:59 ` [PATCH] fs/ntfs3: fix negative shift size in true_sectors_per_clst() Tetsuo Handa 2 siblings, 0 replies; 17+ messages in thread From: syzbot @ 2022-04-21 8:37 UTC (permalink / raw) To: almaz.alexandrovich, linux-kernel, llvm, nathan, ndesaulniers, ntfs3, syzkaller-bugs, trix syzbot has found a reproducer for the following issue on: HEAD commit: b253435746d9 Merge tag 'xtensa-20220416' of https://github.. git tree: upstream console output: https://syzkaller.appspot.com/x/log.txt?x=177e30e8f00000 kernel config: https://syzkaller.appspot.com/x/.config?x=ff9f8140cbb3af7 dashboard link: https://syzkaller.appspot.com/bug?extid=1631f09646bc214d2e76 compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2 syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12e13cfcf00000 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=135e3008f00000 IMPORTANT: if you fix the issue, please add the following tag to the commit: Reported-by: syzbot+1631f09646bc214d2e76@syzkaller.appspotmail.com loop0: detected capacity change from 0 to 8189 ================================================================================ UBSAN: shift-out-of-bounds in fs/ntfs3/super.c:673:16 shift exponent -192 is negative CPU: 1 PID: 3587 Comm: syz-executor611 Not tainted 5.18.0-rc3-syzkaller-00016-gb253435746d9 #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: <TASK> __dump_stack lib/dump_stack.c:88 [inline] dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106 ubsan_epilogue+0xb/0x50 lib/ubsan.c:151 __ubsan_handle_shift_out_of_bounds.cold+0xb1/0x187 lib/ubsan.c:322 ntfs_fill_super.cold+0x2bf/0x549 fs/ntfs/bitmap.c:84 get_tree_bdev+0x440/0x760 fs/super.c:1292 vfs_get_tree+0x89/0x2f0 fs/super.c:1497 do_new_mount fs/namespace.c:3040 [inline] path_mount+0x1320/0x1fa0 fs/namespace.c:3370 do_mount fs/namespace.c:3383 [inline] __do_sys_mount fs/namespace.c:3591 [inline] __se_sys_mount fs/namespace.c:3568 [inline] __x64_sys_mount+0x27f/0x300 fs/namespace.c:3568 do_syscall_x64 arch/x86/entry/common.c:50 [inline] do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80 entry_SYSCALL_64_after_hwframe+0x44/0xae RIP: 0033:0x7fb8d7bcd7ea Code: 83 c4 08 5b 5d c3 66 2e 0f 1f 84 00 00 00 00 00 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 c0 ff ff ff f7 d8 64 89 01 48 RSP: 002b:00007ffdfb6d7948 EFLAGS: 00000286 ORIG_RAX: 00000000000000a5 RAX: ffffffffffffffda RBX: 00007ffdfb6d79a0 RCX: 00007fb8d7bcd7ea RDX: 0000000020000000 RSI: 0000000020000100 RDI: 00007ffdfb6d7960 RBP: 00007ffdfb6d7960 R08: 00007ffdfb6d79a0 R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000000286 R12: 0000000020001b80 R13: 0000000000000003 R14: 0000000000000004 R15: 0000000000000110 </TASK> ================================================================================ ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [syzbot] UBSAN: shift-out-of-bounds in ntfs_fill_super 2022-04-21 8:16 [syzbot] UBSAN: shift-out-of-bounds in ntfs_fill_super syzbot @ 2022-04-21 12:29 ` syzbot 2022-04-21 12:29 ` [f2fs-dev] " syzbot 2022-09-20 15:59 ` [PATCH] fs/ntfs3: fix negative shift size in true_sectors_per_clst() Tetsuo Handa 2 siblings, 0 replies; 17+ messages in thread From: syzbot @ 2022-04-21 12:29 UTC (permalink / raw) To: almaz.alexandrovich, chao, davem, jaegeuk, johan.hedberg, kuba, linux-bluetooth, linux-f2fs-devel, linux-kernel, llvm, luiz.dentz, marcel, nathan, ndesaulniers, netdev, ntfs3, syzkaller-bugs, trix syzbot has bisected this issue to: commit adf9ea89c719c1d23794e363f631e376b3ff8cbc Author: Chao Yu <chao@kernel.org> Date: Thu Aug 26 02:03:15 2021 +0000 f2fs: fix unexpected ENOENT comes from f2fs_map_blocks() bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=101dd0fcf00000 start commit: b253435746d9 Merge tag 'xtensa-20220416' of https://github.. git tree: upstream final oops: https://syzkaller.appspot.com/x/report.txt?x=121dd0fcf00000 console output: https://syzkaller.appspot.com/x/log.txt?x=141dd0fcf00000 kernel config: https://syzkaller.appspot.com/x/.config?x=ff9f8140cbb3af7 dashboard link: https://syzkaller.appspot.com/bug?extid=1631f09646bc214d2e76 syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12e13cfcf00000 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=135e3008f00000 Reported-by: syzbot+1631f09646bc214d2e76@syzkaller.appspotmail.com Fixes: adf9ea89c719 ("f2fs: fix unexpected ENOENT comes from f2fs_map_blocks()") For information about bisection process see: https://goo.gl/tpsmEJ#bisection ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [f2fs-dev] [syzbot] UBSAN: shift-out-of-bounds in ntfs_fill_super @ 2022-04-21 12:29 ` syzbot 0 siblings, 0 replies; 17+ messages in thread From: syzbot @ 2022-04-21 12:29 UTC (permalink / raw) To: almaz.alexandrovich, chao, davem, jaegeuk, johan.hedberg, kuba, linux-bluetooth, linux-f2fs-devel, linux-kernel, llvm, luiz.dentz, marcel, nathan, ndesaulniers, netdev, ntfs3, syzkaller-bugs, trix syzbot has bisected this issue to: commit adf9ea89c719c1d23794e363f631e376b3ff8cbc Author: Chao Yu <chao@kernel.org> Date: Thu Aug 26 02:03:15 2021 +0000 f2fs: fix unexpected ENOENT comes from f2fs_map_blocks() bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=101dd0fcf00000 start commit: b253435746d9 Merge tag 'xtensa-20220416' of https://github.. git tree: upstream final oops: https://syzkaller.appspot.com/x/report.txt?x=121dd0fcf00000 console output: https://syzkaller.appspot.com/x/log.txt?x=141dd0fcf00000 kernel config: https://syzkaller.appspot.com/x/.config?x=ff9f8140cbb3af7 dashboard link: https://syzkaller.appspot.com/bug?extid=1631f09646bc214d2e76 syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12e13cfcf00000 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=135e3008f00000 Reported-by: syzbot+1631f09646bc214d2e76@syzkaller.appspotmail.com Fixes: adf9ea89c719 ("f2fs: fix unexpected ENOENT comes from f2fs_map_blocks()") For information about bisection process see: https://goo.gl/tpsmEJ#bisection _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] fs/ntfs3: fix negative shift size in true_sectors_per_clst() 2022-04-21 8:16 [syzbot] UBSAN: shift-out-of-bounds in ntfs_fill_super syzbot 2022-04-21 8:37 ` syzbot 2022-04-21 12:29 ` [f2fs-dev] " syzbot @ 2022-09-20 15:59 ` Tetsuo Handa 2022-09-23 0:38 ` Randy Dunlap 2022-09-30 16:34 ` Konstantin Komarov 2 siblings, 2 replies; 17+ messages in thread From: Tetsuo Handa @ 2022-09-20 15:59 UTC (permalink / raw) To: Konstantin Komarov, Andrew Morton, Namjae Jeon, Randy Dunlap Cc: syzbot, syzkaller-bugs, ntfs3, LKML syzbot is reporting shift-out-of-bounds in true_sectors_per_clst() [1], for commit a3b774342fa752a5 ("fs/ntfs3: validate BOOT sectors_per_clusters") did not address that (0 - boot->sectors_per_clusters) < 0 because "u8" was chosen for type of boot->sectors_per_clusters because 0x80 needs to be positive in order to support 64K clusters. Use "s8" cast in order to make sure that (0 - (s8) boot->sectors_per_clusters) > 0. Link: https://syzkaller.appspot.com/bug?extid=1631f09646bc214d2e76 [1] Reported-by: syzbot <syzbot+1631f09646bc214d2e76@syzkaller.appspotmail.com> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Tested-by: syzbot <syzbot+1631f09646bc214d2e76@syzkaller.appspotmail.com> Fixes: a3b774342fa752a5 ("fs/ntfs3: validate BOOT sectors_per_clusters") diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c index 47012c9bf505..c7ffd21fb255 100644 --- a/fs/ntfs3/super.c +++ b/fs/ntfs3/super.c @@ -672,7 +672,7 @@ static u32 true_sectors_per_clst(const struct NTFS_BOOT *boot) if (boot->sectors_per_clusters <= 0x80) return boot->sectors_per_clusters; if (boot->sectors_per_clusters >= 0xf4) /* limit shift to 2MB max */ - return 1U << (0 - boot->sectors_per_clusters); + return 1U << (0 - (s8) boot->sectors_per_clusters); return -EINVAL; } -- 2.18.4 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] fs/ntfs3: fix negative shift size in true_sectors_per_clst() 2022-09-20 15:59 ` [PATCH] fs/ntfs3: fix negative shift size in true_sectors_per_clst() Tetsuo Handa @ 2022-09-23 0:38 ` Randy Dunlap 2022-09-23 0:47 ` Joe Perches ` (2 more replies) 2022-09-30 16:34 ` Konstantin Komarov 1 sibling, 3 replies; 17+ messages in thread From: Randy Dunlap @ 2022-09-23 0:38 UTC (permalink / raw) To: Tetsuo Handa, Konstantin Komarov, Andrew Morton, Namjae Jeon, Shigeru Yoshida Cc: syzbot, syzkaller-bugs, ntfs3, LKML Hi, On 9/20/22 08:59, Tetsuo Handa wrote: > syzbot is reporting shift-out-of-bounds in true_sectors_per_clst() [1], for > commit a3b774342fa752a5 ("fs/ntfs3: validate BOOT sectors_per_clusters") > did not address that (0 - boot->sectors_per_clusters) < 0 because "u8" was > chosen for type of boot->sectors_per_clusters because 0x80 needs to be > positive in order to support 64K clusters. Use "s8" cast in order to make > sure that (0 - (s8) boot->sectors_per_clusters) > 0. > > Link: https://syzkaller.appspot.com/bug?extid=1631f09646bc214d2e76 [1] > Reported-by: syzbot <syzbot+1631f09646bc214d2e76@syzkaller.appspotmail.com> > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > Tested-by: syzbot <syzbot+1631f09646bc214d2e76@syzkaller.appspotmail.com> > Fixes: a3b774342fa752a5 ("fs/ntfs3: validate BOOT sectors_per_clusters") > > diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c > index 47012c9bf505..c7ffd21fb255 100644 > --- a/fs/ntfs3/super.c > +++ b/fs/ntfs3/super.c > @@ -672,7 +672,7 @@ static u32 true_sectors_per_clst(const struct NTFS_BOOT *boot) > if (boot->sectors_per_clusters <= 0x80) > return boot->sectors_per_clusters; > if (boot->sectors_per_clusters >= 0xf4) /* limit shift to 2MB max */ > - return 1U << (0 - boot->sectors_per_clusters); > + return 1U << (0 - (s8) boot->sectors_per_clusters); > return -EINVAL; > } > I slightly prefer the earlier patch: https://lore.kernel.org/all/20220823144625.1627736-1-syoshida@redhat.com/ but it appears that the NTFS3 maintainer is MIA again. :( -- ~Randy ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] fs/ntfs3: fix negative shift size in true_sectors_per_clst() 2022-09-23 0:38 ` Randy Dunlap @ 2022-09-23 0:47 ` Joe Perches 2022-09-23 1:14 ` Randy Dunlap 2022-09-23 9:59 ` Dan Carpenter 2022-09-23 1:25 ` Tetsuo Handa 2022-09-23 11:58 ` Konstantin Komarov 2 siblings, 2 replies; 17+ messages in thread From: Joe Perches @ 2022-09-23 0:47 UTC (permalink / raw) To: Randy Dunlap, Tetsuo Handa, Konstantin Komarov, Andrew Morton, Namjae Jeon, Shigeru Yoshida Cc: syzbot, syzkaller-bugs, ntfs3, LKML On Thu, 2022-09-22 at 17:38 -0700, Randy Dunlap wrote: > it appears that the NTFS3 maintainer is MIA again. :( (I've no affiliation with the NTFS3 maintainer or paragon) Perhaps the expectation of prioritized immediate turnaround is unrealistic. It's free. Be patient. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] fs/ntfs3: fix negative shift size in true_sectors_per_clst() 2022-09-23 0:47 ` Joe Perches @ 2022-09-23 1:14 ` Randy Dunlap 2022-09-23 9:59 ` Dan Carpenter 1 sibling, 0 replies; 17+ messages in thread From: Randy Dunlap @ 2022-09-23 1:14 UTC (permalink / raw) To: Joe Perches, Tetsuo Handa, Konstantin Komarov, Andrew Morton, Namjae Jeon, Shigeru Yoshida Cc: syzbot, syzkaller-bugs, ntfs3, LKML On 9/22/22 17:47, Joe Perches wrote: > On Thu, 2022-09-22 at 17:38 -0700, Randy Dunlap wrote: >> it appears that the NTFS3 maintainer is MIA again. :( > > (I've no affiliation with the NTFS3 maintainer or paragon) > > Perhaps the expectation of prioritized immediate turnaround is unrealistic. > > It's free. Be patient. > Yes Joe, I get that. https://lore.kernel.org/all/da20d32b-5185-f40b-48b8-2986922d8b25@stargateuniverse.net/ -- ~Randy ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] fs/ntfs3: fix negative shift size in true_sectors_per_clst() 2022-09-23 0:47 ` Joe Perches 2022-09-23 1:14 ` Randy Dunlap @ 2022-09-23 9:59 ` Dan Carpenter 1 sibling, 0 replies; 17+ messages in thread From: Dan Carpenter @ 2022-09-23 9:59 UTC (permalink / raw) To: Joe Perches Cc: Randy Dunlap, Tetsuo Handa, Konstantin Komarov, Andrew Morton, Namjae Jeon, Shigeru Yoshida, syzbot, syzkaller-bugs, ntfs3, LKML On Thu, Sep 22, 2022 at 05:47:51PM -0700, Joe Perches wrote: > On Thu, 2022-09-22 at 17:38 -0700, Randy Dunlap wrote: > > it appears that the NTFS3 maintainer is MIA again. :( > > (I've no affiliation with the NTFS3 maintainer or paragon) > > Perhaps the expectation of prioritized immediate turnaround is unrealistic. > > It's free. Be patient. It's not just Randy. There are a bunch of fixes which are waiting to be merged... I sometimes look at syzkaller patches but it's like with ntfs3 why even bother because the maintainer is gone? It's not going in anyway. regards, dan carpenter ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] fs/ntfs3: fix negative shift size in true_sectors_per_clst() 2022-09-23 0:38 ` Randy Dunlap 2022-09-23 0:47 ` Joe Perches @ 2022-09-23 1:25 ` Tetsuo Handa 2022-09-23 11:58 ` Aleksandr Nogikh 2022-09-23 11:58 ` Konstantin Komarov 2 siblings, 1 reply; 17+ messages in thread From: Tetsuo Handa @ 2022-09-23 1:25 UTC (permalink / raw) To: Randy Dunlap, Konstantin Komarov, Andrew Morton, Namjae Jeon, Shigeru Yoshida, Dmitry Vyukov Cc: syzbot, syzkaller-bugs, ntfs3, LKML On 2022/09/23 9:38, Randy Dunlap wrote: > I slightly prefer the earlier patch: > > https://lore.kernel.org/all/20220823144625.1627736-1-syoshida@redhat.com/ > > but it appears that the NTFS3 maintainer is MIA again. :( > Shigeru Yoshida posted a patch against https://syzkaller.appspot.com/bug?extid=35b87c668935bb55e666 and I posted a patch against https://syzkaller.appspot.com/bug?extid=1631f09646bc214d2e76 . We didn't realize that these are the same problem. It seems that sending to not only syzbot+XXXXXXXXXXXXXXXXXXXX@syzkaller.appspotmail.com but also syzkaller-bugs@googlegroups.com is required for proposed patches to be recorded (in order to avoid duplicated works) into a page linked from "Status:" in each bug page. Since https://syzkaller.appspot.com/upstream does not have a column for tracking intermediate status between "Open" and "Fixed" (e.g. cause identified, patch proposed) because it can take long time until patches are accepted into one of git trees syzbot is tracking, we need to utilize "Last activity" in the list page and a page linked from "Status:" in each bug page. Time to boost priority for implementing user-supplied comment column (e.g. "#syz memo:" command) to each bug? ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] fs/ntfs3: fix negative shift size in true_sectors_per_clst() 2022-09-23 1:25 ` Tetsuo Handa @ 2022-09-23 11:58 ` Aleksandr Nogikh 2022-09-23 14:35 ` Tetsuo Handa 0 siblings, 1 reply; 17+ messages in thread From: Aleksandr Nogikh @ 2022-09-23 11:58 UTC (permalink / raw) To: Tetsuo Handa Cc: Randy Dunlap, Konstantin Komarov, Andrew Morton, Namjae Jeon, Shigeru Yoshida, Dmitry Vyukov, syzbot, 'Aleksandr Nogikh' via syzkaller-bugs, ntfs3, LKML Hi Tetsuo, Thank you very much for providing the feedback! On Fri, Sep 23, 2022 at 3:26 AM Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> wrote: > > On 2022/09/23 9:38, Randy Dunlap wrote: > > I slightly prefer the earlier patch: > > > > https://lore.kernel.org/all/20220823144625.1627736-1-syoshida@redhat.com/ > > > > but it appears that the NTFS3 maintainer is MIA again. :( > > > > Shigeru Yoshida posted a patch against https://syzkaller.appspot.com/bug?extid=35b87c668935bb55e666 > and I posted a patch against https://syzkaller.appspot.com/bug?extid=1631f09646bc214d2e76 . > We didn't realize that these are the same problem. > > It seems that sending to not only syzbot+XXXXXXXXXXXXXXXXXXXX@syzkaller.appspotmail.com > but also syzkaller-bugs@googlegroups.com is required for proposed patches to be recorded > (in order to avoid duplicated works) into a page linked from "Status:" in each bug page. We do have plans to start inspecting LKML messages for the patches that mention syzbot-reported bugs. It will be possible then to display them all on the bug page and somehow mark bugs with a PATCH sent on the list. > > Since https://syzkaller.appspot.com/upstream does not have a column for tracking intermediate > status between "Open" and "Fixed" (e.g. cause identified, patch proposed) because it can take > long time until patches are accepted into one of git trees syzbot is tracking, we need to > utilize "Last activity" in the list page and a page linked from "Status:" in each bug page. > Time to boost priority for implementing user-supplied comment column (e.g. "#syz memo:" command) > to each bug? And then syzbot should just display all such received comments on the bug's web page, right? -- Best Regards, Aleksandr > > -- > 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/423b1fa6-10fa-3ff9-52bc-1262643c62d9%40I-love.SAKURA.ne.jp. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] fs/ntfs3: fix negative shift size in true_sectors_per_clst() 2022-09-23 11:58 ` Aleksandr Nogikh @ 2022-09-23 14:35 ` Tetsuo Handa 2022-09-24 11:58 ` Aleksandr Nogikh 0 siblings, 1 reply; 17+ messages in thread From: Tetsuo Handa @ 2022-09-23 14:35 UTC (permalink / raw) To: Aleksandr Nogikh Cc: Randy Dunlap, Konstantin Komarov, Andrew Morton, Namjae Jeon, Shigeru Yoshida, Dmitry Vyukov, syzbot, 'Aleksandr Nogikh' via syzkaller-bugs, ntfs3, LKML On 2022/09/23 20:58, Aleksandr Nogikh wrote: > We do have plans to start inspecting LKML messages for the patches > that mention syzbot-reported bugs. It will be possible then to display > them all on the bug page and somehow mark bugs with a PATCH sent on > the list. I interpret it as an attempt to automatically show "Patch proposed" state. But since not all patches have Reported-by: tag, and/or a proposed patch with Reported-by: tag might be withdrawn via review, that state should be also manually changeable. > And then syzbot should just display all such received comments on the > bug's web page, right? Whether "all comments" or "last comment" needs some decision. It might be a few words indicating culprit subsystem (probably "last" should overwrite), it might be memo describing how far debugging went (probably "all" is helpful), it might be some URL where discussions/patches are (probably "all" is helpful), it might be trying to show or hide "Patch proposed" state (probably "last" should overwrite). By the way, a possible improvement on "Patch testing requests:" table. Although the "Patch" link showing diff output after applying proposed patch is OK, I'd like to also see a link to original "#syz test:" mail, for the intent of diff (which would be in patch description part if it was a formal patch) is dropped from diff output in the "Patch" link. For example, https://syzkaller.appspot.com/bug?extid=9ca7a12fd736d93e0232 was forgotten for 1000 days after 7 patch testing requests. I can't easily find the intent of each diff (e.g. just debug printk() or proper fix). It seems the last one was about to formal submit, but I can't find why it is not yet applied. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] fs/ntfs3: fix negative shift size in true_sectors_per_clst() 2022-09-23 14:35 ` Tetsuo Handa @ 2022-09-24 11:58 ` Aleksandr Nogikh 2022-09-24 12:03 ` Aleksandr Nogikh 0 siblings, 1 reply; 17+ messages in thread From: Aleksandr Nogikh @ 2022-09-24 11:58 UTC (permalink / raw) To: Tetsuo Handa Cc: Randy Dunlap, Konstantin Komarov, Andrew Morton, Namjae Jeon, Shigeru Yoshida, Dmitry Vyukov, syzbot, 'Aleksandr Nogikh' via syzkaller-bugs, ntfs3, LKML On Fri, Sep 23, 2022 at 4:35 PM Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> wrote: > > On 2022/09/23 20:58, Aleksandr Nogikh wrote: > > We do have plans to start inspecting LKML messages for the patches > > that mention syzbot-reported bugs. It will be possible then to display > > them all on the bug page and somehow mark bugs with a PATCH sent on > > the list. > > I interpret it as an attempt to automatically show "Patch proposed" state. > But since not all patches have Reported-by: tag, and/or a proposed patch > with Reported-by: tag might be withdrawn via review, that state should be > also manually changeable. Yes, it is meant to be manually changeable. To be honest, I'm a little bit worried about making the syzbot communication protocol more and more complex - e.g. how will other developers figure out that such a feature exists at all.. Though, there are anyway no other options than to extend the protocol. > > > And then syzbot should just display all such received comments on the > > bug's web page, right? > > Whether "all comments" or "last comment" needs some decision. It might be a few words > indicating culprit subsystem (probably "last" should overwrite), it might be memo > describing how far debugging went (probably "all" is helpful), it might be some > URL where discussions/patches are (probably "all" is helpful), it might be trying to > show or hide "Patch proposed" state (probably "last" should overwrite). > It seems that even displaying all patch sending attempts (regardless of their status) should be already very helpful in preventing the situations like you described earlier. E.g. it's very likely that syzbot won't be promptly notified about withdrawn patches, so it's anyway necessary to look at all previous attempts. > > > By the way, a possible improvement on "Patch testing requests:" table. > Although the "Patch" link showing diff output after applying proposed patch is OK, > I'd like to also see a link to original "#syz test:" mail, for the intent of diff > (which would be in patch description part if it was a formal patch) is dropped from > diff output in the "Patch" link. Interesting! I created an issue to keep track of this: https://github.com/google/syzkaller/issues/3392 The presence of the link will, though, depend on whether the user did Cc some public mailing lists while making the patch testing request. > > For example, https://syzkaller.appspot.com/bug?extid=9ca7a12fd736d93e0232 was forgotten > for 1000 days after 7 patch testing requests. I can't easily find the intent of each diff > (e.g. just debug printk() or proper fix). It seems the last one was about to formal submit, > but I can't find why it is not yet applied. Btw there was recently deployed an old repro retesting feature that retests old reproducers and obsoletes bugs if all of them are no longer working. It has already closed > 150 bugs this way (more to come) and in quite a lot of such closed bugs I see a patch testing request from some developer that was done several months or even several years ago. And syzbot was not notified about these fixes. So yes, the presence of a patch testing request can be a strong indicator that the bug is already fixed and syzbot just doesn't know about that. > > -- > 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/ea7c00c1-07d7-c23e-80f0-0693016e9731%40I-love.SAKURA.ne.jp. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] fs/ntfs3: fix negative shift size in true_sectors_per_clst() 2022-09-24 11:58 ` Aleksandr Nogikh @ 2022-09-24 12:03 ` Aleksandr Nogikh 0 siblings, 0 replies; 17+ messages in thread From: Aleksandr Nogikh @ 2022-09-24 12:03 UTC (permalink / raw) To: Tetsuo Handa Cc: Randy Dunlap, Konstantin Komarov, Andrew Morton, Namjae Jeon, Shigeru Yoshida, Dmitry Vyukov, syzbot, 'Aleksandr Nogikh' via syzkaller-bugs, ntfs3, LKML > > By the way, a possible improvement on "Patch testing requests:" table. > > Although the "Patch" link showing diff output after applying proposed patch is OK, > > I'd like to also see a link to original "#syz test:" mail, for the intent of diff > > (which would be in patch description part if it was a formal patch) is dropped from > > diff output in the "Patch" link. > > Interesting! > I created an issue to keep track of this: > https://github.com/google/syzkaller/issues/3392 > The presence of the link will, though, depend on whether the user did > Cc some public mailing lists while making the patch testing request. Upd from Dmitry: https://github.com/google/syzkaller/issues/3392#issuecomment-1256952263 We actually do provide these links, but they're rarely present for the reason I mentioned above. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] fs/ntfs3: fix negative shift size in true_sectors_per_clst() 2022-09-23 0:38 ` Randy Dunlap 2022-09-23 0:47 ` Joe Perches 2022-09-23 1:25 ` Tetsuo Handa @ 2022-09-23 11:58 ` Konstantin Komarov 2022-09-23 16:07 ` Randy Dunlap 2 siblings, 1 reply; 17+ messages in thread From: Konstantin Komarov @ 2022-09-23 11:58 UTC (permalink / raw) To: Randy Dunlap, Tetsuo Handa, Andrew Morton, Namjae Jeon, Shigeru Yoshida Cc: syzbot, syzkaller-bugs, ntfs3, LKML On 9/23/22 03:38, Randy Dunlap wrote: > Hi, > > On 9/20/22 08:59, Tetsuo Handa wrote: >> syzbot is reporting shift-out-of-bounds in true_sectors_per_clst() [1], for >> commit a3b774342fa752a5 ("fs/ntfs3: validate BOOT sectors_per_clusters") >> did not address that (0 - boot->sectors_per_clusters) < 0 because "u8" was >> chosen for type of boot->sectors_per_clusters because 0x80 needs to be >> positive in order to support 64K clusters. Use "s8" cast in order to make >> sure that (0 - (s8) boot->sectors_per_clusters) > 0. >> >> Link: https://syzkaller.appspot.com/bug?extid=1631f09646bc214d2e76 [1] >> Reported-by: syzbot <syzbot+1631f09646bc214d2e76@syzkaller.appspotmail.com> >> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> >> Tested-by: syzbot <syzbot+1631f09646bc214d2e76@syzkaller.appspotmail.com> >> Fixes: a3b774342fa752a5 ("fs/ntfs3: validate BOOT sectors_per_clusters") >> >> diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c >> index 47012c9bf505..c7ffd21fb255 100644 >> --- a/fs/ntfs3/super.c >> +++ b/fs/ntfs3/super.c >> @@ -672,7 +672,7 @@ static u32 true_sectors_per_clst(const struct NTFS_BOOT *boot) >> if (boot->sectors_per_clusters <= 0x80) >> return boot->sectors_per_clusters; >> if (boot->sectors_per_clusters >= 0xf4) /* limit shift to 2MB max */ >> - return 1U << (0 - boot->sectors_per_clusters); >> + return 1U << (0 - (s8) boot->sectors_per_clusters); >> return -EINVAL; >> } >> > > I slightly prefer the earlier patch: > > https://lore.kernel.org/all/20220823144625.1627736-1-syoshida@redhat.com/ > > but it appears that the NTFS3 maintainer is MIA again. :( > Hello I've sent patches on 12.09, so I'm not MIA. I plan to look at patches next week, there are quite a lot of them. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] fs/ntfs3: fix negative shift size in true_sectors_per_clst() 2022-09-23 11:58 ` Konstantin Komarov @ 2022-09-23 16:07 ` Randy Dunlap 0 siblings, 0 replies; 17+ messages in thread From: Randy Dunlap @ 2022-09-23 16:07 UTC (permalink / raw) To: Konstantin Komarov, Tetsuo Handa, Andrew Morton, Namjae Jeon, Shigeru Yoshida Cc: syzbot, syzkaller-bugs, ntfs3, LKML Hi, On 9/23/22 04:58, Konstantin Komarov wrote: > > > On 9/23/22 03:38, Randy Dunlap wrote: >> Hi, >> >> On 9/20/22 08:59, Tetsuo Handa wrote: >>> syzbot is reporting shift-out-of-bounds in true_sectors_per_clst() [1], for >>> commit a3b774342fa752a5 ("fs/ntfs3: validate BOOT sectors_per_clusters") >>> did not address that (0 - boot->sectors_per_clusters) < 0 because "u8" was >>> chosen for type of boot->sectors_per_clusters because 0x80 needs to be >>> positive in order to support 64K clusters. Use "s8" cast in order to make >>> sure that (0 - (s8) boot->sectors_per_clusters) > 0. >>> >>> Link: https://syzkaller.appspot.com/bug?extid=1631f09646bc214d2e76 [1] >>> Reported-by: syzbot <syzbot+1631f09646bc214d2e76@syzkaller.appspotmail.com> >>> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> >>> Tested-by: syzbot <syzbot+1631f09646bc214d2e76@syzkaller.appspotmail.com> >>> Fixes: a3b774342fa752a5 ("fs/ntfs3: validate BOOT sectors_per_clusters") >>> >>> diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c >>> index 47012c9bf505..c7ffd21fb255 100644 >>> --- a/fs/ntfs3/super.c >>> +++ b/fs/ntfs3/super.c >>> @@ -672,7 +672,7 @@ static u32 true_sectors_per_clst(const struct NTFS_BOOT *boot) >>> if (boot->sectors_per_clusters <= 0x80) >>> return boot->sectors_per_clusters; >>> if (boot->sectors_per_clusters >= 0xf4) /* limit shift to 2MB max */ >>> - return 1U << (0 - boot->sectors_per_clusters); >>> + return 1U << (0 - (s8) boot->sectors_per_clusters); >>> return -EINVAL; >>> } >>> >> >> I slightly prefer the earlier patch: >> >> https://lore.kernel.org/all/20220823144625.1627736-1-syoshida@redhat.com/ >> >> but it appears that the NTFS3 maintainer is MIA again. :( >> > > Hello > > I've sent patches on 12.09, so I'm not MIA. > I plan to look at patches next week, there are quite a lot of them. OK, good news. Thanks. -- ~Randy ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] fs/ntfs3: fix negative shift size in true_sectors_per_clst() 2022-09-20 15:59 ` [PATCH] fs/ntfs3: fix negative shift size in true_sectors_per_clst() Tetsuo Handa 2022-09-23 0:38 ` Randy Dunlap @ 2022-09-30 16:34 ` Konstantin Komarov 1 sibling, 0 replies; 17+ messages in thread From: Konstantin Komarov @ 2022-09-30 16:34 UTC (permalink / raw) To: Tetsuo Handa, Andrew Morton, Namjae Jeon, Randy Dunlap Cc: syzbot, syzkaller-bugs, ntfs3, LKML On 9/20/22 18:59, Tetsuo Handa wrote: > syzbot is reporting shift-out-of-bounds in true_sectors_per_clst() [1], for > commit a3b774342fa752a5 ("fs/ntfs3: validate BOOT sectors_per_clusters") > did not address that (0 - boot->sectors_per_clusters) < 0 because "u8" was > chosen for type of boot->sectors_per_clusters because 0x80 needs to be > positive in order to support 64K clusters. Use "s8" cast in order to make > sure that (0 - (s8) boot->sectors_per_clusters) > 0. > > Link: https://syzkaller.appspot.com/bug?extid=1631f09646bc214d2e76 [1] > Reported-by: syzbot <syzbot+1631f09646bc214d2e76@syzkaller.appspotmail.com> > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > Tested-by: syzbot <syzbot+1631f09646bc214d2e76@syzkaller.appspotmail.com> > Fixes: a3b774342fa752a5 ("fs/ntfs3: validate BOOT sectors_per_clusters") > > diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c > index 47012c9bf505..c7ffd21fb255 100644 > --- a/fs/ntfs3/super.c > +++ b/fs/ntfs3/super.c > @@ -672,7 +672,7 @@ static u32 true_sectors_per_clst(const struct NTFS_BOOT *boot) > if (boot->sectors_per_clusters <= 0x80) > return boot->sectors_per_clusters; > if (boot->sectors_per_clusters >= 0xf4) /* limit shift to 2MB max */ > - return 1U << (0 - boot->sectors_per_clusters); > + return 1U << (0 - (s8) boot->sectors_per_clusters); > return -EINVAL; > } > Hello Thanks for patch, but there was already a similar patch by Shigeru Yoshida, so I chose it. Sorry about that, thanks again for your work. ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2022-09-30 16:34 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-04-21 8:16 [syzbot] UBSAN: shift-out-of-bounds in ntfs_fill_super syzbot 2022-04-21 8:37 ` syzbot 2022-04-21 12:29 ` syzbot 2022-04-21 12:29 ` [f2fs-dev] " syzbot 2022-09-20 15:59 ` [PATCH] fs/ntfs3: fix negative shift size in true_sectors_per_clst() Tetsuo Handa 2022-09-23 0:38 ` Randy Dunlap 2022-09-23 0:47 ` Joe Perches 2022-09-23 1:14 ` Randy Dunlap 2022-09-23 9:59 ` Dan Carpenter 2022-09-23 1:25 ` Tetsuo Handa 2022-09-23 11:58 ` Aleksandr Nogikh 2022-09-23 14:35 ` Tetsuo Handa 2022-09-24 11:58 ` Aleksandr Nogikh 2022-09-24 12:03 ` Aleksandr Nogikh 2022-09-23 11:58 ` Konstantin Komarov 2022-09-23 16:07 ` Randy Dunlap 2022-09-30 16:34 ` Konstantin Komarov
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.