* [PATCH] mm/page_alloc: Fix uninitialized variable
@ 2025-02-25 2:20 76824143
2025-02-25 5:29 ` Andrew Morton
0 siblings, 1 reply; 3+ messages in thread
From: 76824143 @ 2025-02-25 2:20 UTC (permalink / raw)
To: akpm; +Cc: linux-mm, Hao Zhang, syzbot+0cfd5e38e96a5596f2b6
From: Hao Zhang <zhanghao1@kylinos.cn>
The variable "compact_result" is not initialized in function
__alloc_pages_direct_compact if order is zero.
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@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=0cfd5e38e96a5596f2b6
Signed-off-by: Hao Zhang <zhanghao1@kylinos.cn>
---
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;
+ }
psi_memstall_enter(&pflags);
delayacct_compact_start();
base-commit: d082ecbc71e9e0bf49883ee4afd435a77a5101b6
--
2.25.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] mm/page_alloc: Fix uninitialized variable
2025-02-25 2:20 [PATCH] mm/page_alloc: Fix uninitialized variable 76824143
@ 2025-02-25 5:29 ` Andrew Morton
2025-02-25 8:12 ` Vlastimil Babka
0 siblings, 1 reply; 3+ messages in thread
From: Andrew Morton @ 2025-02-25 5:29 UTC (permalink / raw)
To: 76824143
Cc: linux-mm, Hao Zhang, syzbot+0cfd5e38e96a5596f2b6, Michal Hocko,
Vlastimil Babka, Mel Gorman
On Tue, 25 Feb 2025 10:20:34 +0800 76824143@qq.com wrote:
> From: Hao Zhang <zhanghao1@kylinos.cn>
>
> 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?
> 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@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=0cfd5e38e96a5596f2b6
> Signed-off-by: Hao Zhang <zhanghao1@kylinos.cn>
> ---
> 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()?
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] mm/page_alloc: Fix uninitialized variable
2025-02-25 5:29 ` Andrew Morton
@ 2025-02-25 8:12 ` Vlastimil Babka
0 siblings, 0 replies; 3+ messages in thread
From: Vlastimil Babka @ 2025-02-25 8:12 UTC (permalink / raw)
To: Andrew Morton, 76824143
Cc: linux-mm, Hao Zhang, syzbot+0cfd5e38e96a5596f2b6, Michal Hocko,
Mel Gorman
On 2/25/25 06:29, Andrew Morton wrote:
> On Tue, 25 Feb 2025 10:20:34 +0800 76824143@qq.com wrote:
>
>> From: Hao Zhang <zhanghao1@kylinos.cn>
>>
>> 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@syzkaller.appspotmail.com
>> Closes: https://syzkaller.appspot.com/bug?extid=0cfd5e38e96a5596f2b6
>> Signed-off-by: Hao Zhang <zhanghao1@kylinos.cn>
Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
>> ---
>> 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.
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-02-25 8:13 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-02-25 2:20 [PATCH] mm/page_alloc: Fix uninitialized variable 76824143
2025-02-25 5:29 ` Andrew Morton
2025-02-25 8:12 ` Vlastimil Babka
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox