Hi, Harry,
On Wed, Feb 12, 2025 at 11:39 PM Harry (Hyeonggon) Yoo 42.hyeyoo@gmail.com wrote:
On Wed, Feb 12, 2025 at 11:17 PM Huacai Chen chenhuacai@loongson.cn wrote:
Hibernation assumes the memory layout after resume be the same as that before sleep, but CONFIG_RANDOM_KMALLOC_CACHES breaks this assumption.
[Let's also Cc SLAB ALLOCATOR folks in MAINTAINERS file]
Could you please elaborate what do you mean by hibernation assumes 'the memory layout' after resume be the same as that before sleep?
I don't understand how updating random_kmalloc_seed breaks resuming from hibernation. Changing random_kmalloc_seed affects which kmalloc caches newly allocated objects are from, but it should not affect the objects that are already allocated (before hibernation).
When resuming, the booting kernel should switch to the target kernel, if the address of switch code (from the booting kernel) is the effective data of the target kernel, then the switch code may be overwritten.
For LoongArch there is an additional problem: the regular kernel function uses absolute address to call exception handlers, this means the code calls to exception handlers should at the same address for booting kernel and target kernel.
At least on LoongArch and ARM64 we observed failures of resuming from hibernation (on LoongArch non-boot CPUs fail to bringup, on ARM64 some devices are unusable).
Did you have any chance to reproduce it on x86_64?
I haven't reproduce on x86_64, but I have heard that x86_32 has problems.
software_resume_initcall(), the function which resume the target kernel is a initcall function. So, move the random_kmalloc_seed initialisation after all initcalls.
Cc: stable@vger.kernel.org Fixes: 3c6152940584290668 ("Randomized slab caches for kmalloc()") Reported-by: Yuli Wang wangyuli@uniontech.com Signed-off-by: Huacai Chen chenhuacai@loongson.cn
init/main.c | 3 +++ mm/slab_common.c | 3 --- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/init/main.c b/init/main.c index 2a1757826397..1362957bdbe4 100644 --- a/init/main.c +++ b/init/main.c @@ -1458,6 +1458,9 @@ static int __ref kernel_init(void *unused) /* need to finish all async __init code before freeing the memory */ async_synchronize_full();
+#ifdef CONFIG_RANDOM_KMALLOC_CACHES
random_kmalloc_seed = get_random_u64();+#endif
It doesn’t seem appropriate to put slab code in kernel_init.
Additionally, it introduces a dependency that the code must be executed after all late_initcalls, which sounds like introducing yet another type of initcall.
What about introducing a function to initialize kmalloc seed in slab_common.c, and then call it at kernel_init()? I don't have a better solution than this.
system_state = SYSTEM_FREEING_INITMEM; kprobe_free_init_mem(); ftrace_free_init_mem();diff --git a/mm/slab_common.c b/mm/slab_common.c index 4030907b6b7d..23e324aee218 100644 --- a/mm/slab_common.c +++ b/mm/slab_common.c @@ -971,9 +971,6 @@ void __init create_kmalloc_caches(void) for (i = KMALLOC_SHIFT_LOW; i <= KMALLOC_SHIFT_HIGH; i++) new_kmalloc_cache(i, type); } -#ifdef CONFIG_RANDOM_KMALLOC_CACHES
random_kmalloc_seed = get_random_u64();-#endif
I have no idea how hibernation and resume work, but let me ask here: Can we simply skip or defer updating random_kmalloc_seed when the system is resuming from hibernation? (probably system_state represents this?)
Do you mean something like below? It does work (it is my original solution), but this patch is simpler.
diff --git a/include/linux/slab.h b/include/linux/slab.h index b35e2db7eb0e..42fb91650b13 100644 --- a/include/linux/slab.h +++ b/include/linux/slab.h @@ -614,14 +614,20 @@ static __always_inline enum kmalloc_cache_type kmalloc_type(gfp_t flags, unsigne * The most common case is KMALLOC_NORMAL, so test for it * with a single branch for all the relevant flags. */ - if (likely((flags & KMALLOC_NOT_NORMAL_BITS) == 0)) + if (likely((flags & KMALLOC_NOT_NORMAL_BITS) == 0)) { #ifdef CONFIG_RANDOM_KMALLOC_CACHES + unsigned long random_seed = 0; + + if (system_state > SYSTEM_SCHEDULING) + random_seed = random_kmalloc_seed; + /* RANDOM_KMALLOC_CACHES_NR (=15) copies + the KMALLOC_NORMAL */ - return KMALLOC_RANDOM_START + hash_64(caller ^ random_kmalloc_seed, + return KMALLOC_RANDOM_START + hash_64(caller ^ random_seed,
ilog2(RANDOM_KMALLOC_CACHES_NR + 1)); #else return KMALLOC_NORMAL; #endif + }
Huacai
/* Kmalloc array is now usable */ slab_state = UP;-- Harry