[syzbot] WARNING in wnd_init

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit: 49c13ed0316d Merge tag 'soc-fixes-6.0-rc7' of git://git.ke..
git tree: upstream
console+strace: https://syzkaller.appspot.com/x/log.txt?x=12615824880000
kernel config: https://syzkaller.appspot.com/x/.config?x=ba0d23aa7e1ffaf5
dashboard link: https://syzkaller.appspot.com/bug?extid=fa4648a5446460b7b963
compiler: Debian clang version 13.0.1-++20220126092033+75e33f71c2da-1~exp1~20220126212112.63, GNU ld (GNU Binutils for Debian) 2.35.2
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=11cad4e0880000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1303781f080000

Downloadable assets:
disk image: https://storage.googleapis.com/418654aab051/disk-49c13ed0.raw.xz
vmlinux: https://storage.googleapis.com/49c501fc7ae3/vmlinux-49c13ed0.xz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+fa4648...@syzkaller.appspotmail.com

loop0: detected capacity change from 0 to 4096
ntfs3: loop0: Different NTFS' sector size (1024) and media sector size (512)
------------[ cut here ]------------
WARNING: CPU: 1 PID: 3607 at mm/page_alloc.c:5525 __alloc_pages+0x30a/0x560 mm/page_alloc.c:5525
Modules linked in:
CPU: 1 PID: 3607 Comm: syz-executor265 Not tainted 6.0.0-rc7-syzkaller-00068-g49c13ed0316d #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/22/2022
RIP: 0010:__alloc_pages+0x30a/0x560 mm/page_alloc.c:5525
Code: 5c 24 04 0f 85 f3 00 00 00 44 89 e1 81 e1 7f ff ff ff a9 00 00 04 00 41 0f 44 cc 41 89 cc e9 e3 00 00 00 c6 05 73 17 29 0c 01 <0f> 0b 83 fb 0a 0f 86 c8 fd ff ff 31 db 48 c7 44 24 20 0e 36 e0 45
RSP: 0018:ffffc900039bf8a0 EFLAGS: 00010246
RAX: ffffc900039bf900 RBX: 0000000000000026 RCX: 0000000000000000
RDX: 0000000000000028 RSI: 0000000000000000 RDI: ffffc900039bf928
RBP: ffffc900039bf9b0 R08: dffffc0000000000 R09: ffffc900039bf900
R10: fffff52000737f25 R11: 1ffff92000737f20 R12: 0000000000040d40
R13: 1ffff92000737f1c R14: dffffc0000000000 R15: 1ffff92000737f18
FS: 0000555556f45300(0000) GS:ffff8880b9b00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00000000005d84c8 CR3: 000000007f9fc000 CR4: 00000000003506e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<TASK>
kmalloc_order+0x41/0x140 mm/slab_common.c:933
kmalloc_order_trace+0x15/0x70 mm/slab_common.c:949
kmalloc_large include/linux/slab.h:529 [inline]
__kmalloc+0x26e/0x370 mm/slub.c:4418
kmalloc_array include/linux/slab.h:640 [inline]
kcalloc include/linux/slab.h:671 [inline]
wnd_init+0x1db/0x310 fs/ntfs3/bitmap.c:664
ntfs_fill_super+0x28ce/0x42a0 fs/ntfs3/super.c:1058
get_tree_bdev+0x400/0x620 fs/super.c:1323
vfs_get_tree+0x88/0x270 fs/super.c:1530
do_new_mount+0x289/0xad0 fs/namespace.c:3040
do_mount fs/namespace.c:3383 [inline]
__do_sys_mount fs/namespace.c:3591 [inline]
__se_sys_mount+0x2d3/0x3c0 fs/namespace.c:3568
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x3d/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
RIP: 0033:0x7efe2672d73a
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:00007fffb31dc6b8 EFLAGS: 00000286 ORIG_RAX: 00000000000000a5
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00007efe2672d73a
RDX: 0000000020000000 RSI: 0000000020000100 RDI: 00007fffb31dc6d0
RBP: 00007fffb31dc6d0 R08: 00007fffb31dc710 R09: 00007fffb31dc710
R10: 0000000000000000 R11: 0000000000000286 R12: 0000000000000004
R13: 00007fffb31dc710 R14: 000000000000010e R15: 0000000020001b50
</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 syzk...@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this issue, for details see:
https://goo.gl/tpsmEJ#testing-patches

[syzbot]

syzbot has bisected this issue to:

commit fa3cacf544636b2dc48cfb2f277a2071f14d66a2
Author: Kari Argillander <kari.arg...@gmail.com>
Date: Thu Aug 26 08:56:29 2021 +0000

fs/ntfs3: Use kernel ALIGN macros over driver specific

bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=11e776f4880000
start commit: 49c13ed0316d Merge tag 'soc-fixes-6.0-rc7' of git://git.ke..
git tree: upstream
final oops: https://syzkaller.appspot.com/x/report.txt?x=13e776f4880000
console output: https://syzkaller.appspot.com/x/log.txt?x=15e776f4880000

kernel config: https://syzkaller.appspot.com/x/.config?x=ba0d23aa7e1ffaf5
dashboard link: https://syzkaller.appspot.com/bug?extid=fa4648a5446460b7b963

syz repro: https://syzkaller.appspot.com/x/repro.syz?x=11cad4e0880000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1303781f080000

Reported-by: syzbot+fa4648...@syzkaller.appspotmail.com
Fixes: fa3cacf54463 ("fs/ntfs3: Use kernel ALIGN macros over driver specific")

For information about bisection process see: https://goo.gl/tpsmEJ#bisection

[Tetsuo Handa]

syzbot is reporting too large allocation at wnd_init() [1], for a crafted
filesystem can become wnd->nwnd close to UINT_MAX. Add __GFP_NOWARN in
order to avoid too large allocation warning, than exhausting memory by
using kvcalloc().

Link: https://syzkaller.appspot.com/bug?extid=fa4648a5446460b7b963 [1]
Reported-by: syzot <syzbot+fa4648...@syzkaller.appspotmail.com>
Signed-off-by: Tetsuo Handa <penguin...@I-love.SAKURA.ne.jp>
---
fs/ntfs3/bitmap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ntfs3/bitmap.c b/fs/ntfs3/bitmap.c
index 5d44ceac855b..90f3c4e84856 100644
--- a/fs/ntfs3/bitmap.c
+++ b/fs/ntfs3/bitmap.c
@@ -661,7 +661,7 @@ int wnd_init(struct wnd_bitmap *wnd, struct super_block *sb, size_t nbits)
if (!wnd->bits_last)
wnd->bits_last = wbits;

- wnd->free_bits = kcalloc(wnd->nwnd, sizeof(u16), GFP_NOFS);
+ wnd->free_bits = kcalloc(wnd->nwnd, sizeof(u16), GFP_NOFS | __GFP_NOWARN);
if (!wnd->free_bits)
return -ENOMEM;

--
2.34.1

[Tetsuo Handa]

syzbot is reporting too large allocation at wnd_init() [1], for a crafted
filesystem can become wnd->nwnd close to UINT_MAX. Add __GFP_NOWARN in
order to avoid too large allocation warning, than exhausting memory by
using kvcalloc().

Link: https://syzkaller.appspot.com/bug?extid=fa4648a5446460b7b963 [1]
Reported-by: syzot <syzbot+fa4648...@syzkaller.appspotmail.com>
Signed-off-by: Tetsuo Handa <penguin...@I-love.SAKURA.ne.jp>
---

Sorry, forgot to update subject line.

[Enrico Mioso]

Hello all!

Is this the expected fix for the issue?
Shouldn't the value be sanitized somehow?
This is intended to be an "honest" question - I am not an experienced kernel nor filesystem programmer, just wondering...

Enrico


On Sun, 2 Oct 2022, Tetsuo Handa wrote:

> Date: Sun, 2 Oct 2022 16:37:34
> From: Tetsuo Handa <penguin...@i-love.sakura.ne.jp>
> To: Konstantin Komarov <almaz.ale...@paragon-software.com>
> Cc: syzbot <syzbot+fa4648...@syzkaller.appspotmail.com>,
> syzkall...@googlegroups.com, nt...@lists.linux.dev,
> Kari Argillander <kari.arg...@gmail.com>
> Subject: Re: [syzbot] WARNING in wnd_init

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-and-tested-by: syzbot+fa4648...@syzkaller.appspotmail.com

Tested on:

commit: a962b54e Merge tag 'i2c-for-6.0-rc8' of git://git.kern..
git tree: https://kernel.googlesource.com/pub/scm/linux/kernel/git/torvalds/linux master
console output: https://syzkaller.appspot.com/x/log.txt?x=126f1304880000
kernel config: https://syzkaller.appspot.com/x/.config?x=755695d26ad09807
dashboard link: https://syzkaller.appspot.com/bug?extid=fa4648a5446460b7b963
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
patch: https://syzkaller.appspot.com/x/patch.diff?x=16c34f22880000

Note: testing is done by a robot and is best-effort only.

[Kari Argillander]

I check what my patch actually changed. In my original patch I did

diff --git a/fs/ntfs3/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h
index b5da2f06f7cbd..d4dd19b822bc2 100644
--- a/fs/ntfs3/ntfs_fs.h
+++ b/fs/ntfs3/ntfs_fs.h
@@ -900,7 +900,7 @@ static inline bool run_is_empty(struct runs_tree *run)
/* NTFS uses quad aligned bitmaps */
static inline size_t bitmap_size(size_t bits)
{
- return QuadAlign((bits + 7) >> 3);
+ return ALIGN((bits + 7) >> 3, 8);
}

QuadAlign was "buggy" so that it did always give a 32 bit result back. ALIGN
macro will give a 64 bit. So bitmap_size now gives different result. To me it
looks like my patch actually fix this behavior. I just didn't notice
this behavior
when I did the patch.

I have tested that if bitmap_size return u32 syzbot will not trigger the issue
anymore. You can see my test patch in the Syzbot dashboard [1]. That is not
prober fix imo, but just wanted to help anyone looking at this problem.

[1]: https://syzkaller.appspot.com/bug?extid=fa4648a5446460b7b963

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-and-tested-by: syzbot+fa4648...@syzkaller.appspotmail.com

Tested on:

commit: 4fe89d07 Linux 6.0
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=15555a0a880000
kernel config: https://syzkaller.appspot.com/x/.config?x=a1c7bd9b9b7d4c47

dashboard link: https://syzkaller.appspot.com/bug?extid=fa4648a5446460b7b963
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2

patch: https://syzkaller.appspot.com/x/patch.diff?x=10e158f0880000

[Tetsuo Handa]

On 2022/10/03 1:04, Enrico Mioso wrote:
> Hello all!
>
> Is this the expected fix for the issue?
> Shouldn't the value be sanitized somehow?
> This is intended to be an "honest" question - I am not an experienced kernel nor filesystem programmer, just wondering...

Well, there would be two approaches.
One is to limit values in u32 range, as attempted by Kari Argillander.
The other is to handle values in u64 range, as attempted by me.

A crafted filesystem image used by this reproducer has

inode->i_size=18446743966335434752 sbi->record_bits=10 tt=18446744073604694080

before calling

err = wnd_init(&sbi->mft.bitmap, sb, tt);

in ntfs_fill_super().

Then, passing such value to wnd_init() makes

nbits=18446744073604694080 bitmap_size(nbits)=2305843009200586760
sb->s_blocksize=4096 sb->s_blocksize_bits=12
wnd->nwnd=562949953418113

which attempts 1023TB memory allocation.

If bitmap_size() were truncated to u32, wnd_init() gets

nbits=18446744073604694080 bitmap_size(nbits)=4281860104
sb->s_blocksize=4096 sb->s_blocksize_bits=12
wnd->nwnd=1045377

which attempts 2MB memory allocation.

Although we need to be careful with u64 overflow inside bitmap_size()
and bytes_to_block(), I guess that handling all values as 64bits is
a preferred approach.

If you want to go with limiting to "u32", please add explanation to
Kari's https://syzkaller.appspot.com/text?tag=Patch&x=16c34f22880000 that
u32 truncation is needed for avoiding too large memory allocation.
A mismatch between function's return value "size_t == u64" and return
statement's "u32" casting looks buggy without clear explanation.
Well, bitmap_size() should not depend on size_t which can be "u32"?

But after all, a patch for "WARNING in ntfs_fill_super" case at
https://lkml.kernel.org/r/2405d6f0-3b1a-6405...@I-love.SAKURA.ne.jp
cannot be replaced by Kari's patch, for bitmap_size() is not called in this case.

>> filesystem can become wnd->nwnd close to UINT_MAX. Add __GFP_NOWARN in

Debug printk() showed that wnd->nwnd was far beyond UINT_MAX due to 64bits.
I should update this part if we go with handle "u64" values approach.

[Konstantin Komarov]

Applied, thanks!