Re: [PATCH v5] module: Add CONFIG_MODULE_DISABLE_INIT_FREE option
On Fri, Oct 27, 2023 at 03:00:00PM +0300, Dan Carpenter wrote: > 607c543f939d8c kernel/module.c Andrii Nakryiko 2020-11-20 2579 #ifdef > CONFIG_DEBUG_INFO_BTF_MODULES > 607c543f939d8c kernel/module.c Andrii Nakryiko 2020-11-20 2580 > /* .BTF is not SHF_ALLOC and will get removed, so sanitize pointer */ > 607c543f939d8c kernel/module.c Andrii Nakryiko 2020-11-20 2581 > mod->btf_data = NULL; > 607c543f939d8c kernel/module.c Andrii Nakryiko 2020-11-20 2582 #endif > c749637909eea5 kernel/module.c Rusty Russell2015-01-20 2583 > /* > c749637909eea5 kernel/module.c Rusty Russell2015-01-20 2584 > * We want to free module_init, but be aware that kallsyms may be > 0be964be0d4508 kernel/module.c Peter Zijlstra 2015-05-27 2585 > * walking this with preempt disabled. In all the failure paths, we > cb2f55369d3a9e kernel/module.c Paul E. McKenney 2018-11-06 2586 > * call synchronize_rcu(), but we don't want to slow down the success > 1a7b7d9220819a kernel/module.c Rick Edgecombe 2019-04-25 2587 > * path. module_memfree() cannot be called in an interrupt, so do the > 1a7b7d9220819a kernel/module.c Rick Edgecombe 2019-04-25 2588 > * work and call synchronize_rcu() in a work queue. > 1a7b7d9220819a kernel/module.c Rick Edgecombe 2019-04-25 2589 > * > ae646f0b9ca135 kernel/module.c Jeffrey Hugo 2018-05-11 2590 > * Note that module_alloc() on most architectures creates W+X page > ae646f0b9ca135 kernel/module.c Jeffrey Hugo 2018-05-11 2591 > * mappings which won't be cleaned up until do_free_init() runs. Any > ae646f0b9ca135 kernel/module.c Jeffrey Hugo 2018-05-11 2592 > * code such as mark_rodata_ro() which depends on those mappings to > ae646f0b9ca135 kernel/module.c Jeffrey Hugo 2018-05-11 2593 > * be cleaned up needs to sync with the queued work - ie > cb2f55369d3a9e kernel/module.c Paul E. McKenney 2018-11-06 2594 > * rcu_barrier() > c749637909eea5 kernel/module.c Rusty Russell2015-01-20 2595 > */ > 36022a47582048 kernel/module/main.c Joey Jiao2023-10-13 2596 > if (!IS_ENABLED(CONFIG_MODULE_DISABLE_INIT_FREE) && > 36022a47582048 kernel/module/main.c Joey Jiao2023-10-13 2597 > llist_add(>node, _free_list)) > > Let's not allocate freeinit if CONFIG_MODULE_DISABLE_INIT_FREE is not > enabled. Wait. It's the other way around actually. freeinit isn't used if CONFIG_MODULE_DISABLE_INIT_FREE is enabled. regards, dan carpenter
Re: [PATCH v5] module: Add CONFIG_MODULE_DISABLE_INIT_FREE option
Hi Joey,
kernel test robot noticed the following build warnings:
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url:
https://github.com/intel-lab-lkp/linux/commits/Joey-Jiao/module-Add-CONFIG_MODULE_DISABLE_INIT_FREE-option/20231017-115509
base: https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git
modules-next
patch link:
https://lore.kernel.org/r/20231013062711.28852-1-quic_jiangenj%40quicinc.com
patch subject: [PATCH v5] module: Add CONFIG_MODULE_DISABLE_INIT_FREE option
config: x86_64-randconfig-161-20231026
(https://download.01.org/0day-ci/archive/20231027/202310271751.28pkvu4k-...@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
reproduce:
(https://download.01.org/0day-ci/archive/20231027/202310271751.28pkvu4k-...@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Reported-by: Dan Carpenter
| Closes: https://lore.kernel.org/r/202310271751.28pkvu4k-...@intel.com/
smatch warnings:
kernel/module/main.c:2608 do_init_module() warn: possible memory leak of
'freeinit'
vim +/freeinit +2608 kernel/module/main.c
c749637909eea5 kernel/module.c Rusty Russell2015-01-20 2517
c749637909eea5 kernel/module.c Rusty Russell2015-01-20 2518
freeinit = kmalloc(sizeof(*freeinit), GFP_KERNEL);
c749637909eea5 kernel/module.c Rusty Russell2015-01-20 2519 if
(!freeinit) {
c749637909eea5 kernel/module.c Rusty Russell2015-01-20 2520
ret = -ENOMEM;
c749637909eea5 kernel/module.c Rusty Russell2015-01-20 2521
goto fail;
c749637909eea5 kernel/module.c Rusty Russell2015-01-20 2522 }
ac3b4328392344 kernel/module/main.c Song Liu 2023-02-06 2523
freeinit->init_text = mod->mem[MOD_INIT_TEXT].base;
ac3b4328392344 kernel/module/main.c Song Liu 2023-02-06 2524
freeinit->init_data = mod->mem[MOD_INIT_DATA].base;
ac3b4328392344 kernel/module/main.c Song Liu 2023-02-06 2525
freeinit->init_rodata = mod->mem[MOD_INIT_RODATA].base;
34e1169d996ab1 kernel/module.c Kees Cook2012-10-16 2526
34e1169d996ab1 kernel/module.c Kees Cook2012-10-16 2527
do_mod_ctors(mod);
34e1169d996ab1 kernel/module.c Kees Cook2012-10-16 2528 /*
Start the module */
34e1169d996ab1 kernel/module.c Kees Cook2012-10-16 2529 if
(mod->init != NULL)
34e1169d996ab1 kernel/module.c Kees Cook2012-10-16 2530
ret = do_one_initcall(mod->init);
34e1169d996ab1 kernel/module.c Kees Cook2012-10-16 2531 if (ret
< 0) {
c749637909eea5 kernel/module.c Rusty Russell2015-01-20 2532
goto fail_free_freeinit;
34e1169d996ab1 kernel/module.c Kees Cook2012-10-16 2533 }
34e1169d996ab1 kernel/module.c Kees Cook2012-10-16 2534 if (ret
> 0) {
bddb12b32f90c5 kernel/module.c Andrew Morton2013-11-12 2535
pr_warn("%s: '%s'->init suspiciously returned %d, it should "
bddb12b32f90c5 kernel/module.c Andrew Morton2013-11-12 2536
"follow 0/-E convention\n"
34e1169d996ab1 kernel/module.c Kees Cook2012-10-16 2537
"%s: loading module anyway...\n",
bddb12b32f90c5 kernel/module.c Andrew Morton2013-11-12 2538
__func__, mod->name, ret, __func__);
34e1169d996ab1 kernel/module.c Kees Cook2012-10-16 2539
dump_stack();
34e1169d996ab1 kernel/module.c Kees Cook2012-10-16 2540 }
34e1169d996ab1 kernel/module.c Kees Cook2012-10-16 2541
34e1169d996ab1 kernel/module.c Kees Cook2012-10-16 2542 /* Now
it's a first class citizen! */
34e1169d996ab1 kernel/module.c Kees Cook2012-10-16 2543
mod->state = MODULE_STATE_LIVE;
34e1169d996ab1 kernel/module.c Kees Cook2012-10-16 2544
blocking_notifier_call_chain(_notify_list,
34e1169d996ab1 kernel/module.c Kees Cook2012-10-16 2545
MODULE_STATE_LIVE, mod);
34e1169d996ab1 kernel/module.c Kees Cook2012-10-16 2546
38dc717e97153e kernel/module.c Jessica Yu 2020-11-27 2547 /*
Delay uevent until module has finished its init routine */
38dc717e97153e kernel/module.c Jessica Yu 2020-11-27 2548
kobject_uevent(>mkobj.kobj, KOBJ_ADD);
38dc717e97153e kernel/module.c Jessica Yu 2020-11-27 2549
774a1221e862b3 kernel/module.c Tejun Heo2013-01-15 2550 /*
774a1221e862b3 kernel/module.c Tejun Heo2013-01-15 2551* We
need to finish all async code before the module init sequence
67d6212afda218 kernel/module.c Igor Pylypiv 2022-01-27 2552* is
done. This has potential to deadlock if synchronou
[PATCH v5] module: Add CONFIG_MODULE_DISABLE_INIT_FREE option
Syzkaller uses the _RET_IP_ (also known as pc) to decode covered file/function/line, and it employs pc ^ hash(prev_pc) (referred to as signal) to indicate covered edge. If the pc for the same file/line keeps changing across reboots, syzkaller will report incorrect coverage data. Additionally, even if kaslr can be disabled, we cannot get the same covered edge for module because both pc and prev_pc have changed, thus altering pc ^ hash(prev_pc). To facilitate syzkaller coverage, it is crucial for both the core kernel and modules to maintain at the same addresses across reboots. So, the following steps are necessary: - In userspace: 1) To maintain an uninterrupted loading sequence, it is recommended to execute modprobe commands by loading one module at a time, to avoid any interference from the scheduler. 2) Avoid unloading any module during fuzzing. - In kernel: 1) Disable CONFIG_RANDOMIZE_BASE to load the core kernel at the same address consistently. 2) To ensure deterministic module loading at the same address, enabling CONFIG_MODULE_DISABLE_INIT_FREE prevents the asynchronous freeing of init sections. Without this option, there is a possibility that the next module could be loaded into previous freed init pages of a previous loaded module. It is important to note that this option is intended for fuzzing tests only and should not be set as the default configuration in production builds. Signed-off-by: Joey Jiao --- kernel/module/Kconfig | 13 + kernel/module/main.c | 3 ++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig index 33a2e991f608..d0df0b5997b0 100644 --- a/kernel/module/Kconfig +++ b/kernel/module/Kconfig @@ -389,4 +389,17 @@ config MODULES_TREE_LOOKUP def_bool y depends on PERF_EVENTS || TRACING || CFI_CLANG +config MODULE_DISABLE_INIT_FREE + bool "Disable freeing of init sections" + default n + depends on !RANDOMIZE_BASE + help + By default, the kernel frees init sections after module is fully + loaded. + + Enabling MODULE_DISABLE_INIT_FREE allows users to prevent the freeing + of init sections. It is particularly helpful for syzkaller fuzzing, + ensuring that the module consistently loads at the same address + across reboots. + endif # MODULES diff --git a/kernel/module/main.c b/kernel/module/main.c index 98fedfdb8db5..d226df3a6cf6 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -2593,7 +2593,8 @@ static noinline int do_init_module(struct module *mod) * be cleaned up needs to sync with the queued work - ie * rcu_barrier() */ - if (llist_add(>node, _free_list)) + if (!IS_ENABLED(CONFIG_MODULE_DISABLE_INIT_FREE) && + llist_add(>node, _free_list)) schedule_work(_free_wq); mutex_unlock(_mutex); -- 2.42.0
