[PATCH] kernel/module: disable cfi for do_mod_ctors

[Joey Jiao]

CFI failure when both CONFIG_CONSTRUCTORS and CFI_CLANG enabled.

CFI failure at do_init_module+0x100/0x384 (target:
tsan.module_ctor+0x0/0xa98 [module_name_xx]; expected type: 0xa540670c)

Disable cfi for do_mod_ctors to avoid cfi check on mod->ctors[i]().

Signed-off-by: Joey Jiao 
---
 kernel/module/main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/module/main.c b/kernel/module/main.c
index e1e8a7a9d6c1..d51e63795637 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2453,6 +2453,7 @@ static int post_relocation(struct module *mod, const 
struct load_info *info)
 }
 
 /* Call module constructors. */
+__nocfi
 static void do_mod_ctors(struct module *mod)
 {
 #ifdef CONFIG_CONSTRUCTORS
-- 
2.43.2

[PATCH v5] module: Add CONFIG_MODULE_DISABLE_INIT_FREE option

[Joey Jiao]

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

[PATCH v4] module: Add CONFIG_MODULE_DISABLE_INIT_FREE option

[Joey Jiao]

To facilitate syzkaller test, it's essential for the module to retain the
same address across reboots. In userspace, the execution of modprobe
commands must occur sequentially. In the kernel, selecting the
CONFIG_MODULE_DISABLE_INIT_FREE option disables the asynchronous freeing
of init sections.

Signed-off-by: Joey Jiao 
---
 kernel/module/Kconfig | 12 
 kernel/module/main.c  |  3 ++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig
index 33a2e991f608..88206bc4c7d4 100644
--- a/kernel/module/Kconfig
+++ b/kernel/module/Kconfig
@@ -389,4 +389,16 @@ 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
+   help
+ By default, kernel will free init sections after module being fully
+ loaded.
+
+ MODULE_DISABLE_INIT_FREE allows users to prevent the freeing of init
+ sections. This option is particularly helpful for syzkaller fuzzing,
+ ensuring that the module consistently loads into the same address
+ across reboots.
+
 endif # MODULES
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 98fedfdb8db5..0f242b7b29fe 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 (llist_add(>node, _free_list) &&
+   !IS_ENABLED(CONFIG_MODULE_DISABLE_INIT_FREE))
schedule_work(_free_wq);
 
mutex_unlock(_mutex);
-- 
2.42.0

[PATCH v3] module: Add CONFIG_MODULE_DISABLE_INIT_FREE option

[Joey Jiao]

To facilitate syzkaller test, it's essential for the module to retain the same
address across reboots. In userspace, the execution of modprobe commands must
occur sequentially. In the kernel, selecting the CONFIG_MODULE_DISABLE_INIT_FREE
option disables the asynchronous freeing of init sections.

Signed-off-by: Joey Jiao 
---
 kernel/module/Kconfig | 8 
 kernel/module/main.c  | 5 +++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig
index 33a2e991f608..1cdbee4c51de 100644
--- a/kernel/module/Kconfig
+++ b/kernel/module/Kconfig
@@ -389,4 +389,12 @@ 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
+   help
+ Allows users to prevent the freeing of init sections. This option is
+ particularly helpful for syzkaller fuzzing, ensuring that the module
+ consistently loads into the same address across reboots.
+
 endif # MODULES
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 98fedfdb8db5..a5210b90c078 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2593,8 +2593,9 @@ 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))
-   schedule_work(_free_wq);
+   if (llist_add(>node, _free_list) &&
+   !IS_ENABLED(CONFIG_MODULE_DISABLE_INIT_FREE))
+   schedule_work(_free_wq);
 
mutex_unlock(_mutex);
wake_up_all(_wq);
-- 
2.42.0

[PATCH v2] module: Add CONFIG_MODULE_LOAD_IN_SEQUENCE option

[Joey Jiao]

When modprobe cmds are executed one by one, the final loaded modules
are not in fixed sequence as expected.

Add the option to make sure modules are in fixed sequence across reboot.

Signed-off-by: Joey Jiao 
---
 kernel/module/Kconfig | 11 +++
 kernel/module/main.c  |  3 ++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig
index 33a2e991f608..b45a45f31d6d 100644
--- a/kernel/module/Kconfig
+++ b/kernel/module/Kconfig
@@ -389,4 +389,15 @@ config MODULES_TREE_LOOKUP
def_bool y
depends on PERF_EVENTS || TRACING || CFI_CLANG
 
+config MODULE_LOAD_IN_SEQUENCE
+   bool "Load module in sequence"
+   default n
+   help
+ By default, modules are loaded in random sequence depending on when 
modprobe
+ is executed.
+
+ This option allows modules to be loaded in sequence if modprobe cmds 
are
+ executed one by one in sequence. This option is helpful during 
syzkaller fuzzing
+ to make sure module is loaded into fixed address across device reboot.
+
 endif # MODULES
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 98fedfdb8db5..e238a31d09eb 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2594,7 +2594,8 @@ static noinline int do_init_module(struct module *mod)
 * rcu_barrier()
 */
if (llist_add(>node, _free_list))
-   schedule_work(_free_wq);
+   if (!IS_ENABLED(CONFIG_MODULE_LOAD_IN_SEQUENCE)) {
+   schedule_work(_free_wq);
 
mutex_unlock(_mutex);
wake_up_all(_wq);
-- 
2.42.0

Re: [PATCH] module: Add CONFIG_MODULE_LOAD_IN_SEQUENCE option

[Joey Jiao]

Thanks Luis, will recheck these two points.

On Tue, Oct 10, 2023 at 07:21:13PM -0700, Luis Chamberlain wrote:
> Please find a good email client to reply to patches.
> 
> On Wed, Oct 11, 2023 at 01:57:58AM +0000, Joey Jiao (QUIC) wrote:
> > Hi Luis,
> > 
> > > How is ignoring an error ensuring ordering?
> > The change is just to disable the schedule_work.
> 
> That's different and can be made clearer. Try:
> 
> if (!IS_ENABLED(CONFIG_FOO))
>   schedule_stuff
> 
> > > Why are you making this only now be called with this new kconfig option?
> > This sequence loading is especially helpful for syzkaller coverage decoding.
> > When kaslr is disabled, address inside core kernel is fixed, so syzkaller 
> > can always get right function/line number from addr2line.
> > But module address keeps change across rebooting, in first booting, it 
> > might be loaded at X1, and at X2 after reboot, and at X3 after another 
> > reboot.
> > In this way, syzkaller just can't decode correctly for module address. And 
> > syzkaller currently uses PC and branch info for coverage guided things.
> > 
> > There was a discussion previously here 
> > https://groups.google.com/g/syzkaller/c/1Pnm_BjrZO8/m/WOyAKx8ZAgAJ for 
> > modprobe.
> 
> You are missing my point, you are disabling in effect a piece of code
> where it was not before.
> 
>   Luis

RE: [PATCH] module: Add CONFIG_MODULE_LOAD_IN_SEQUENCE option

[Joey Jiao (QUIC)]

Hi Luis,

> How is ignoring an error ensuring ordering?
It's not ignoring an error ensuring ordering.
I think It's because `schedule_work(_free_wq);` changes when the free_wq 
will happen.
If sometimes freeing init for module A happens before loading another module B, 
that memory is freed. B might be loaded into that freed address.
If sometimes freeing init for module A happens after loading another module B, 
B might be loaded into another memory address.
The change is just to disable the schedule_work.

> Why are you making this only now be called with this new kconfig option?
This sequence loading is especially helpful for syzkaller coverage decoding.
When kaslr is disabled, address inside core kernel is fixed, so syzkaller can 
always get right function/line number from addr2line.
But module address keeps change across rebooting, in first booting, it might be 
loaded at X1, and at X2 after reboot, and at X3 after another reboot.
In this way, syzkaller just can't decode correctly for module address. And 
syzkaller currently uses PC and branch info for coverage guided things.

There was a discussion previously here 
https://groups.google.com/g/syzkaller/c/1Pnm_BjrZO8/m/WOyAKx8ZAgAJ for modprobe.

THX
Joey
-Original Message-
From: Luis Chamberlain  On Behalf Of Luis Chamberlain
Sent: Wednesday, October 11, 2023 6:36 AM
To: Joey Jiao (QUIC) 
Cc: linux-modu...@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH] module: Add CONFIG_MODULE_LOAD_IN_SEQUENCE option

On Mon, Oct 09, 2023 at 10:26:35AM +0530, Joey Jiao wrote:
> When modprobe cmds are executed one by one, the final loaded modules 
> are not in fixed sequence as expected.
> 
> Add the option to make sure modules are in fixed sequence across reboot.
> 
> Signed-off-by: Joey Jiao 
> ---
>  kernel/module/Kconfig | 11 +++  kernel/module/main.c  |  6 
> ++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig index 
> 33a2e991f608..b45a45f31d6d 100644
> --- a/kernel/module/Kconfig
> +++ b/kernel/module/Kconfig
> @@ -389,4 +389,15 @@ config MODULES_TREE_LOOKUP
>   def_bool y
>   depends on PERF_EVENTS || TRACING || CFI_CLANG
>  
> +config MODULE_LOAD_IN_SEQUENCE
> + bool "Load module in sequence"
> + default n
> + help
> +   By default, modules are loaded in random sequence depending on when 
> modprobe
> +   is executed.
> +
> +   This option allows modules to be loaded in sequence if modprobe cmds 
> are
> +   executed one by one in sequence. This option is helpful during 
> syzkaller fuzzing
> +   to make sure module is loaded into fixed address across device reboot.
> +
>  endif # MODULES
> diff --git a/kernel/module/main.c b/kernel/module/main.c index 
> 98fedfdb8db5..587fd84083ae 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -2593,11 +2593,17 @@ static noinline int do_init_module(struct module *mod)
>* be cleaned up needs to sync with the queued work - ie
>* rcu_barrier()
>*/
> +#ifdef CONFIG_MODULE_LOAD_IN_SEQUENCE
> + llist_add(>node, _free_list); #else
>   if (llist_add(>node, _free_list))
>   schedule_work(_free_wq);
> +#endif

How is ignoring an error ensuring ordering?

>   mutex_unlock(_mutex);
> +#ifdef CONFIG_MODULE_LOAD_IN_SEQUENCE
>   wake_up_all(_wq);
> +#endif

Why are you making this only now be called with this new kconfig option?

  Luis

[PATCH] module: Add CONFIG_MODULE_LOAD_IN_SEQUENCE option

[Joey Jiao]

When modprobe cmds are executed one by one, the final loaded modules
are not in fixed sequence as expected.

Add the option to make sure modules are in fixed sequence across reboot.

Signed-off-by: Joey Jiao 
---
 kernel/module/Kconfig | 11 +++
 kernel/module/main.c  |  6 ++
 2 files changed, 17 insertions(+)

diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig
index 33a2e991f608..b45a45f31d6d 100644
--- a/kernel/module/Kconfig
+++ b/kernel/module/Kconfig
@@ -389,4 +389,15 @@ config MODULES_TREE_LOOKUP
def_bool y
depends on PERF_EVENTS || TRACING || CFI_CLANG
 
+config MODULE_LOAD_IN_SEQUENCE
+   bool "Load module in sequence"
+   default n
+   help
+ By default, modules are loaded in random sequence depending on when 
modprobe
+ is executed.
+
+ This option allows modules to be loaded in sequence if modprobe cmds 
are
+ executed one by one in sequence. This option is helpful during 
syzkaller fuzzing
+ to make sure module is loaded into fixed address across device reboot.
+
 endif # MODULES
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 98fedfdb8db5..587fd84083ae 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2593,11 +2593,17 @@ static noinline int do_init_module(struct module *mod)
 * be cleaned up needs to sync with the queued work - ie
 * rcu_barrier()
 */
+#ifdef CONFIG_MODULE_LOAD_IN_SEQUENCE
+   llist_add(>node, _free_list);
+#else
if (llist_add(>node, _free_list))
schedule_work(_free_wq);
+#endif
 
mutex_unlock(_mutex);
+#ifdef CONFIG_MODULE_LOAD_IN_SEQUENCE
wake_up_all(_wq);
+#endif
 
mod_stat_add_long(text_size, _text_size);
mod_stat_add_long(total_size, _mod_size);
-- 
2.42.0