[syzbot] WARNING in wnd_init

[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+fa4648a5446460b7b963@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 syzkaller@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

Re: [syzbot] WARNING in wnd_init

[syzbot]

syzbot has bisected this issue to:

commit fa3cacf544636b2dc48cfb2f277a2071f14d66a2
Author: Kari Argillander <kari.argillander@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+fa4648a5446460b7b963@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

Re: [syzbot] WARNING in wnd_init

[Kari Argillander]

2.10.2022 syzbot (syzbot+fa4648a5446460b7b963@syzkaller.appspotmail.com) wrote:
>
> syzbot has bisected this issue to:
>
> commit fa3cacf544636b2dc48cfb2f277a2071f14d66a2
> Author: Kari Argillander <kari.argillander@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+fa4648a5446460b7b963@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

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

[PATCH] fs/ntfs3: Validate attribute data and valid sizes

[Abdun Nihaal]

The data_size and valid_size fields of non resident attributes should be
less than the its alloc_size field, but this is not checked in
ntfs_read_mft function.

Syzbot reports a allocation order warning due to a large unchecked value
of data_size getting assigned to inode->i_size which is then passed to
kcalloc.

Add sanity check for ensuring that the data_size and valid_size fields
are not larger than alloc_size field.

Link: https://syzkaller.appspot.com/bug?extid=fa4648a5446460b7b963
Reported-and-tested-by: syzbot+fa4648a5446460b7b963@syzkaller.appspotmail.com
Fixes: (82cae269cfa95) fs/ntfs3: Add initialization of super block
Signed-off-by: Abdun Nihaal <abdun.nihaal@gmail.com>
---
 fs/ntfs3/inode.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c
index e9cf00d14733..9c244029be75 100644
--- a/fs/ntfs3/inode.c
+++ b/fs/ntfs3/inode.c
@@ -132,6 +132,13 @@ static struct inode *ntfs_read_mft(struct inode *inode,
 	if (le16_to_cpu(attr->name_off) + attr->name_len > asize)
 		goto out;
 
+	if (attr->non_res) {
+		t64 = le64_to_cpu(attr->nres.alloc_size);
+		if (le64_to_cpu(attr->nres.data_size) > t64 ||
+		    le64_to_cpu(attr->nres.valid_size) > t64)
+			goto out;
+	}
+
 	switch (attr->type) {
 	case ATTR_STD:
 		if (attr->non_res ||
-- 
2.37.3

Re: [PATCH] fs/ntfs3: Validate attribute data and valid sizes

[Konstantin Komarov]

On 10/4/22 06:15, Abdun Nihaal wrote:
> The data_size and valid_size fields of non resident attributes should be
> less than the its alloc_size field, but this is not checked in
> ntfs_read_mft function.
> 
> Syzbot reports a allocation order warning due to a large unchecked value
> of data_size getting assigned to inode->i_size which is then passed to
> kcalloc.
> 
> Add sanity check for ensuring that the data_size and valid_size fields
> are not larger than alloc_size field.
> 
> Link: https://syzkaller.appspot.com/bug?extid=fa4648a5446460b7b963
> Reported-and-tested-by: syzbot+fa4648a5446460b7b963@syzkaller.appspotmail.com
> Fixes: (82cae269cfa95) fs/ntfs3: Add initialization of super block
> Signed-off-by: Abdun Nihaal <abdun.nihaal@gmail.com>
> ---
>   fs/ntfs3/inode.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c
> index e9cf00d14733..9c244029be75 100644
> --- a/fs/ntfs3/inode.c
> +++ b/fs/ntfs3/inode.c
> @@ -132,6 +132,13 @@ static struct inode *ntfs_read_mft(struct inode *inode,
>   	if (le16_to_cpu(attr->name_off) + attr->name_len > asize)
>   		goto out;
>   
> +	if (attr->non_res) {
> +		t64 = le64_to_cpu(attr->nres.alloc_size);
> +		if (le64_to_cpu(attr->nres.data_size) > t64 ||
> +		    le64_to_cpu(attr->nres.valid_size) > t64)
> +			goto out;
> +	}
> +
>   	switch (attr->type) {
>   	case ATTR_STD:
>   		if (attr->non_res ||

Applied, thanks again for patch!