Received-SPF: pass (google.com: domain of linux-kernel+bounces-126475-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
From: Lizhi Xu <lizhi.xu@windriver.com>
To: <syzbot+fa7b3ab32bcb56c10961@syzkaller.appspotmail.com>
CC: <linux-fsdevel@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <syzkaller-bugs@googlegroups.com>
Subject: [PATCH V2] fs/hfsplus: fix uaf in hfsplus_read_wrapper
Date: Mon, 1 Apr 2024 14:37:54 +0800
Message-ID: <20240401063754.3161903-1-lizhi.xu@windriver.com>
In-Reply-To: <0000000000001126200614f5c9c4@google.com>
References: <0000000000001126200614f5c9c4@google.com>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Content-Type: text/plain

[Syzbot reported]
BUG: KASAN: slab-use-after-free in hfsplus_read_wrapper+0xf86/0x1070 fs/hfsplus/wrapper.c:226
Read of size 2 at addr ffff888024fba400 by task syz-executor204/5218

CPU: 1 PID: 5218 Comm: syz-executor204 Not tainted 6.8.0-syzkaller-08951-gfe46a7dd189e #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 02/29/2024
Call Trace:
 <TASK>
 __dump_stack lib/dump_stack.c:88 [inline]
 dump_stack_lvl+0x116/0x1f0 lib/dump_stack.c:114
 print_address_description mm/kasan/report.c:377 [inline]
 print_report+0xc3/0x620 mm/kasan/report.c:488
 kasan_report+0xd9/0x110 mm/kasan/report.c:601
 hfsplus_read_wrapper+0xf86/0x1070 fs/hfsplus/wrapper.c:226
 hfsplus_fill_super+0x352/0x1bc0 fs/hfsplus/super.c:419
 mount_bdev+0x1e6/0x2d0 fs/super.c:1658
 legacy_get_tree+0x10c/0x220 fs/fs_context.c:662
 vfs_get_tree+0x92/0x380 fs/super.c:1779
 do_new_mount fs/namespace.c:3352 [inline]
 path_mount+0x14e6/0x1f20 fs/namespace.c:3679
 do_mount fs/namespace.c:3692 [inline]
 __do_sys_mount fs/namespace.c:3898 [inline]
 __se_sys_mount fs/namespace.c:3875 [inline]
 __x64_sys_mount+0x297/0x320 fs/namespace.c:3875
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xd5/0x260 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x6d/0x75
RIP: 0033:0x7f706ca0c69a
Code: d8 64 89 02 48 c7 c0 ff ff ff ff eb a6 e8 5e 04 00 00 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffcd3a1c1c8 EFLAGS: 00000286 ORIG_RAX: 00000000000000a5
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f706ca0c69a
RDX: 0000000020000000 RSI: 0000000020000100 RDI: 00007ffcd3a1c210
RBP: 0000000000000004 R08: 00007ffcd3a1c250 R09: 0000000000000632
R10: 0000000000000050 R11: 0000000000000286 R12: 00007ffcd3a1c210
R13: 00007ffcd3a1c250 R14: 0000000000080000 R15: 0000000000000003
 </TASK>
[Fix] 
When the logical_block_size was changed from 512 to 2048, it resulted in 
insufficient space pre allocated to s_backup_vhdr_buf. To solve this problem, 
move the memory allocation of s_backup_vhdr_buf to after the logical_block_size
has been changed.

Reported-and-tested-by: syzbot+fa7b3ab32bcb56c10961@syzkaller.appspotmail.com
Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
---
 fs/hfsplus/wrapper.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/hfsplus/wrapper.c b/fs/hfsplus/wrapper.c
index ce9346099c72..974786e30259 100644
--- a/fs/hfsplus/wrapper.c
+++ b/fs/hfsplus/wrapper.c
@@ -179,16 +179,13 @@ int hfsplus_read_wrapper(struct super_block *sb)
 	sbi->s_vhdr_buf = kmalloc(hfsplus_min_io_size(sb), GFP_KERNEL);
 	if (!sbi->s_vhdr_buf)
 		goto out;
-	sbi->s_backup_vhdr_buf = kmalloc(hfsplus_min_io_size(sb), GFP_KERNEL);
-	if (!sbi->s_backup_vhdr_buf)
-		goto out_free_vhdr;
 
 reread:
 	error = hfsplus_submit_bio(sb, part_start + HFSPLUS_VOLHEAD_SECTOR,
 				   sbi->s_vhdr_buf, (void **)&sbi->s_vhdr,
 				   REQ_OP_READ);
 	if (error)
-		goto out_free_backup_vhdr;
+		goto out_free_vhdr;
 
 	error = -EINVAL;
 	switch (sbi->s_vhdr->signature) {
@@ -199,7 +196,7 @@ int hfsplus_read_wrapper(struct super_block *sb)
 		break;
 	case cpu_to_be16(HFSP_WRAP_MAGIC):
 		if (!hfsplus_read_mdb(sbi->s_vhdr, &wd))
-			goto out_free_backup_vhdr;
+			goto out_free_vhdr;
 		wd.ablk_size >>= HFSPLUS_SECTOR_SHIFT;
 		part_start += (sector_t)wd.ablk_start +
 			       (sector_t)wd.embed_start * wd.ablk_size;
@@ -212,10 +209,13 @@ int hfsplus_read_wrapper(struct super_block *sb)
 		 * (should do this only for cdrom/loop though)
 		 */
 		if (hfs_part_find(sb, &part_start, &part_size))
-			goto out_free_backup_vhdr;
+			goto out_free_vhdr;
 		goto reread;
 	}
 
+	sbi->s_backup_vhdr_buf = kmalloc(hfsplus_min_io_size(sb), GFP_KERNEL);
+	if (!sbi->s_backup_vhdr_buf)
+		goto out_free_vhdr;
 	error = hfsplus_submit_bio(sb, part_start + part_size - 2,
 				   sbi->s_backup_vhdr_buf,
 				   (void **)&sbi->s_backup_vhdr, REQ_OP_READ);
-- 
2.43.0