* [PATCH] hfsplus: fix KMSAN: uninit-value in hfsplus_lookup()
@ 2025-08-04 19:50 Viacheslav Dubeyko
2025-08-05 16:18 ` Yangtao Li
0 siblings, 1 reply; 4+ messages in thread
From: Viacheslav Dubeyko @ 2025-08-04 19:50 UTC (permalink / raw)
To: glaubitz, linux-fsdevel, frank.li
Cc: Slava.Dubeyko, Viacheslav Dubeyko, syzbot
If Catalog File contains corrupted record for the case of
hidden directory (particularly, entry.folder.id is lesser
than HFSPLUS_FIRSTUSER_CNID), then it can trigger the issue:
[ 65.773760][ T9320] BUG: KMSAN: uninit-value in hfsplus_lookup+0xcd7/0x11f0
[ 65.774362][ T9320] hfsplus_lookup+0xcd7/0x11f0
[ 65.774756][ T9320] __lookup_slow+0x525/0x720
[ 65.775160][ T9320] lookup_slow+0x6a/0xd0
[ 65.775513][ T9320] walk_component+0x393/0x680
[ 65.775896][ T9320] path_lookupat+0x257/0x6c0
[ 65.776313][ T9320] filename_lookup+0x2ac/0x800
[ 65.776693][ T9320] user_path_at+0x8f/0x3c0
[ 65.777078][ T9320] __x64_sys_umount+0x146/0x250
[ 65.777484][ T9320] x64_sys_call+0x2806/0x3d90
[ 65.777851][ T9320] do_syscall_64+0xd9/0x1e0
[ 65.778263][ T9320] entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 65.778716][ T9320]
[ 65.778906][ T9320] Uninit was created at:
[ 65.779294][ T9320] __alloc_frozen_pages_noprof+0x714/0xe60
[ 65.779750][ T9320] alloc_pages_mpol+0x295/0x890
[ 65.780148][ T9320] alloc_frozen_pages_noprof+0xf8/0x1f0
[ 65.780597][ T9320] allocate_slab+0x216/0x1190
[ 65.780961][ T9320] ___slab_alloc+0x104c/0x33c0
[ 65.781543][ T9320] kmem_cache_alloc_lru_noprof+0x8f6/0xe70
[ 65.782135][ T9320] hfsplus_alloc_inode+0x5a/0xd0
[ 65.782608][ T9320] alloc_inode+0x82/0x490
[ 65.783055][ T9320] iget_locked+0x22e/0x1320
[ 65.783495][ T9320] hfsplus_iget+0xc9/0xd70
[ 65.783944][ T9320] hfsplus_btree_open+0x12b/0x1de0
[ 65.784456][ T9320] hfsplus_fill_super+0xc1c/0x27b0
[ 65.784922][ T9320] get_tree_bdev_flags+0x6e6/0x920
[ 65.785403][ T9320] get_tree_bdev+0x38/0x50
[ 65.785819][ T9320] hfsplus_get_tree+0x35/0x40
[ 65.786275][ T9320] vfs_get_tree+0xb3/0x5c0
[ 65.786674][ T9320] do_new_mount+0x73e/0x1630
[ 65.787135][ T9320] path_mount+0x6e3/0x1eb0
[ 65.787564][ T9320] __se_sys_mount+0x73a/0x830
[ 65.787944][ T9320] __x64_sys_mount+0xe4/0x150
[ 65.788346][ T9320] x64_sys_call+0x3904/0x3d90
[ 65.788707][ T9320] do_syscall_64+0xd9/0x1e0
[ 65.789090][ T9320] entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 65.789557][ T9320]
[ 65.789744][ T9320] CPU: 0 UID: 0 PID: 9320 Comm: repro Not tainted 6.14.0-rc5 #5
[ 65.790355][ T9320] Hardware name: QEMU Ubuntu 24.04 PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[ 65.791197][ T9320] =====================================================
[ 65.791814][ T9320] Disabling lock debugging due to kernel taint
[ 65.792419][ T9320] Kernel panic - not syncing: kmsan.panic set ...
[ 65.793000][ T9320] CPU: 0 UID: 0 PID: 9320 Comm: repro Tainted: G B 6.14.0-rc5 #5
[ 65.793830][ T9320] Tainted: [B]=BAD_PAGE
[ 65.794235][ T9320] Hardware name: QEMU Ubuntu 24.04 PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[ 65.795211][ T9320] Call Trace:
[ 65.795519][ T9320] <TASK>
[ 65.795797][ T9320] dump_stack_lvl+0x1fd/0x2b0
[ 65.796256][ T9320] dump_stack+0x1e/0x25
[ 65.796677][ T9320] panic+0x505/0xca0
[ 65.797112][ T9320] ? kmsan_get_metadata+0xf9/0x150
[ 65.797625][ T9320] kmsan_report+0x299/0x2a0
[ 65.798105][ T9320] ? kmsan_internal_unpoison_memory+0x14/0x20
[ 65.798696][ T9320] ? __msan_metadata_ptr_for_load_4+0x24/0x40
[ 65.799291][ T9320] ? __msan_warning+0x96/0x120
[ 65.799785][ T9320] ? hfsplus_lookup+0xcd7/0x11f0
[ 65.800294][ T9320] ? __lookup_slow+0x525/0x720
[ 65.800772][ T9320] ? lookup_slow+0x6a/0xd0
[ 65.801239][ T9320] ? walk_component+0x393/0x680
[ 65.801730][ T9320] ? path_lookupat+0x257/0x6c0
[ 65.802225][ T9320] ? filename_lookup+0x2ac/0x800
[ 65.802720][ T9320] ? user_path_at+0x8f/0x3c0
[ 65.803202][ T9320] ? __x64_sys_umount+0x146/0x250
[ 65.803683][ T9320] ? x64_sys_call+0x2806/0x3d90
[ 65.804177][ T9320] ? do_syscall_64+0xd9/0x1e0
[ 65.804634][ T9320] ? entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 65.805251][ T9320] ? kmsan_get_metadata+0x70/0x150
[ 65.805764][ T9320] ? vprintk_default+0x3f/0x50
[ 65.806256][ T9320] ? vprintk+0x36/0x50
[ 65.806659][ T9320] ? _printk+0x17e/0x1b0
[ 65.807107][ T9320] ? kmsan_get_metadata+0xf9/0x150
[ 65.807621][ T9320] __msan_warning+0x96/0x120
[ 65.808103][ T9320] hfsplus_lookup+0xcd7/0x11f0
[ 65.808587][ T9320] ? kmsan_get_metadata+0x70/0x150
[ 65.809108][ T9320] ? kmsan_get_metadata+0xf9/0x150
[ 65.809627][ T9320] ? kmsan_get_metadata+0xf9/0x150
[ 65.810142][ T9320] ? __pfx_hfsplus_lookup+0x10/0x10
[ 65.810669][ T9320] ? kmsan_get_shadow_origin_ptr+0x4a/0xb0
[ 65.811258][ T9320] ? __pfx_hfsplus_lookup+0x10/0x10
[ 65.811787][ T9320] __lookup_slow+0x525/0x720
[ 65.812258][ T9320] lookup_slow+0x6a/0xd0
[ 65.812700][ T9320] walk_component+0x393/0x680
[ 65.813178][ T9320] ? kmsan_get_metadata+0xf9/0x150
[ 65.813697][ T9320] path_lookupat+0x257/0x6c0
[ 65.814196][ T9320] filename_lookup+0x2ac/0x800
[ 65.814677][ T9320] ? strncpy_from_user+0x255/0x470
[ 65.815193][ T9320] ? kmsan_get_metadata+0xf9/0x150
[ 65.815706][ T9320] ? kmsan_get_shadow_origin_ptr+0x4a/0xb0
[ 65.816290][ T9320] ? __msan_metadata_ptr_for_load_8+0x24/0x40
[ 65.816886][ T9320] user_path_at+0x8f/0x3c0
[ 65.817342][ T9320] ? __x64_sys_umount+0x6d/0x250
[ 65.817834][ T9320] __x64_sys_umount+0x146/0x250
[ 65.818333][ T9320] ? kmsan_internal_set_shadow_origin+0x79/0x110
[ 65.818945][ T9320] x64_sys_call+0x2806/0x3d90
[ 65.819420][ T9320] do_syscall_64+0xd9/0x1e0
[ 65.819876][ T9320] ? irqentry_exit+0x16/0x60
[ 65.820353][ T9320] entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 65.820951][ T9320] RIP: 0033:0x7f822cb8fb07
[ 65.821427][ T9320] Code: 23 0d 00 f7 d8 64 89 01 48 83 c8 ff c3 66 0f 1f 44 00 00 31 f6 e9 09 00 00 00 66 0f 1f 84 00 00 08
[ 65.823225][ T9320] RSP: 002b:00007fff4858f038 EFLAGS: 00000202 ORIG_RAX: 00000000000000a6
[ 65.824037][ T9320] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f822cb8fb07
[ 65.824818][ T9320] RDX: 0000000000000009 RSI: 0000000000000009 RDI: 00007fff4858f0e0
[ 65.825568][ T9320] RBP: 00007fff48590120 R08: 00007f822cc23040 R09: 00007fff4858eed0
[ 65.826329][ T9320] R10: 00007f822cc22fc0 R11: 0000000000000202 R12: 000055bd891e22d0
[ 65.827086][ T9320] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
[ 65.827850][ T9320] </TASK>
[ 65.828677][ T9320] Kernel Offset: disabled
[ 65.829095][ T9320] Rebooting in 86400 seconds..
It means that if hfsplus_iget() receives inode ID lesser than
HFSPLUS_FIRSTUSER_CNID, then it treats it as system inode and
hfsplus_system_read_inode() will be called. As result,
struct hfsplus_inode_info is not initialized properly for
the case of hidden directory. The hidden directory is the record of
Catalog File and hfsplus_cat_read_inode() should be called
for the proper initalization of hidden directory's inode.
This patch adds checking of entry.folder.id for the case of
hidden directory in hfsplus_fill_super(). The CNID of hidden folder
cannot be lesser than HFSPLUS_FIRSTUSER_CNID. And if we receive
such invalid CNID, then record is corrupted and hfsplus_fill_super()
returns the EIO error. Also, patch adds invalid CNID declaration and
declarations of another reserved CNIDs.
Reported-by: syzbot <syzbot+91db973302e7b18c7653@syzkaller.appspotmail.com>
Closes: https://syzkaller.appspot.com/bug?extid=91db973302e7b18c7653
Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
cc: Yangtao Li <frank.li@vivo.com>
cc: linux-fsdevel@vger.kernel.org
---
fs/hfsplus/hfsplus_raw.h | 7 +++++++
fs/hfsplus/super.c | 4 ++++
2 files changed, 11 insertions(+)
diff --git a/fs/hfsplus/hfsplus_raw.h b/fs/hfsplus/hfsplus_raw.h
index 68b4240c6191..bdd4deab46c6 100644
--- a/fs/hfsplus/hfsplus_raw.h
+++ b/fs/hfsplus/hfsplus_raw.h
@@ -194,6 +194,7 @@ struct hfs_btree_header_rec {
#define HFSPLUS_BTREE_HDR_USER_BYTES 128
/* Some special File ID numbers (stolen from hfs.h) */
+#define HFSPLUS_INVALID_CNID 0 /* Invalid id */
#define HFSPLUS_POR_CNID 1 /* Parent Of the Root */
#define HFSPLUS_ROOT_CNID 2 /* ROOT directory */
#define HFSPLUS_EXT_CNID 3 /* EXTents B-tree */
@@ -202,6 +203,12 @@ struct hfs_btree_header_rec {
#define HFSPLUS_ALLOC_CNID 6 /* ALLOCation file */
#define HFSPLUS_START_CNID 7 /* STARTup file */
#define HFSPLUS_ATTR_CNID 8 /* ATTRibutes file */
+#define HFSPLUS_RESERVED_CNID_9 9
+#define HFSPLUS_RESERVED_CNID_10 10
+#define HFSPLUS_RESERVED_CNID_11 11
+#define HFSPLUS_RESERVED_CNID_12 12
+#define HFSPLUS_RESERVED_CNID_13 13
+#define HFSPLUS_REPAIR_CAT_CNID 14 /* Repair CATalog File id */
#define HFSPLUS_EXCH_CNID 15 /* ExchangeFiles temp id */
#define HFSPLUS_FIRSTUSER_CNID 16 /* first available user id */
diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
index 86351bdc8985..8f2790a78e08 100644
--- a/fs/hfsplus/super.c
+++ b/fs/hfsplus/super.c
@@ -527,6 +527,10 @@ static int hfsplus_fill_super(struct super_block *sb, struct fs_context *fc)
err = -EINVAL;
goto out_put_root;
}
+ if (be32_to_cpu(entry.folder.id) < HFSPLUS_FIRSTUSER_CNID) {
+ err = -EIO;
+ goto out_put_root;
+ }
inode = hfsplus_iget(sb, be32_to_cpu(entry.folder.id));
if (IS_ERR(inode)) {
err = PTR_ERR(inode);
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] hfsplus: fix KMSAN: uninit-value in hfsplus_lookup()
2025-08-04 19:50 [PATCH] hfsplus: fix KMSAN: uninit-value in hfsplus_lookup() Viacheslav Dubeyko
@ 2025-08-05 16:18 ` Yangtao Li
2025-08-05 16:21 ` Yangtao Li
2025-08-08 19:47 ` Viacheslav Dubeyko
0 siblings, 2 replies; 4+ messages in thread
From: Yangtao Li @ 2025-08-05 16:18 UTC (permalink / raw)
To: Viacheslav Dubeyko, glaubitz, linux-fsdevel; +Cc: Slava.Dubeyko, syzbot
Hi Slava,
在 2025/8/5 3:50, Viacheslav Dubeyko 写道:
> If Catalog File contains corrupted record for the case of
> hidden directory (particularly, entry.folder.id is lesser
> than HFSPLUS_FIRSTUSER_CNID), then it can trigger the issue:
>
> [ 65.773760][ T9320] BUG: KMSAN: uninit-value in hfsplus_lookup+0xcd7/0x11f0
> [ 65.774362][ T9320] hfsplus_lookup+0xcd7/0x11f0
> [ 65.774756][ T9320] __lookup_slow+0x525/0x720
> [ 65.775160][ T9320] lookup_slow+0x6a/0xd0
> [ 65.775513][ T9320] walk_component+0x393/0x680
> [ 65.775896][ T9320] path_lookupat+0x257/0x6c0
> [ 65.776313][ T9320] filename_lookup+0x2ac/0x800
> [ 65.776693][ T9320] user_path_at+0x8f/0x3c0
> [ 65.777078][ T9320] __x64_sys_umount+0x146/0x250
> [ 65.777484][ T9320] x64_sys_call+0x2806/0x3d90
> [ 65.777851][ T9320] do_syscall_64+0xd9/0x1e0
> [ 65.778263][ T9320] entry_SYSCALL_64_after_hwframe+0x77/0x7f
> [ 65.778716][ T9320]
> [ 65.778906][ T9320] Uninit was created at:
> [ 65.779294][ T9320] __alloc_frozen_pages_noprof+0x714/0xe60
> [ 65.779750][ T9320] alloc_pages_mpol+0x295/0x890
> [ 65.780148][ T9320] alloc_frozen_pages_noprof+0xf8/0x1f0
> [ 65.780597][ T9320] allocate_slab+0x216/0x1190
> [ 65.780961][ T9320] ___slab_alloc+0x104c/0x33c0
> [ 65.781543][ T9320] kmem_cache_alloc_lru_noprof+0x8f6/0xe70
> [ 65.782135][ T9320] hfsplus_alloc_inode+0x5a/0xd0
> [ 65.782608][ T9320] alloc_inode+0x82/0x490
> [ 65.783055][ T9320] iget_locked+0x22e/0x1320
> [ 65.783495][ T9320] hfsplus_iget+0xc9/0xd70
> [ 65.783944][ T9320] hfsplus_btree_open+0x12b/0x1de0
> [ 65.784456][ T9320] hfsplus_fill_super+0xc1c/0x27b0
> [ 65.784922][ T9320] get_tree_bdev_flags+0x6e6/0x920
> [ 65.785403][ T9320] get_tree_bdev+0x38/0x50
> [ 65.785819][ T9320] hfsplus_get_tree+0x35/0x40
> [ 65.786275][ T9320] vfs_get_tree+0xb3/0x5c0
> [ 65.786674][ T9320] do_new_mount+0x73e/0x1630
> [ 65.787135][ T9320] path_mount+0x6e3/0x1eb0
> [ 65.787564][ T9320] __se_sys_mount+0x73a/0x830
> [ 65.787944][ T9320] __x64_sys_mount+0xe4/0x150
> [ 65.788346][ T9320] x64_sys_call+0x3904/0x3d90
> [ 65.788707][ T9320] do_syscall_64+0xd9/0x1e0
> [ 65.789090][ T9320] entry_SYSCALL_64_after_hwframe+0x77/0x7f
> [ 65.789557][ T9320]
> [ 65.789744][ T9320] CPU: 0 UID: 0 PID: 9320 Comm: repro Not tainted 6.14.0-rc5 #5
> [ 65.790355][ T9320] Hardware name: QEMU Ubuntu 24.04 PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
> [ 65.791197][ T9320] =====================================================
> [ 65.791814][ T9320] Disabling lock debugging due to kernel taint
> [ 65.792419][ T9320] Kernel panic - not syncing: kmsan.panic set ...
> [ 65.793000][ T9320] CPU: 0 UID: 0 PID: 9320 Comm: repro Tainted: G B 6.14.0-rc5 #5
> [ 65.793830][ T9320] Tainted: [B]=BAD_PAGE
> [ 65.794235][ T9320] Hardware name: QEMU Ubuntu 24.04 PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
> [ 65.795211][ T9320] Call Trace:
> [ 65.795519][ T9320] <TASK>
> [ 65.795797][ T9320] dump_stack_lvl+0x1fd/0x2b0
> [ 65.796256][ T9320] dump_stack+0x1e/0x25
> [ 65.796677][ T9320] panic+0x505/0xca0
> [ 65.797112][ T9320] ? kmsan_get_metadata+0xf9/0x150
> [ 65.797625][ T9320] kmsan_report+0x299/0x2a0
> [ 65.798105][ T9320] ? kmsan_internal_unpoison_memory+0x14/0x20
> [ 65.798696][ T9320] ? __msan_metadata_ptr_for_load_4+0x24/0x40
> [ 65.799291][ T9320] ? __msan_warning+0x96/0x120
> [ 65.799785][ T9320] ? hfsplus_lookup+0xcd7/0x11f0
> [ 65.800294][ T9320] ? __lookup_slow+0x525/0x720
> [ 65.800772][ T9320] ? lookup_slow+0x6a/0xd0
> [ 65.801239][ T9320] ? walk_component+0x393/0x680
> [ 65.801730][ T9320] ? path_lookupat+0x257/0x6c0
> [ 65.802225][ T9320] ? filename_lookup+0x2ac/0x800
> [ 65.802720][ T9320] ? user_path_at+0x8f/0x3c0
> [ 65.803202][ T9320] ? __x64_sys_umount+0x146/0x250
> [ 65.803683][ T9320] ? x64_sys_call+0x2806/0x3d90
> [ 65.804177][ T9320] ? do_syscall_64+0xd9/0x1e0
> [ 65.804634][ T9320] ? entry_SYSCALL_64_after_hwframe+0x77/0x7f
> [ 65.805251][ T9320] ? kmsan_get_metadata+0x70/0x150
> [ 65.805764][ T9320] ? vprintk_default+0x3f/0x50
> [ 65.806256][ T9320] ? vprintk+0x36/0x50
> [ 65.806659][ T9320] ? _printk+0x17e/0x1b0
> [ 65.807107][ T9320] ? kmsan_get_metadata+0xf9/0x150
> [ 65.807621][ T9320] __msan_warning+0x96/0x120
> [ 65.808103][ T9320] hfsplus_lookup+0xcd7/0x11f0
> [ 65.808587][ T9320] ? kmsan_get_metadata+0x70/0x150
> [ 65.809108][ T9320] ? kmsan_get_metadata+0xf9/0x150
> [ 65.809627][ T9320] ? kmsan_get_metadata+0xf9/0x150
> [ 65.810142][ T9320] ? __pfx_hfsplus_lookup+0x10/0x10
> [ 65.810669][ T9320] ? kmsan_get_shadow_origin_ptr+0x4a/0xb0
> [ 65.811258][ T9320] ? __pfx_hfsplus_lookup+0x10/0x10
> [ 65.811787][ T9320] __lookup_slow+0x525/0x720
> [ 65.812258][ T9320] lookup_slow+0x6a/0xd0
> [ 65.812700][ T9320] walk_component+0x393/0x680
> [ 65.813178][ T9320] ? kmsan_get_metadata+0xf9/0x150
> [ 65.813697][ T9320] path_lookupat+0x257/0x6c0
> [ 65.814196][ T9320] filename_lookup+0x2ac/0x800
> [ 65.814677][ T9320] ? strncpy_from_user+0x255/0x470
> [ 65.815193][ T9320] ? kmsan_get_metadata+0xf9/0x150
> [ 65.815706][ T9320] ? kmsan_get_shadow_origin_ptr+0x4a/0xb0
> [ 65.816290][ T9320] ? __msan_metadata_ptr_for_load_8+0x24/0x40
> [ 65.816886][ T9320] user_path_at+0x8f/0x3c0
> [ 65.817342][ T9320] ? __x64_sys_umount+0x6d/0x250
> [ 65.817834][ T9320] __x64_sys_umount+0x146/0x250
> [ 65.818333][ T9320] ? kmsan_internal_set_shadow_origin+0x79/0x110
> [ 65.818945][ T9320] x64_sys_call+0x2806/0x3d90
> [ 65.819420][ T9320] do_syscall_64+0xd9/0x1e0
> [ 65.819876][ T9320] ? irqentry_exit+0x16/0x60
> [ 65.820353][ T9320] entry_SYSCALL_64_after_hwframe+0x77/0x7f
> [ 65.820951][ T9320] RIP: 0033:0x7f822cb8fb07
> [ 65.821427][ T9320] Code: 23 0d 00 f7 d8 64 89 01 48 83 c8 ff c3 66 0f 1f 44 00 00 31 f6 e9 09 00 00 00 66 0f 1f 84 00 00 08
> [ 65.823225][ T9320] RSP: 002b:00007fff4858f038 EFLAGS: 00000202 ORIG_RAX: 00000000000000a6
> [ 65.824037][ T9320] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f822cb8fb07
> [ 65.824818][ T9320] RDX: 0000000000000009 RSI: 0000000000000009 RDI: 00007fff4858f0e0
> [ 65.825568][ T9320] RBP: 00007fff48590120 R08: 00007f822cc23040 R09: 00007fff4858eed0
> [ 65.826329][ T9320] R10: 00007f822cc22fc0 R11: 0000000000000202 R12: 000055bd891e22d0
> [ 65.827086][ T9320] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
> [ 65.827850][ T9320] </TASK>
> [ 65.828677][ T9320] Kernel Offset: disabled
> [ 65.829095][ T9320] Rebooting in 86400 seconds..
>
> It means that if hfsplus_iget() receives inode ID lesser than
> HFSPLUS_FIRSTUSER_CNID, then it treats it as system inode and
> hfsplus_system_read_inode() will be called. As result,
> struct hfsplus_inode_info is not initialized properly for
> the case of hidden directory. The hidden directory is the record of
> Catalog File and hfsplus_cat_read_inode() should be called
> for the proper initalization of hidden directory's inode.
>
> This patch adds checking of entry.folder.id for the case of
> hidden directory in hfsplus_fill_super(). The CNID of hidden folder
> cannot be lesser than HFSPLUS_FIRSTUSER_CNID. And if we receive
> such invalid CNID, then record is corrupted and hfsplus_fill_super()
> returns the EIO error. Also, patch adds invalid CNID declaration and
> declarations of another reserved CNIDs.
>
> Reported-by: syzbot <syzbot+91db973302e7b18c7653@syzkaller.appspotmail.com>
> Closes: https://syzkaller.appspot.com/bug?extid=91db973302e7b18c7653
> Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
> cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
> cc: Yangtao Li <frank.li@vivo.com>
> cc: linux-fsdevel@vger.kernel.org
> ---
> fs/hfsplus/hfsplus_raw.h | 7 +++++++
> fs/hfsplus/super.c | 4 ++++
> 2 files changed, 11 insertions(+)
>
> diff --git a/fs/hfsplus/hfsplus_raw.h b/fs/hfsplus/hfsplus_raw.h
> index 68b4240c6191..bdd4deab46c6 100644
> --- a/fs/hfsplus/hfsplus_raw.h
> +++ b/fs/hfsplus/hfsplus_raw.h
> @@ -194,6 +194,7 @@ struct hfs_btree_header_rec {
> #define HFSPLUS_BTREE_HDR_USER_BYTES 128
>
> /* Some special File ID numbers (stolen from hfs.h) */
> +#define HFSPLUS_INVALID_CNID 0 /* Invalid id */
Could we drop those for this patch?
If they're needed, we can add them in other patches.
I don't see their usage.
> #define HFSPLUS_POR_CNID 1 /* Parent Of the Root */
> #define HFSPLUS_ROOT_CNID 2 /* ROOT directory */
> #define HFSPLUS_EXT_CNID 3 /* EXTents B-tree */
> @@ -202,6 +203,12 @@ struct hfs_btree_header_rec {
> #define HFSPLUS_ALLOC_CNID 6 /* ALLOCation file */
> #define HFSPLUS_START_CNID 7 /* STARTup file */
> #define HFSPLUS_ATTR_CNID 8 /* ATTRibutes file */
> +#define HFSPLUS_RESERVED_CNID_9 9
> +#define HFSPLUS_RESERVED_CNID_10 10
> +#define HFSPLUS_RESERVED_CNID_11 11
> +#define HFSPLUS_RESERVED_CNID_12 12
> +#define HFSPLUS_RESERVED_CNID_13 13
> +#define HFSPLUS_REPAIR_CAT_CNID 14 /* Repair CATalog File id */
Same.
> #define HFSPLUS_EXCH_CNID 15 /* ExchangeFiles temp id */
> #define HFSPLUS_FIRSTUSER_CNID 16 /* first available user id */
>
> diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
> index 86351bdc8985..8f2790a78e08 100644
> --- a/fs/hfsplus/super.c
> +++ b/fs/hfsplus/super.c
> @@ -527,6 +527,10 @@ static int hfsplus_fill_super(struct super_block *sb, struct fs_context *fc)
> err = -EINVAL;
> goto out_put_root;
> }
> + if (be32_to_cpu(entry.folder.id) < HFSPLUS_FIRSTUSER_CNID) {
> + err = -EIO;
> + goto out_put_root;
> + }
Otherwise, LGTM.
Reviewed-by: Yangtao Li <frank.li <http://frank.li/>@vivo.com
<http://vivo.com/>>
Thx,
Yangtao
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] hfsplus: fix KMSAN: uninit-value in hfsplus_lookup()
2025-08-05 16:18 ` Yangtao Li
@ 2025-08-05 16:21 ` Yangtao Li
2025-08-08 19:47 ` Viacheslav Dubeyko
1 sibling, 0 replies; 4+ messages in thread
From: Yangtao Li @ 2025-08-05 16:21 UTC (permalink / raw)
To: Viacheslav Dubeyko, glaubitz, linux-fsdevel; +Cc: Slava.Dubeyko, syzbot
> Reviewed-by: Yangtao Li <frank.li <http://frank.li/>@vivo.com
> <http://vivo.com/>>
Reviewed-by: Yangtao Li <frank.li@vivo.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] hfsplus: fix KMSAN: uninit-value in hfsplus_lookup()
2025-08-05 16:18 ` Yangtao Li
2025-08-05 16:21 ` Yangtao Li
@ 2025-08-08 19:47 ` Viacheslav Dubeyko
1 sibling, 0 replies; 4+ messages in thread
From: Viacheslav Dubeyko @ 2025-08-08 19:47 UTC (permalink / raw)
To: Yangtao Li, glaubitz, linux-fsdevel; +Cc: Slava.Dubeyko, syzbot
On Wed, 2025-08-06 at 00:18 +0800, Yangtao Li wrote:
> Hi Slava,
>
> 在 2025/8/5 3:50, Viacheslav Dubeyko 写道:
> > If Catalog File contains corrupted record for the case of
> > hidden directory (particularly, entry.folder.id is lesser
> > than HFSPLUS_FIRSTUSER_CNID), then it can trigger the issue:
> >
> > [ 65.773760][ T9320] BUG: KMSAN: uninit-value in
> > hfsplus_lookup+0xcd7/0x11f0
> > [ 65.774362][ T9320] hfsplus_lookup+0xcd7/0x11f0
> > [ 65.774756][ T9320] __lookup_slow+0x525/0x720
> > [ 65.775160][ T9320] lookup_slow+0x6a/0xd0
> > [ 65.775513][ T9320] walk_component+0x393/0x680
> > [ 65.775896][ T9320] path_lookupat+0x257/0x6c0
> > [ 65.776313][ T9320] filename_lookup+0x2ac/0x800
> > [ 65.776693][ T9320] user_path_at+0x8f/0x3c0
> > [ 65.777078][ T9320] __x64_sys_umount+0x146/0x250
> > [ 65.777484][ T9320] x64_sys_call+0x2806/0x3d90
> > [ 65.777851][ T9320] do_syscall_64+0xd9/0x1e0
> > [ 65.778263][ T9320] entry_SYSCALL_64_after_hwframe+0x77/0x7f
> > [ 65.778716][ T9320]
> > [ 65.778906][ T9320] Uninit was created at:
> > [ 65.779294][ T9320] __alloc_frozen_pages_noprof+0x714/0xe60
> > [ 65.779750][ T9320] alloc_pages_mpol+0x295/0x890
> > [ 65.780148][ T9320] alloc_frozen_pages_noprof+0xf8/0x1f0
> > [ 65.780597][ T9320] allocate_slab+0x216/0x1190
> > [ 65.780961][ T9320] ___slab_alloc+0x104c/0x33c0
> > [ 65.781543][ T9320] kmem_cache_alloc_lru_noprof+0x8f6/0xe70
> > [ 65.782135][ T9320] hfsplus_alloc_inode+0x5a/0xd0
> > [ 65.782608][ T9320] alloc_inode+0x82/0x490
> > [ 65.783055][ T9320] iget_locked+0x22e/0x1320
> > [ 65.783495][ T9320] hfsplus_iget+0xc9/0xd70
> > [ 65.783944][ T9320] hfsplus_btree_open+0x12b/0x1de0
> > [ 65.784456][ T9320] hfsplus_fill_super+0xc1c/0x27b0
> > [ 65.784922][ T9320] get_tree_bdev_flags+0x6e6/0x920
> > [ 65.785403][ T9320] get_tree_bdev+0x38/0x50
> > [ 65.785819][ T9320] hfsplus_get_tree+0x35/0x40
> > [ 65.786275][ T9320] vfs_get_tree+0xb3/0x5c0
> > [ 65.786674][ T9320] do_new_mount+0x73e/0x1630
> > [ 65.787135][ T9320] path_mount+0x6e3/0x1eb0
> > [ 65.787564][ T9320] __se_sys_mount+0x73a/0x830
> > [ 65.787944][ T9320] __x64_sys_mount+0xe4/0x150
> > [ 65.788346][ T9320] x64_sys_call+0x3904/0x3d90
> > [ 65.788707][ T9320] do_syscall_64+0xd9/0x1e0
> > [ 65.789090][ T9320] entry_SYSCALL_64_after_hwframe+0x77/0x7f
> > [ 65.789557][ T9320]
> > [ 65.789744][ T9320] CPU: 0 UID: 0 PID: 9320 Comm: repro Not
> > tainted 6.14.0-rc5 #5
> > [ 65.790355][ T9320] Hardware name: QEMU Ubuntu 24.04 PC (i440FX
> > + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
> > [ 65.791197][ T9320]
> > =====================================================
> > [ 65.791814][ T9320] Disabling lock debugging due to kernel taint
> > [ 65.792419][ T9320] Kernel panic - not syncing: kmsan.panic set
> > ...
> > [ 65.793000][ T9320] CPU: 0 UID: 0 PID: 9320 Comm: repro Tainted:
> > G B 6.14.0-rc5 #5
> > [ 65.793830][ T9320] Tainted: [B]=BAD_PAGE
> > [ 65.794235][ T9320] Hardware name: QEMU Ubuntu 24.04 PC (i440FX
> > + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
> > [ 65.795211][ T9320] Call Trace:
> > [ 65.795519][ T9320] <TASK>
> > [ 65.795797][ T9320] dump_stack_lvl+0x1fd/0x2b0
> > [ 65.796256][ T9320] dump_stack+0x1e/0x25
> > [ 65.796677][ T9320] panic+0x505/0xca0
> > [ 65.797112][ T9320] ? kmsan_get_metadata+0xf9/0x150
> > [ 65.797625][ T9320] kmsan_report+0x299/0x2a0
> > [ 65.798105][ T9320] ? kmsan_internal_unpoison_memory+0x14/0x20
> > [ 65.798696][ T9320] ? __msan_metadata_ptr_for_load_4+0x24/0x40
> > [ 65.799291][ T9320] ? __msan_warning+0x96/0x120
> > [ 65.799785][ T9320] ? hfsplus_lookup+0xcd7/0x11f0
> > [ 65.800294][ T9320] ? __lookup_slow+0x525/0x720
> > [ 65.800772][ T9320] ? lookup_slow+0x6a/0xd0
> > [ 65.801239][ T9320] ? walk_component+0x393/0x680
> > [ 65.801730][ T9320] ? path_lookupat+0x257/0x6c0
> > [ 65.802225][ T9320] ? filename_lookup+0x2ac/0x800
> > [ 65.802720][ T9320] ? user_path_at+0x8f/0x3c0
> > [ 65.803202][ T9320] ? __x64_sys_umount+0x146/0x250
> > [ 65.803683][ T9320] ? x64_sys_call+0x2806/0x3d90
> > [ 65.804177][ T9320] ? do_syscall_64+0xd9/0x1e0
> > [ 65.804634][ T9320] ? entry_SYSCALL_64_after_hwframe+0x77/0x7f
> > [ 65.805251][ T9320] ? kmsan_get_metadata+0x70/0x150
> > [ 65.805764][ T9320] ? vprintk_default+0x3f/0x50
> > [ 65.806256][ T9320] ? vprintk+0x36/0x50
> > [ 65.806659][ T9320] ? _printk+0x17e/0x1b0
> > [ 65.807107][ T9320] ? kmsan_get_metadata+0xf9/0x150
> > [ 65.807621][ T9320] __msan_warning+0x96/0x120
> > [ 65.808103][ T9320] hfsplus_lookup+0xcd7/0x11f0
> > [ 65.808587][ T9320] ? kmsan_get_metadata+0x70/0x150
> > [ 65.809108][ T9320] ? kmsan_get_metadata+0xf9/0x150
> > [ 65.809627][ T9320] ? kmsan_get_metadata+0xf9/0x150
> > [ 65.810142][ T9320] ? __pfx_hfsplus_lookup+0x10/0x10
> > [ 65.810669][ T9320] ? kmsan_get_shadow_origin_ptr+0x4a/0xb0
> > [ 65.811258][ T9320] ? __pfx_hfsplus_lookup+0x10/0x10
> > [ 65.811787][ T9320] __lookup_slow+0x525/0x720
> > [ 65.812258][ T9320] lookup_slow+0x6a/0xd0
> > [ 65.812700][ T9320] walk_component+0x393/0x680
> > [ 65.813178][ T9320] ? kmsan_get_metadata+0xf9/0x150
> > [ 65.813697][ T9320] path_lookupat+0x257/0x6c0
> > [ 65.814196][ T9320] filename_lookup+0x2ac/0x800
> > [ 65.814677][ T9320] ? strncpy_from_user+0x255/0x470
> > [ 65.815193][ T9320] ? kmsan_get_metadata+0xf9/0x150
> > [ 65.815706][ T9320] ? kmsan_get_shadow_origin_ptr+0x4a/0xb0
> > [ 65.816290][ T9320] ? __msan_metadata_ptr_for_load_8+0x24/0x40
> > [ 65.816886][ T9320] user_path_at+0x8f/0x3c0
> > [ 65.817342][ T9320] ? __x64_sys_umount+0x6d/0x250
> > [ 65.817834][ T9320] __x64_sys_umount+0x146/0x250
> > [ 65.818333][ T9320] ?
> > kmsan_internal_set_shadow_origin+0x79/0x110
> > [ 65.818945][ T9320] x64_sys_call+0x2806/0x3d90
> > [ 65.819420][ T9320] do_syscall_64+0xd9/0x1e0
> > [ 65.819876][ T9320] ? irqentry_exit+0x16/0x60
> > [ 65.820353][ T9320] entry_SYSCALL_64_after_hwframe+0x77/0x7f
> > [ 65.820951][ T9320] RIP: 0033:0x7f822cb8fb07
> > [ 65.821427][ T9320] Code: 23 0d 00 f7 d8 64 89 01 48 83 c8 ff c3
> > 66 0f 1f 44 00 00 31 f6 e9 09 00 00 00 66 0f 1f 84 00 00 08
> > [ 65.823225][ T9320] RSP: 002b:00007fff4858f038 EFLAGS: 00000202
> > ORIG_RAX: 00000000000000a6
> > [ 65.824037][ T9320] RAX: ffffffffffffffda RBX: 0000000000000000
> > RCX: 00007f822cb8fb07
> > [ 65.824818][ T9320] RDX: 0000000000000009 RSI: 0000000000000009
> > RDI: 00007fff4858f0e0
> > [ 65.825568][ T9320] RBP: 00007fff48590120 R08: 00007f822cc23040
> > R09: 00007fff4858eed0
> > [ 65.826329][ T9320] R10: 00007f822cc22fc0 R11: 0000000000000202
> > R12: 000055bd891e22d0
> > [ 65.827086][ T9320] R13: 0000000000000000 R14: 0000000000000000
> > R15: 0000000000000000
> > [ 65.827850][ T9320] </TASK>
> > [ 65.828677][ T9320] Kernel Offset: disabled
> > [ 65.829095][ T9320] Rebooting in 86400 seconds..
> >
> > It means that if hfsplus_iget() receives inode ID lesser than
> > HFSPLUS_FIRSTUSER_CNID, then it treats it as system inode and
> > hfsplus_system_read_inode() will be called. As result,
> > struct hfsplus_inode_info is not initialized properly for
> > the case of hidden directory. The hidden directory is the record of
> > Catalog File and hfsplus_cat_read_inode() should be called
> > for the proper initalization of hidden directory's inode.
> >
> > This patch adds checking of entry.folder.id for the case of
> > hidden directory in hfsplus_fill_super(). The CNID of hidden folder
> > cannot be lesser than HFSPLUS_FIRSTUSER_CNID. And if we receive
> > such invalid CNID, then record is corrupted and
> > hfsplus_fill_super()
> > returns the EIO error. Also, patch adds invalid CNID declaration
> > and
> > declarations of another reserved CNIDs.
> >
> > Reported-by: syzbot
> > <syzbot+91db973302e7b18c7653@syzkaller.appspotmail.com>
> > Closes:
> > https://syzkaller.appspot.com/bug?extid=91db973302e7b18c7653
> > Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
> > cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
> > cc: Yangtao Li <frank.li@vivo.com>
> > cc: linux-fsdevel@vger.kernel.org
> > ---
> > fs/hfsplus/hfsplus_raw.h | 7 +++++++
> > fs/hfsplus/super.c | 4 ++++
> > 2 files changed, 11 insertions(+)
> >
> > diff --git a/fs/hfsplus/hfsplus_raw.h b/fs/hfsplus/hfsplus_raw.h
> > index 68b4240c6191..bdd4deab46c6 100644
> > --- a/fs/hfsplus/hfsplus_raw.h
> > +++ b/fs/hfsplus/hfsplus_raw.h
> > @@ -194,6 +194,7 @@ struct hfs_btree_header_rec {
> > #define HFSPLUS_BTREE_HDR_USER_BYTES 128
> >
> > /* Some special File ID numbers (stolen from hfs.h) */
> > +#define HFSPLUS_INVALID_CNID 0 /* Invalid id */
>
>
> Could we drop those for this patch?
>
> If they're needed, we can add them in other patches.
>
> I don't see their usage.
>
>
I have spent last two weeks on discussing these declarations for HFS
case. And I see a lot of sense to add these declarations with the goal
not to spend more time for advising not to use hardcoded plain values
instead of human-friendly constants declarations. :) The main point of
these declarations is to clearly define that zero is invalid CNID.
>
> > #define HFSPLUS_POR_CNID 1 /* Parent Of the
> > Root */
> > #define HFSPLUS_ROOT_CNID 2 /* ROOT directory
> > */
> > #define HFSPLUS_EXT_CNID 3 /* EXTents B-tree
> > */
> > @@ -202,6 +203,12 @@ struct hfs_btree_header_rec {
> > #define HFSPLUS_ALLOC_CNID 6 /* ALLOCation file
> > */
> > #define HFSPLUS_START_CNID 7 /* STARTup file */
> > #define HFSPLUS_ATTR_CNID 8 /* ATTRibutes file
> > */
> > +#define HFSPLUS_RESERVED_CNID_9 9
> > +#define HFSPLUS_RESERVED_CNID_10 10
> > +#define HFSPLUS_RESERVED_CNID_11 11
> > +#define HFSPLUS_RESERVED_CNID_12 12
> > +#define HFSPLUS_RESERVED_CNID_13 13
> > +#define HFSPLUS_REPAIR_CAT_CNID 14 /* Repair
> > CATalog File id */
>
>
> Same.
>
The HFSPLUS_REPAIR_CAT_CNID is defined in HFS+ specification [1]. So,
it makes sense to add it here. And the rest declarations show the
reserved CNID values. Now it can be used in the code and I don't need
to advice again to declare these constants.
Thanks,
Slava.
[1]
https://developer.apple.com/library/archive/technotes/tn/tn1150.html#CatalogFile
>
> > #define HFSPLUS_EXCH_CNID 15 /* ExchangeFiles
> > temp id */
> > #define HFSPLUS_FIRSTUSER_CNID 16 /* first
> > available user id */
> >
> > diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
> > index 86351bdc8985..8f2790a78e08 100644
> > --- a/fs/hfsplus/super.c
> > +++ b/fs/hfsplus/super.c
> > @@ -527,6 +527,10 @@ static int hfsplus_fill_super(struct
> > super_block *sb, struct fs_context *fc)
> > err = -EINVAL;
> > goto out_put_root;
> > }
> > + if (be32_to_cpu(entry.folder.id) <
> > HFSPLUS_FIRSTUSER_CNID) {
> > + err = -EIO;
> > + goto out_put_root;
> > + }
>
>
> Otherwise, LGTM.
>
> Reviewed-by: Yangtao Li <frank.li <http://frank.li/>@vivo.com
> <http://vivo.com/>>
>
> Thx,
>
> Yangtao
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-08-08 19:47 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-04 19:50 [PATCH] hfsplus: fix KMSAN: uninit-value in hfsplus_lookup() Viacheslav Dubeyko
2025-08-05 16:18 ` Yangtao Li
2025-08-05 16:21 ` Yangtao Li
2025-08-08 19:47 ` Viacheslav Dubeyko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).