2022-09-29 08:27:35

by syzbot

[permalink] [raw]
Subject: [syzbot] WARNING in wnd_init

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: [email protected]

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 [email protected].

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


2022-10-02 07:21:38

by syzbot

[permalink] [raw]
Subject: Re: [syzbot] WARNING in wnd_init

syzbot has bisected this issue to:

commit fa3cacf544636b2dc48cfb2f277a2071f14d66a2
Author: Kari Argillander <[email protected]>
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: [email protected]
Fixes: fa3cacf54463 ("fs/ntfs3: Use kernel ALIGN macros over driver specific")

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

2022-10-02 22:09:48

by Kari Argillander

[permalink] [raw]
Subject: Re: [syzbot] WARNING in wnd_init

2.10.2022 syzbot ([email protected]) wrote:
>
> syzbot has bisected this issue to:
>
> commit fa3cacf544636b2dc48cfb2f277a2071f14d66a2
> Author: Kari Argillander <[email protected]>
> 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: [email protected]
> 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

2022-10-04 03:17:37

by Abdun Nihaal

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

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: [email protected]
Fixes: (82cae269cfa95) fs/ntfs3: Add initialization of super block
Signed-off-by: Abdun Nihaal <[email protected]>
---
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

2022-11-12 19:07:05

by Konstantin Komarov

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



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: [email protected]
> Fixes: (82cae269cfa95) fs/ntfs3: Add initialization of super block
> Signed-off-by: Abdun Nihaal <[email protected]>
> ---
> 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!