From:   Ivan Orlov <ivan.orlov0322@gmail.com>
To:     almaz.alexandrovich@paragon-software.com, nathan@kernel.org,
        ndesaulniers@google.com, trix@redhat.com
Cc:     Ivan Orlov <ivan.orlov0322@gmail.com>, ntfs3@lists.linux.dev,
        linux-kernel@vger.kernel.org, llvm@lists.linux.dev,
        skhan@linuxfoundation.org, himadrispandya@gmail.com,
        linux-kernel-mentees@lists.linuxfoundation.org,
        syzbot+478c1bf0e6bf4a8f3a04@syzkaller.appspotmail.com
Subject: [PATCH] FS: NTFS3: Fix UBSAN shift-out-of-bounds in ntfs_fill_super
Date:   Fri, 17 Mar 2023 18:29:58 +0400
Message-Id: <20230317142958.5566-1-ivan.orlov0322@gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

Syzkaller reported the following issue:

UBSAN: shift-out-of-bounds in fs/ntfs3/super.c:777:25
shift exponent 128 is too large for 32-bit type 'unsigned int'
CPU: 0 PID: 5928 Comm: syz-executor258 Not tainted 6.2.0-syzkaller-18300-g2ebd1fbb946d #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/21/2023
Call trace:
 dump_backtrace+0x1c8/0x1f4 arch/arm64/kernel/stacktrace.c:158
 show_stack+0x2c/0x3c arch/arm64/kernel/stacktrace.c:165
 __dump_stack lib/dump_stack.c:88 [inline]
 dump_stack_lvl+0xd0/0x124 lib/dump_stack.c:106
 dump_stack+0x1c/0x28 lib/dump_stack.c:113
 ubsan_epilogue lib/ubsan.c:151 [inline]
 __ubsan_handle_shift_out_of_bounds+0x2f4/0x36c lib/ubsan.c:321
 ntfs_init_from_boot fs/ntfs3/super.c:777 [inline]
 ntfs_fill_super+0x2544/0x3b9c fs/ntfs3/super.c:970
 get_tree_bdev+0x360/0x54c fs/super.c:1282
 ntfs_fs_get_tree+0x28/0x38 fs/ntfs3/super.c:1408
 vfs_get_tree+0x90/0x274 fs/super.c:1489
 do_new_mount+0x25c/0x8c8 fs/namespace.c:3145
 path_mount+0x590/0xe58 fs/namespace.c:3475
 do_mount fs/namespace.c:3488 [inline]
 __do_sys_mount fs/namespace.c:3697 [inline]
 __se_sys_mount fs/namespace.c:3674 [inline]
 __arm64_sys_mount+0x45c/0x594 fs/namespace.c:3674
 __invoke_syscall arch/arm64/kernel/syscall.c:38 [inline]
 invoke_syscall+0x98/0x2c0 arch/arm64/kernel/syscall.c:52
 el0_svc_common+0x138/0x258 arch/arm64/kernel/syscall.c:142
 do_el0_svc+0x64/0x198 arch/arm64/kernel/syscall.c:193
 el0_svc+0x58/0x168 arch/arm64/kernel/entry-common.c:637
 el0t_64_sync_handler+0x84/0xf0 arch/arm64/kernel/entry-common.c:655
 el0t_64_sync+0x190/0x194 arch/arm64/kernel/entry.S:591

This issue is caused by the left shift in 'ntfs_init_from_boot'
function, which uses the absolute value of 'index_size' field as
an exponent in case it is negative. If 'index_size' absolute value
is larger than 30, the issue occurs.

Absolute value of 'index_size' can't be larger than 31, because in case
it is negative it is used as a shift exponent to compute the superblock
index_size (which is a 32-bit unsigned integer). So we need to check this.
If it is larger than 31, the superblock index_size will be zero (because it
will be equal to 1u << (>31))

Additionally, if the absolute value of index_size is equal to 31 it is
correct, and we need to process it correctly. The result of 2U << 31
calculation will be 0, which will cause the ntfs_init_from_boot to
return an error. So let's shift the 2UL instead of 2U to avoid this issue.

We need to do the same with boot record_size as well, because it works in
the similar way.

Tested via syzbot.

Reported-by: syzbot+478c1bf0e6bf4a8f3a04@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?id=00a7d35a2790926d5b91a5279462e0cb958cc689
Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com>
---
 fs/ntfs3/super.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index ef4ea3f21905..9097cf2081df 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -766,15 +766,23 @@ static int ntfs_init_from_boot(struct super_block *sb, u32 sector_size,
 		goto out;
 
 	/* Check MFT record size. */
+	if (boot->record_size < 0 &&
+		-boot->record_size > 31)
+		goto out;
+
 	if ((boot->record_size < 0 &&
-	     SECTOR_SIZE > (2U << (-boot->record_size))) ||
+	     SECTOR_SIZE > (2UL << (-boot->record_size))) ||
 	    (boot->record_size >= 0 && !is_power_of_2(boot->record_size))) {
 		goto out;
 	}
 
 	/* Check index record size. */
+	if (boot->index_size < 0 &&
+		-boot->index_size > 31)
+		goto out;
+
 	if ((boot->index_size < 0 &&
-	     SECTOR_SIZE > (2U << (-boot->index_size))) ||
+	     SECTOR_SIZE > (2UL << (-boot->index_size))) ||
 	    (boot->index_size >= 0 && !is_power_of_2(boot->index_size))) {
 		goto out;
 	}
-- 
2.34.1