2023-11-20 10:56:59

by Valentin Schneider

[permalink] [raw]
Subject: [PATCH 1/5] jump_label,module: Don't alloc static_key_mod for __ro_after_init keys

From: Peter Zijlstra <[email protected]>

When a static_key is marked ro_after_init, its state will never change
(after init), therefore jump_label_update() will never need to iterate
the entries, and thus module load won't actually need to track this --
avoiding the static_key::next write.

Therefore, mark these keys such that jump_label_add_module() might
recognise them and avoid the modification.

Use the special state: 'static_key_linked(key) && !static_key_mod(key)'
to denote such keys.

Link: http://lore.kernel.org/r/[email protected]
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Signed-off-by: Valentin Schneider <[email protected]>
---
include/asm-generic/sections.h | 5 ++++
include/linux/jump_label.h | 1 +
init/main.c | 1 +
kernel/jump_label.c | 49 ++++++++++++++++++++++++++++++++++
4 files changed, 56 insertions(+)

diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
index db13bb620f527..c768de6f19a9a 100644
--- a/include/asm-generic/sections.h
+++ b/include/asm-generic/sections.h
@@ -180,6 +180,11 @@ static inline bool is_kernel_rodata(unsigned long addr)
addr < (unsigned long)__end_rodata;
}

+static inline bool is_kernel_ro_after_init(unsigned long addr)
+{
+ return addr >= (unsigned long)__start_ro_after_init &&
+ addr < (unsigned long)__end_ro_after_init;
+}
/**
* is_kernel_inittext - checks if the pointer address is located in the
* .init.text section
diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index f0a949b7c9733..88ef9e776af8d 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -216,6 +216,7 @@ extern struct jump_entry __start___jump_table[];
extern struct jump_entry __stop___jump_table[];

extern void jump_label_init(void);
+extern void jump_label_ro(void);
extern void jump_label_lock(void);
extern void jump_label_unlock(void);
extern void arch_jump_label_transform(struct jump_entry *entry,
diff --git a/init/main.c b/init/main.c
index e24b0780fdff7..5f51d8b910dc1 100644
--- a/init/main.c
+++ b/init/main.c
@@ -1407,6 +1407,7 @@ static void mark_readonly(void)
* insecure pages which are W+X.
*/
rcu_barrier();
+ jump_label_ro();
mark_rodata_ro();
rodata_test();
} else
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index d9c822bbffb8d..661ef74dee9b7 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -530,6 +530,45 @@ void __init jump_label_init(void)
cpus_read_unlock();
}

+static inline bool static_key_sealed(struct static_key *key)
+{
+ return (key->type & JUMP_TYPE_LINKED) && !(key->type & ~JUMP_TYPE_MASK);
+}
+
+static inline void static_key_seal(struct static_key *key)
+{
+ unsigned long type = key->type & JUMP_TYPE_TRUE;
+ key->type = JUMP_TYPE_LINKED | type;
+}
+
+void jump_label_ro(void)
+{
+ struct jump_entry *iter_start = __start___jump_table;
+ struct jump_entry *iter_stop = __stop___jump_table;
+ struct jump_entry *iter;
+
+ if (WARN_ON_ONCE(!static_key_initialized))
+ return;
+
+ cpus_read_lock();
+ jump_label_lock();
+
+ for (iter = iter_start; iter < iter_stop; iter++) {
+ struct static_key *iterk = jump_entry_key(iter);
+
+ if (!is_kernel_ro_after_init((unsigned long)iterk))
+ continue;
+
+ if (static_key_sealed(iterk))
+ continue;
+
+ static_key_seal(iterk);
+ }
+
+ jump_label_unlock();
+ cpus_read_unlock();
+}
+
#ifdef CONFIG_MODULES

enum jump_label_type jump_label_init_type(struct jump_entry *entry)
@@ -650,6 +689,15 @@ static int jump_label_add_module(struct module *mod)
static_key_set_entries(key, iter);
continue;
}
+
+ /*
+ * If the key was sealed at init, then there's no need to keep a
+ * a reference to its module entries - just patch them now and
+ * be done with it.
+ */
+ if (static_key_sealed(key))
+ goto do_poke;
+
jlm = kzalloc(sizeof(struct static_key_mod), GFP_KERNEL);
if (!jlm)
return -ENOMEM;
@@ -675,6 +723,7 @@ static int jump_label_add_module(struct module *mod)
static_key_set_linked(key);

/* Only update if we've changed from our initial state */
+do_poke:
if (jump_label_type(iter) != jump_label_init_type(iter))
__jump_label_update(key, iter, iter_stop, true);
}
--
2.41.0


2023-11-20 21:38:35

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 1/5] jump_label,module: Don't alloc static_key_mod for __ro_after_init keys

Hi Valentin,

kernel test robot noticed the following build errors:

[auto build test ERROR on tip/x86/core]
[also build test ERROR on linus/master v6.7-rc2 next-20231120]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Valentin-Schneider/jump_label-module-Don-t-alloc-static_key_mod-for-__ro_after_init-keys/20231120-190044
base: tip/x86/core
patch link: https://lore.kernel.org/r/20231120105528.760306-2-vschneid%40redhat.com
patch subject: [PATCH 1/5] jump_label,module: Don't alloc static_key_mod for __ro_after_init keys
config: s390-allnoconfig (https://download.01.org/0day-ci/archive/20231121/[email protected]/config)
compiler: s390-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231121/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

init/main.c: In function 'mark_readonly':
>> init/main.c:1406:17: error: implicit declaration of function 'jump_label_ro'; did you mean 'jump_label_lock'? [-Werror=implicit-function-declaration]
1406 | jump_label_ro();
| ^~~~~~~~~~~~~
| jump_label_lock
cc1: some warnings being treated as errors


vim +1406 init/main.c

1394
1395 #ifdef CONFIG_STRICT_KERNEL_RWX
1396 static void mark_readonly(void)
1397 {
1398 if (rodata_enabled) {
1399 /*
1400 * load_module() results in W+X mappings, which are cleaned
1401 * up with call_rcu(). Let's make sure that queued work is
1402 * flushed so that we don't hit false positives looking for
1403 * insecure pages which are W+X.
1404 */
1405 rcu_barrier();
> 1406 jump_label_ro();
1407 mark_rodata_ro();
1408 rodata_test();
1409 } else
1410 pr_info("Kernel memory protection disabled.\n");
1411 }
1412 #elif defined(CONFIG_ARCH_HAS_STRICT_KERNEL_RWX)
1413 static inline void mark_readonly(void)
1414 {
1415 pr_warn("Kernel memory protection not selected by kernel config.\n");
1416 }
1417 #else
1418 static inline void mark_readonly(void)
1419 {
1420 pr_warn("This architecture does not have kernel memory protection.\n");
1421 }
1422 #endif
1423

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2023-11-20 22:49:01

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 1/5] jump_label,module: Don't alloc static_key_mod for __ro_after_init keys

Hi Valentin,

kernel test robot noticed the following build errors:

[auto build test ERROR on tip/x86/core]
[also build test ERROR on linus/master v6.7-rc2 next-20231120]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Valentin-Schneider/jump_label-module-Don-t-alloc-static_key_mod-for-__ro_after_init-keys/20231120-190044
base: tip/x86/core
patch link: https://lore.kernel.org/r/20231120105528.760306-2-vschneid%40redhat.com
patch subject: [PATCH 1/5] jump_label,module: Don't alloc static_key_mod for __ro_after_init keys
config: i386-allnoconfig (https://download.01.org/0day-ci/archive/20231121/[email protected]/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231121/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

>> init/main.c:1406:3: error: call to undeclared function 'jump_label_ro'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
jump_label_ro();
^
init/main.c:1406:3: note: did you mean 'jump_label_lock'?
include/linux/jump_label.h:315:20: note: 'jump_label_lock' declared here
static inline void jump_label_lock(void) {}
^
1 error generated.


vim +/jump_label_ro +1406 init/main.c

1394
1395 #ifdef CONFIG_STRICT_KERNEL_RWX
1396 static void mark_readonly(void)
1397 {
1398 if (rodata_enabled) {
1399 /*
1400 * load_module() results in W+X mappings, which are cleaned
1401 * up with call_rcu(). Let's make sure that queued work is
1402 * flushed so that we don't hit false positives looking for
1403 * insecure pages which are W+X.
1404 */
1405 rcu_barrier();
> 1406 jump_label_ro();
1407 mark_rodata_ro();
1408 rodata_test();
1409 } else
1410 pr_info("Kernel memory protection disabled.\n");
1411 }
1412 #elif defined(CONFIG_ARCH_HAS_STRICT_KERNEL_RWX)
1413 static inline void mark_readonly(void)
1414 {
1415 pr_warn("Kernel memory protection not selected by kernel config.\n");
1416 }
1417 #else
1418 static inline void mark_readonly(void)
1419 {
1420 pr_warn("This architecture does not have kernel memory protection.\n");
1421 }
1422 #endif
1423

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki