Thread (17 messages) 17 messages, 6 authors, 2024-01-31

Re: [PATCH 1/3] init: Declare rodata_enabled and mark_rodata_ro() at all time

From: Christophe Leroy <hidden>
Date: 2023-12-22 18:23:19
Also in: linux-modules, lkml


Le 22/12/2023 à 06:35, Kees Cook a écrit :
[Vous ne recevez pas souvent de courriers de kees@kernel.org. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]

On December 21, 2023 4:16:56 AM PST, Michael Ellerman [off-list ref] wrote:
quoted
Cc +Kees

Christophe Leroy [off-list ref] writes:
quoted
Declaring rodata_enabled and mark_rodata_ro() at all time
helps removing related #ifdefery in C files.

Signed-off-by: Christophe Leroy <redacted>
---
  include/linux/init.h |  4 ----
  init/main.c          | 21 +++++++--------------
  2 files changed, 7 insertions(+), 18 deletions(-)
diff --git a/include/linux/init.h b/include/linux/init.h
index 01b52c9c7526..d2b47be38a07 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -168,12 +168,8 @@ extern initcall_entry_t __initcall_end[];

  extern struct file_system_type rootfs_fs_type;

-#if defined(CONFIG_STRICT_KERNEL_RWX) || defined(CONFIG_STRICT_MODULE_RWX)
  extern bool rodata_enabled;
-#endif
-#ifdef CONFIG_STRICT_KERNEL_RWX
  void mark_rodata_ro(void);
-#endif

  extern void (*late_time_init)(void);
diff --git a/init/main.c b/init/main.c
index e24b0780fdff..807df08c501f 100644
--- a/init/main.c
+++ b/init/main.c
@@ -1396,10 +1396,9 @@ static int __init set_debug_rodata(char *str)
  early_param("rodata", set_debug_rodata);
  #endif

-#ifdef CONFIG_STRICT_KERNEL_RWX
  static void mark_readonly(void)
  {
-    if (rodata_enabled) {
+    if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX) && rodata_enabled) {
I think this will break without rodata_enabled actual existing on other architectures. (Only declaration was made visible, not the definition, which is above here and still behind ifdefs?)
The compiler constant-folds IS_ENABLED(CONFIG_STRICT_KERNEL_RWX).
When it is false, the second part is dropped.

Exemple:

bool test(void)
{
	if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX) && rodata_enabled)
		return true;
	else
		return false;
}

With CONFIG_STRICT_KERNEL_RWX set, it directly returns the content of 
rodata_enabled:

00000160 <test>:
  160:	3d 20 00 00 	lis     r9,0
			162: R_PPC_ADDR16_HA	rodata_enabled
  164:	88 69 00 00 	lbz     r3,0(r9)
			166: R_PPC_ADDR16_LO	rodata_enabled
  168:	4e 80 00 20 	blr

With CONFIG_STRICT_KERNEL_RWX unset, it returns 0 and doesn't reference 
rodata_enabled at all:

000000bc <test>:
   bc:	38 60 00 00 	li      r3,0
   c0:	4e 80 00 20 	blr

Many places in the kernel use this approach to minimise amount of #ifdefs.

Christophe
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help