On 2/25/25 06:29, Andrew Morton wrote: > On Tue, 25 Feb 2025 10:20:34 +0800 76824143@xxxxxx wrote: > >> From: Hao Zhang <zhanghao1@xxxxxxxxxx> >> >> The variable "compact_result" is not initialized in function >> __alloc_pages_direct_compact if order is zero. > > Thanks, this could be rather old. Or did we do something recently to > trigger this? I don't think so, perhaps KMSAN improved or something? Note it catches the passing of uninitalized value to should_compact_retry() (according to the reported line 4416), but when compact_result is not initialized, order is 0 and in that case should_compact_retry() does not even look at compact_result. So it was probably harmless all the time, modulo "undefined behavior" I guess. >> If order is zero, initialize "compact_result" to COMPACT_SKIPPED. >> >> BUG: KMSAN: uninit-value in __alloc_pages_slowpath+0xee8/0x16c0 mm/page_alloc.c:4416 >> __alloc_pages_slowpath+0xee8/0x16c0 mm/page_alloc.c:4416 >> __alloc_frozen_pages_noprof+0xa4c/0xe00 mm/page_alloc.c:4752 >> alloc_pages_mpol+0x4cd/0x890 mm/mempolicy.c:2270 >> alloc_frozen_pages_noprof mm/mempolicy.c:2341 [inline] >> alloc_pages_noprof mm/mempolicy.c:2361 [inline] >> folio_alloc_noprof+0x1dc/0x350 mm/mempolicy.c:2371 >> filemap_alloc_folio_noprof+0xa6/0x440 mm/filemap.c:1019 >> __filemap_get_folio+0xb9a/0x1840 mm/filemap.c:1970 >> grow_dev_folio fs/buffer.c:1039 [inline] >> grow_buffers fs/buffer.c:1105 [inline] >> __getblk_slow fs/buffer.c:1131 [inline] >> bdev_getblk+0x2c9/0xab0 fs/buffer.c:1431 >> getblk_unmovable include/linux/buffer_head.h:369 [inline] >> ext4_getblk+0x3b7/0xe50 fs/ext4/inode.c:864 >> ext4_bread_batch+0x9f/0x7d0 fs/ext4/inode.c:933 >> __ext4_find_entry+0x1ebb/0x36c0 fs/ext4/namei.c:1627 >> ext4_lookup_entry fs/ext4/namei.c:1729 [inline] >> ext4_lookup+0x189/0xb40 fs/ext4/namei.c:1797 >> __lookup_slow+0x538/0x710 fs/namei.c:1793 >> lookup_slow+0x6a/0xd0 fs/namei.c:1810 >> walk_component fs/namei.c:2114 [inline] >> link_path_walk+0xf29/0x1420 fs/namei.c:2479 >> path_openat+0x30f/0x6250 fs/namei.c:3985 >> do_filp_open+0x268/0x600 fs/namei.c:4016 >> do_sys_openat2+0x1bf/0x2f0 fs/open.c:1428 >> do_sys_open fs/open.c:1443 [inline] >> __do_sys_openat fs/open.c:1459 [inline] >> __se_sys_openat fs/open.c:1454 [inline] >> __x64_sys_openat+0x2a1/0x310 fs/open.c:1454 >> x64_sys_call+0x36f5/0x3c30 arch/x86/include/generated/asm/syscalls_64.h:258 >> do_syscall_x64 arch/x86/entry/common.c:52 [inline] >> do_syscall_64+0xcd/0x1e0 arch/x86/entry/common.c:83 >> entry_SYSCALL_64_after_hwframe+0x77/0x7f >> >> Local variable compact_result created at: >> __alloc_pages_slowpath+0x66/0x16c0 mm/page_alloc.c:4218 >> __alloc_frozen_pages_noprof+0xa4c/0xe00 mm/page_alloc.c:4752 >> >> Reported-by: syzbot+0cfd5e38e96a5596f2b6@xxxxxxxxxxxxxxxxxxxxxxxxx >> Closes: https://syzkaller.appspot.com/bug?extid=0cfd5e38e96a5596f2b6 >> Signed-off-by: Hao Zhang <zhanghao1@xxxxxxxxxx> Reviewed-by: Vlastimil Babka <vbabka@xxxxxxx> >> --- >> mm/page_alloc.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/mm/page_alloc.c b/mm/page_alloc.c >> index 579789600a3c..7f0136a404b5 100644 >> --- a/mm/page_alloc.c >> +++ b/mm/page_alloc.c >> @@ -3675,8 +3675,10 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order, >> unsigned long pflags; >> unsigned int noreclaim_flag; >> >> - if (!order) >> + if (!order) { >> + *compact_result = COMPACT_SKIPPED; >> return NULL; >> + } >> > > Maybe. Or maybe __alloc_pages_slowpath() shouldn't be passing an > uninitialized compact_result into should_compact_retry()? I think this solution is fine. We could init it from outside to a safe value, but then if we again forget to set it in __alloc_pages_direct_compact() in some corner case, will that value be correct? Better if KMSAN tells us again I think. >
