All of lore.kernel.org
 help / color / mirror / Atom feed
* [syzbot] WARNING in wnd_init
@ 2022-09-29  8:15 syzbot
  2022-10-02  7:07 ` syzbot
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: syzbot @ 2022-09-29  8:15 UTC (permalink / raw)
  To: almaz.alexandrovich, linux-kernel, ntfs3, syzkaller-bugs

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [syzbot] WARNING in wnd_init
  2022-09-29  8:15 [syzbot] WARNING in wnd_init syzbot
@ 2022-10-02  7:07 ` syzbot
  2022-10-02 21:52   ` Kari Argillander
  2022-10-02 14:37 ` Tetsuo Handa
  2022-10-04  3:15   ` Abdun Nihaal
  2 siblings, 1 reply; 12+ messages in thread
From: syzbot @ 2022-10-02  7:07 UTC (permalink / raw)
  To: almaz.alexandrovich, kari.argillander, linux-kernel, ntfs3,
	pjwatson999, syzkaller-bugs

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [syzbot] WARNING in wnd_init
  2022-09-29  8:15 [syzbot] WARNING in wnd_init syzbot
  2022-10-02  7:07 ` syzbot
@ 2022-10-02 14:37 ` Tetsuo Handa
  2022-10-02 14:39   ` [PATCH] ntfs3: use __GFP_NOWARN allocation at wnd_init() Tetsuo Handa
  2022-10-02 16:04   ` [syzbot] WARNING in wnd_init Enrico Mioso
  2022-10-04  3:15   ` Abdun Nihaal
  2 siblings, 2 replies; 12+ messages in thread
From: Tetsuo Handa @ 2022-10-02 14:37 UTC (permalink / raw)
  To: Konstantin Komarov; +Cc: syzbot, syzkaller-bugs, ntfs3, Kari Argillander

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+fa4648a5446460b7b963@syzkaller.appspotmail.com>
Signed-off-by: Tetsuo Handa <penguin-kernel@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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH] ntfs3: use __GFP_NOWARN allocation at wnd_init()
  2022-10-02 14:37 ` Tetsuo Handa
@ 2022-10-02 14:39   ` Tetsuo Handa
  2022-11-12 18:08     ` Konstantin Komarov
  2022-10-02 16:04   ` [syzbot] WARNING in wnd_init Enrico Mioso
  1 sibling, 1 reply; 12+ messages in thread
From: Tetsuo Handa @ 2022-10-02 14:39 UTC (permalink / raw)
  To: Konstantin Komarov; +Cc: syzbot, syzkaller-bugs, ntfs3, Kari Argillander

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+fa4648a5446460b7b963@syzkaller.appspotmail.com>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
Sorry, forgot to update subject line.

 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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [syzbot] WARNING in wnd_init
  2022-10-02 14:37 ` Tetsuo Handa
  2022-10-02 14:39   ` [PATCH] ntfs3: use __GFP_NOWARN allocation at wnd_init() Tetsuo Handa
@ 2022-10-02 16:04   ` Enrico Mioso
  2022-10-05 12:17     ` Tetsuo Handa
  1 sibling, 1 reply; 12+ messages in thread
From: Enrico Mioso @ 2022-10-02 16:04 UTC (permalink / raw)
  To: Tetsuo Handa
  Cc: Konstantin Komarov, syzbot, syzkaller-bugs, ntfs3, Kari Argillander

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-kernel@i-love.sakura.ne.jp>
> To: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
> Cc: syzbot <syzbot+fa4648a5446460b7b963@syzkaller.appspotmail.com>,
>     syzkaller-bugs@googlegroups.com, ntfs3@lists.linux.dev,
>     Kari Argillander <kari.argillander@gmail.com>
> Subject: Re: [syzbot] WARNING in wnd_init
> 
> 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+fa4648a5446460b7b963@syzkaller.appspotmail.com>
> Signed-off-by: Tetsuo Handa <penguin-kernel@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
>
>
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [syzbot] WARNING in wnd_init
  2022-10-02  7:07 ` syzbot
@ 2022-10-02 21:52   ` Kari Argillander
  0 siblings, 0 replies; 12+ messages in thread
From: Kari Argillander @ 2022-10-02 21:52 UTC (permalink / raw)
  To: syzbot
  Cc: almaz.alexandrovich, linux-kernel, ntfs3, pjwatson999, syzkaller-bugs

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH] fs/ntfs3: Validate attribute data and valid sizes
  2022-09-29  8:15 [syzbot] WARNING in wnd_init syzbot
@ 2022-10-04  3:15   ` Abdun Nihaal
  2022-10-02 14:37 ` Tetsuo Handa
  2022-10-04  3:15   ` Abdun Nihaal
  2 siblings, 0 replies; 12+ messages in thread
From: Abdun Nihaal @ 2022-10-04  3:15 UTC (permalink / raw)
  To: almaz.alexandrovich
  Cc: ntfs3, linux-kernel, skhan, linux-kernel-mentees, Abdun Nihaal,
	syzbot+fa4648a5446460b7b963

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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH] fs/ntfs3: Validate attribute data and valid sizes
@ 2022-10-04  3:15   ` Abdun Nihaal
  0 siblings, 0 replies; 12+ messages in thread
From: Abdun Nihaal @ 2022-10-04  3:15 UTC (permalink / raw)
  To: almaz.alexandrovich
  Cc: linux-kernel, syzbot+fa4648a5446460b7b963, ntfs3, linux-kernel-mentees

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

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [syzbot] WARNING in wnd_init
  2022-10-02 16:04   ` [syzbot] WARNING in wnd_init Enrico Mioso
@ 2022-10-05 12:17     ` Tetsuo Handa
  0 siblings, 0 replies; 12+ messages in thread
From: Tetsuo Handa @ 2022-10-05 12:17 UTC (permalink / raw)
  To: Enrico Mioso
  Cc: Konstantin Komarov, syzbot, syzkaller-bugs, ntfs3, Kari Argillander

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-610f-fd2efab86bdd@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.

>> order to avoid too large allocation warning, than exhausting memory by
>> using kvcalloc().


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] ntfs3: use __GFP_NOWARN allocation at wnd_init()
  2022-10-02 14:39   ` [PATCH] ntfs3: use __GFP_NOWARN allocation at wnd_init() Tetsuo Handa
@ 2022-11-12 18:08     ` Konstantin Komarov
  0 siblings, 0 replies; 12+ messages in thread
From: Konstantin Komarov @ 2022-11-12 18:08 UTC (permalink / raw)
  To: Tetsuo Handa; +Cc: syzbot, syzkaller-bugs, ntfs3, Kari Argillander



On 10/2/22 17:39, Tetsuo Handa wrote:
> 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+fa4648a5446460b7b963@syzkaller.appspotmail.com>
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> ---
> Sorry, forgot to update subject line.
> 
>   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;
>   

Applied, thanks!

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] fs/ntfs3: Validate attribute data and valid sizes
  2022-10-04  3:15   ` Abdun Nihaal
@ 2022-11-12 18:09     ` Konstantin Komarov via Linux-kernel-mentees
  -1 siblings, 0 replies; 12+ messages in thread
From: Konstantin Komarov @ 2022-11-12 18:09 UTC (permalink / raw)
  To: Abdun Nihaal
  Cc: ntfs3, linux-kernel, skhan, linux-kernel-mentees,
	syzbot+fa4648a5446460b7b963



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!

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] fs/ntfs3: Validate attribute data and valid sizes
@ 2022-11-12 18:09     ` Konstantin Komarov via Linux-kernel-mentees
  0 siblings, 0 replies; 12+ messages in thread
From: Konstantin Komarov via Linux-kernel-mentees @ 2022-11-12 18:09 UTC (permalink / raw)
  To: Abdun Nihaal
  Cc: syzbot+fa4648a5446460b7b963, ntfs3, linux-kernel-mentees, linux-kernel



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!
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-11-12 18:19 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-29  8:15 [syzbot] WARNING in wnd_init syzbot
2022-10-02  7:07 ` syzbot
2022-10-02 21:52   ` Kari Argillander
2022-10-02 14:37 ` Tetsuo Handa
2022-10-02 14:39   ` [PATCH] ntfs3: use __GFP_NOWARN allocation at wnd_init() Tetsuo Handa
2022-11-12 18:08     ` Konstantin Komarov
2022-10-02 16:04   ` [syzbot] WARNING in wnd_init Enrico Mioso
2022-10-05 12:17     ` Tetsuo Handa
2022-10-04  3:15 ` [PATCH] fs/ntfs3: Validate attribute data and valid sizes Abdun Nihaal
2022-10-04  3:15   ` Abdun Nihaal
2022-11-12 18:09   ` Konstantin Komarov
2022-11-12 18:09     ` Konstantin Komarov via Linux-kernel-mentees

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.