* [PATCH v3 0/3] kprobes: Remove MODULES dependency
@ 2020-07-14 22:32 Jarkko Sakkinen
2020-07-14 22:32 ` [PATCH v3 1/3] kprobes: Add text_alloc() and text_free() Jarkko Sakkinen
` (2 more replies)
0 siblings, 3 replies; 18+ messages in thread
From: Jarkko Sakkinen @ 2020-07-14 22:32 UTC (permalink / raw)
To: linux-kernel
Cc: x86, Jarkko Sakkinen, Alexandre Ghiti, Andrew Morton,
Andy Lutomirski, Aneesh Kumar K.V, Arnd Bergmann, Babu Moger,
Borislav Petkov, Brian Gerst, H. Peter Anvin, Jiri Kosina,
Joe Lawrence, Josh Poimboeuf, Kees Cook, Krzysztof Kozlowski,
open list:LIVE PATCHING, Marco Elver, Masahiro Yamada,
Mike Rapoport, Miroslav Benes, Nayna Jain, Omar Sandoval,
Paul E. McKenney, Peter Collingbourne, Peter Zijlstra,
Sami Tolvanen, Stephen Boyd, Thomas Gleixner, Will Deacon
Remove MODULES dependency and migrate from module_alloc to the new
text_alloc() API Right now one has to compile LKM support only to enable
kprobes. With this change applied, it is somewhat easier to create
custom test kernel's with a proper debugging capabilities, thus making
Linux more developer friendly.
Jarkko Sakkinen (3):
kprobes: Add text_alloc() and text_free()
module: Add lock_modules() and unlock_modules()
kprobes: Flag out CONFIG_MODULES dependent code
arch/Kconfig | 2 +-
arch/x86/Kconfig | 3 ++
arch/x86/kernel/Makefile | 1 +
arch/x86/kernel/module.c | 49 -------------------------
arch/x86/kernel/text.c | 71 +++++++++++++++++++++++++++++++++++++
include/linux/module.h | 29 +++++++++++----
include/linux/text.h | 17 +++++++++
kernel/kprobes.c | 22 ++++++++++--
kernel/livepatch/core.c | 8 ++---
kernel/module.c | 70 ++++++++++++++++++++----------------
kernel/trace/trace_kprobe.c | 20 +++++++++--
11 files changed, 196 insertions(+), 96 deletions(-)
create mode 100644 arch/x86/kernel/text.c
create mode 100644 include/linux/text.h
--
2.25.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 1/3] kprobes: Add text_alloc() and text_free()
2020-07-14 22:32 [PATCH v3 0/3] kprobes: Remove MODULES dependency Jarkko Sakkinen
@ 2020-07-14 22:32 ` Jarkko Sakkinen
2020-07-15 0:59 ` kernel test robot
` (3 more replies)
2020-07-14 22:32 ` [PATCH v3 2/3] module: Add lock_modules() and unlock_modules() Jarkko Sakkinen
2020-07-14 22:32 ` [PATCH v3 3/3] kprobes: Flag out CONFIG_MODULES dependent code Jarkko Sakkinen
2 siblings, 4 replies; 18+ messages in thread
From: Jarkko Sakkinen @ 2020-07-14 22:32 UTC (permalink / raw)
To: linux-kernel
Cc: x86, Jarkko Sakkinen, Andi Kleen, Peter Zijlstra,
Thomas Gleixner, Ingo Molnar, Borislav Petkov,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
H. Peter Anvin, Naveen N. Rao, Anil S Keshavamurthy,
David S. Miller, Masami Hiramatsu, Jessica Yu, Andrew Morton,
Aneesh Kumar K.V, Kees Cook, Will Deacon, Sami Tolvanen,
Alexandre Ghiti, Masahiro Yamada, Peter Collingbourne,
Frederic Weisbecker, Krzysztof Kozlowski, Arnd Bergmann,
Stephen Boyd, Andy Lutomirski, Josh Poimboeuf, Miroslav Benes,
Babu Moger, Omar Sandoval, Nayna Jain, Marco Elver, Brian Gerst,
Jiri Kosina, Joe Lawrence, Mike Rapoport
Introduce new API for allocating space for code generaed at run-time
leveraging from module_alloc() and module_memfree() code. Use this to
perform memory allocations in the kprobes code in order to loose the
bound between kprobes and modules subsystem.
Initially, use this API only with arch/x86 and define a new config
flag CONFIG_ARCH_HAS_TEXT_ALLOC to promote the availability of the
new API.
Cc: Andi Kleen <ak@linux.intel.com>
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
arch/Kconfig | 2 +-
arch/x86/Kconfig | 3 ++
arch/x86/kernel/Makefile | 1 +
arch/x86/kernel/module.c | 49 ---------------------------
arch/x86/kernel/text.c | 71 ++++++++++++++++++++++++++++++++++++++++
include/linux/text.h | 17 ++++++++++
kernel/kprobes.c | 11 +++++++
kernel/module.c | 10 ++++++
8 files changed, 114 insertions(+), 50 deletions(-)
create mode 100644 arch/x86/kernel/text.c
create mode 100644 include/linux/text.h
diff --git a/arch/Kconfig b/arch/Kconfig
index 8cc35dc556c7..e3d6b6868ccb 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -61,7 +61,7 @@ config OPROFILE_NMI_TIMER
config KPROBES
bool "Kprobes"
- depends on MODULES
+ depends on MODULES || ARCH_HAS_TEXT_ALLOC
depends on HAVE_KPROBES
select KALLSYMS
help
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 0dea7fdd7a00..a4ee5d1300f6 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2035,6 +2035,9 @@ config KEXEC_FILE
config ARCH_HAS_KEXEC_PURGATORY
def_bool KEXEC_FILE
+config ARCH_HAS_TEXT_ALLOC
+ def_bool y
+
config KEXEC_SIG
bool "Verify kernel signature during kexec_file_load() syscall"
depends on KEXEC_FILE
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index e77261db2391..2878e4b753a0 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -68,6 +68,7 @@ obj-y += tsc.o tsc_msr.o io_delay.o rtc.o
obj-y += pci-iommu_table.o
obj-y += resource.o
obj-y += irqflags.o
+obj-y += text.o
obj-y += process.o
obj-y += fpu/
diff --git a/arch/x86/kernel/module.c b/arch/x86/kernel/module.c
index 34b153cbd4ac..261df078f127 100644
--- a/arch/x86/kernel/module.c
+++ b/arch/x86/kernel/module.c
@@ -36,55 +36,6 @@ do { \
} while (0)
#endif
-#ifdef CONFIG_RANDOMIZE_BASE
-static unsigned long module_load_offset;
-
-/* Mutex protects the module_load_offset. */
-static DEFINE_MUTEX(module_kaslr_mutex);
-
-static unsigned long int get_module_load_offset(void)
-{
- if (kaslr_enabled()) {
- mutex_lock(&module_kaslr_mutex);
- /*
- * Calculate the module_load_offset the first time this
- * code is called. Once calculated it stays the same until
- * reboot.
- */
- if (module_load_offset == 0)
- module_load_offset =
- (get_random_int() % 1024 + 1) * PAGE_SIZE;
- mutex_unlock(&module_kaslr_mutex);
- }
- return module_load_offset;
-}
-#else
-static unsigned long int get_module_load_offset(void)
-{
- return 0;
-}
-#endif
-
-void *module_alloc(unsigned long size)
-{
- void *p;
-
- if (PAGE_ALIGN(size) > MODULES_LEN)
- return NULL;
-
- p = __vmalloc_node_range(size, MODULE_ALIGN,
- MODULES_VADDR + get_module_load_offset(),
- MODULES_END, GFP_KERNEL,
- PAGE_KERNEL, 0, NUMA_NO_NODE,
- __builtin_return_address(0));
- if (p && (kasan_module_alloc(p, size) < 0)) {
- vfree(p);
- return NULL;
- }
-
- return p;
-}
-
#ifdef CONFIG_X86_32
int apply_relocate(Elf32_Shdr *sechdrs,
const char *strtab,
diff --git a/arch/x86/kernel/text.c b/arch/x86/kernel/text.c
new file mode 100644
index 000000000000..986b92ff1434
--- /dev/null
+++ b/arch/x86/kernel/text.c
@@ -0,0 +1,71 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Kernel module help for x86.
+ * Copyright (C) 2001 Rusty Russell.
+ */
+
+#include <linux/kasan.h>
+#include <linux/mm.h>
+#include <linux/moduleloader.h>
+#include <linux/vmalloc.h>
+#include <asm/setup.h>
+
+#ifdef CONFIG_RANDOMIZE_BASE
+static unsigned long module_load_offset;
+
+/* Mutex protects the module_load_offset. */
+static DEFINE_MUTEX(module_kaslr_mutex);
+
+static unsigned long get_module_load_offset(void)
+{
+ if (kaslr_enabled()) {
+ mutex_lock(&module_kaslr_mutex);
+ /*
+ * Calculate the module_load_offset the first time this
+ * code is called. Once calculated it stays the same until
+ * reboot.
+ */
+ if (module_load_offset == 0)
+ module_load_offset =
+ (get_random_int() % 1024 + 1) * PAGE_SIZE;
+ mutex_unlock(&module_kaslr_mutex);
+ }
+ return module_load_offset;
+}
+#else
+static unsigned long get_module_load_offset(void)
+{
+ return 0;
+}
+#endif
+
+void *text_alloc(unsigned long size)
+{
+ void *p;
+
+ if (PAGE_ALIGN(size) > MODULES_LEN)
+ return NULL;
+
+ p = __vmalloc_node_range(size, MODULE_ALIGN,
+ MODULES_VADDR + get_module_load_offset(),
+ MODULES_END, GFP_KERNEL,
+ PAGE_KERNEL, 0, NUMA_NO_NODE,
+ __builtin_return_address(0));
+ if (p && (kasan_module_alloc(p, size) < 0)) {
+ vfree(p);
+ return NULL;
+ }
+
+ return p;
+}
+
+void text_free(void *region)
+{
+ /*
+ * This memory may be RO, and freeing RO memory in an interrupt is not
+ * supported by vmalloc.
+ */
+ WARN_ON(in_interrupt());
+
+ vfree(region);
+}
diff --git a/include/linux/text.h b/include/linux/text.h
new file mode 100644
index 000000000000..a27d4a42ecda
--- /dev/null
+++ b/include/linux/text.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef _LINUX_TEXT_H
+#define _LINUX_TEXT_H
+
+/*
+ * An allocator used for allocating modules, core sections and init sections.
+ * Returns NULL on failure.
+ */
+void *text_alloc(unsigned long size);
+
+/*
+ * Free memory returned from text_alloc().
+ */
+void text_free(void *region);
+
+#endif /* _LINUX_TEXT_H */
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 2e97febeef77..fa7687eb0c0e 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -35,6 +35,7 @@
#include <linux/ftrace.h>
#include <linux/cpu.h>
#include <linux/jump_label.h>
+#include <linux/text.h>
#include <asm/sections.h>
#include <asm/cacheflush.h>
@@ -111,12 +112,20 @@ enum kprobe_slot_state {
void __weak *alloc_insn_page(void)
{
+#ifdef CONFIG_ARCH_HAS_TEXT_ALLOC
+ return text_alloc(PAGE_SIZE);
+#else
return module_alloc(PAGE_SIZE);
+#endif
}
void __weak free_insn_page(void *page)
{
+#ifdef CONFIG_ARCH_HAS_TEXT_ALLOC
+ text_free(page);
+#else
module_memfree(page);
+#endif
}
struct kprobe_insn_cache kprobe_insn_slots = {
@@ -1608,6 +1617,7 @@ static int check_kprobe_address_safe(struct kprobe *p,
goto out;
}
+#ifdef CONFIG_MODULES
/*
* If the module freed .init.text, we couldn't insert
* kprobes in there.
@@ -1618,6 +1628,7 @@ static int check_kprobe_address_safe(struct kprobe *p,
*probed_mod = NULL;
ret = -ENOENT;
}
+#endif
}
out:
preempt_enable();
diff --git a/kernel/module.c b/kernel/module.c
index aa183c9ac0a2..8adeb126b02c 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -56,6 +56,7 @@
#include <linux/bsearch.h>
#include <linux/dynamic_debug.h>
#include <linux/audit.h>
+#include <linux/text.h>
#include <uapi/linux/module.h>
#include "module-internal.h"
@@ -2151,7 +2152,12 @@ void __weak module_memfree(void *module_region)
* supported by vmalloc.
*/
WARN_ON(in_interrupt());
+
+#ifdef CONFIG_ARCH_HAS_TEXT_ALLOC
+ text_free(module_region);
+#else
vfree(module_region);
+#endif
}
void __weak module_arch_cleanup(struct module *mod)
@@ -2786,9 +2792,13 @@ static void dynamic_debug_remove(struct module *mod, struct _ddebug *debug)
void * __weak module_alloc(unsigned long size)
{
+#ifdef CONFIG_ARCH_HAS_TEXT_ALLOC
+ return text_alloc(size);
+#else
return __vmalloc_node_range(size, 1, VMALLOC_START, VMALLOC_END,
GFP_KERNEL, PAGE_KERNEL_EXEC, VM_FLUSH_RESET_PERMS,
NUMA_NO_NODE, __builtin_return_address(0));
+#endif
}
bool __weak module_init_section(const char *name)
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 2/3] module: Add lock_modules() and unlock_modules()
2020-07-14 22:32 [PATCH v3 0/3] kprobes: Remove MODULES dependency Jarkko Sakkinen
2020-07-14 22:32 ` [PATCH v3 1/3] kprobes: Add text_alloc() and text_free() Jarkko Sakkinen
@ 2020-07-14 22:32 ` Jarkko Sakkinen
2020-07-15 8:39 ` Masami Hiramatsu
2020-07-14 22:32 ` [PATCH v3 3/3] kprobes: Flag out CONFIG_MODULES dependent code Jarkko Sakkinen
2 siblings, 1 reply; 18+ messages in thread
From: Jarkko Sakkinen @ 2020-07-14 22:32 UTC (permalink / raw)
To: linux-kernel
Cc: x86, Jarkko Sakkinen, Andi Kleen, Masami Hiramatsu, Jessica Yu,
Naveen N. Rao, Anil S Keshavamurthy, David S. Miller,
Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek,
Joe Lawrence, Steven Rostedt, Ingo Molnar,
open list:LIVE PATCHING
Add wrappers to take the modules "big lock" in order to encapsulate
conditional compilation (CONFIG_MODULES) inside the wrapper.
Cc: Andi Kleen <ak@linux.intel.com>
Suggested-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
include/linux/module.h | 15 ++++++++++
kernel/kprobes.c | 4 +--
kernel/livepatch/core.c | 8 ++---
kernel/module.c | 60 ++++++++++++++++++-------------------
kernel/trace/trace_kprobe.c | 4 +--
5 files changed, 53 insertions(+), 38 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index 2e6670860d27..857b84bf9e90 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -902,4 +902,19 @@ static inline bool module_sig_ok(struct module *module)
}
#endif /* CONFIG_MODULE_SIG */
+#ifdef CONFIG_MODULES
+static inline void lock_modules(void)
+{
+ mutex_lock(&module_mutex);
+}
+
+static inline void unlock_modules(void)
+{
+ mutex_unlock(&module_mutex);
+}
+#else
+static inline void lock_modules(void) { }
+static inline void unlock_modules(void) { }
+#endif
+
#endif /* _LINUX_MODULE_H */
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index fa7687eb0c0e..b4f3c24cd2ef 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -573,7 +573,7 @@ static void kprobe_optimizer(struct work_struct *work)
cpus_read_lock();
mutex_lock(&text_mutex);
/* Lock modules while optimizing kprobes */
- mutex_lock(&module_mutex);
+ lock_modules();
/*
* Step 1: Unoptimize kprobes and collect cleaned (unused and disarmed)
@@ -598,7 +598,7 @@ static void kprobe_optimizer(struct work_struct *work)
/* Step 4: Free cleaned kprobes after quiesence period */
do_free_cleaned_kprobes();
- mutex_unlock(&module_mutex);
+ unlock_modules();
mutex_unlock(&text_mutex);
cpus_read_unlock();
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index f76fdb925532..d9d9d4973e6b 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -57,7 +57,7 @@ static void klp_find_object_module(struct klp_object *obj)
if (!klp_is_module(obj))
return;
- mutex_lock(&module_mutex);
+ lock_modules();
/*
* We do not want to block removal of patched modules and therefore
* we do not take a reference here. The patches are removed by
@@ -74,7 +74,7 @@ static void klp_find_object_module(struct klp_object *obj)
if (mod && mod->klp_alive)
obj->mod = mod;
- mutex_unlock(&module_mutex);
+ unlock_modules();
}
static bool klp_initialized(void)
@@ -163,12 +163,12 @@ static int klp_find_object_symbol(const char *objname, const char *name,
.pos = sympos,
};
- mutex_lock(&module_mutex);
+ lock_modules();
if (objname)
module_kallsyms_on_each_symbol(klp_find_callback, &args);
else
kallsyms_on_each_symbol(klp_find_callback, &args);
- mutex_unlock(&module_mutex);
+ unlock_modules();
/*
* Ensure an address was found. If sympos is 0, ensure symbol is unique;
diff --git a/kernel/module.c b/kernel/module.c
index 8adeb126b02c..3fd8686b1637 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -896,7 +896,7 @@ static void module_unload_free(struct module *mod)
{
struct module_use *use, *tmp;
- mutex_lock(&module_mutex);
+ lock_modules();
list_for_each_entry_safe(use, tmp, &mod->target_list, target_list) {
struct module *i = use->target;
pr_debug("%s unusing %s\n", mod->name, i->name);
@@ -905,7 +905,7 @@ static void module_unload_free(struct module *mod)
list_del(&use->target_list);
kfree(use);
}
- mutex_unlock(&module_mutex);
+ unlock_modules();
}
#ifdef CONFIG_MODULE_FORCE_UNLOAD
@@ -1025,7 +1025,7 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
if (ret != 0)
goto out;
- mutex_unlock(&module_mutex);
+ unlock_modules();
/* Final destruction now no one is using it. */
if (mod->exit != NULL)
mod->exit();
@@ -1044,7 +1044,7 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
wake_up_all(&module_wq);
return 0;
out:
- mutex_unlock(&module_mutex);
+ unlock_modules();
return ret;
}
@@ -1449,7 +1449,7 @@ static const struct kernel_symbol *resolve_symbol(struct module *mod,
* in the wait_event_interruptible(), which is harmless.
*/
sched_annotate_sleep();
- mutex_lock(&module_mutex);
+ lock_modules();
sym = find_symbol(name, &owner, &crc,
!(mod->taints & (1 << TAINT_PROPRIETARY_MODULE)), true);
if (!sym)
@@ -1476,7 +1476,7 @@ static const struct kernel_symbol *resolve_symbol(struct module *mod,
/* We must make copy under the lock if we failed to get ref. */
strncpy(ownername, module_name(owner), MODULE_NAME_LEN);
unlock:
- mutex_unlock(&module_mutex);
+ unlock_modules();
return sym;
}
@@ -1731,10 +1731,10 @@ static void del_usage_links(struct module *mod)
#ifdef CONFIG_MODULE_UNLOAD
struct module_use *use;
- mutex_lock(&module_mutex);
+ lock_modules();
list_for_each_entry(use, &mod->target_list, target_list)
sysfs_remove_link(use->target->holders_dir, mod->name);
- mutex_unlock(&module_mutex);
+ unlock_modules();
#endif
}
@@ -1744,14 +1744,14 @@ static int add_usage_links(struct module *mod)
#ifdef CONFIG_MODULE_UNLOAD
struct module_use *use;
- mutex_lock(&module_mutex);
+ lock_modules();
list_for_each_entry(use, &mod->target_list, target_list) {
ret = sysfs_create_link(use->target->holders_dir,
&mod->mkobj.kobj, mod->name);
if (ret)
break;
}
- mutex_unlock(&module_mutex);
+ unlock_modules();
if (ret)
del_usage_links(mod);
#endif
@@ -2177,9 +2177,9 @@ static void free_module(struct module *mod)
/* We leave it in list to prevent duplicate loads, but make sure
* that noone uses it while it's being deconstructed. */
- mutex_lock(&module_mutex);
+ lock_modules();
mod->state = MODULE_STATE_UNFORMED;
- mutex_unlock(&module_mutex);
+ unlock_modules();
/* Remove dynamic debug info */
ddebug_remove_module(mod->name);
@@ -2197,7 +2197,7 @@ static void free_module(struct module *mod)
free_module_elf(mod);
/* Now we can delete it from the lists */
- mutex_lock(&module_mutex);
+ lock_modules();
/* Unlink carefully: kallsyms could be walking list. */
list_del_rcu(&mod->list);
mod_tree_remove(mod);
@@ -2205,7 +2205,7 @@ static void free_module(struct module *mod)
module_bug_cleanup(mod);
/* Wait for RCU-sched synchronizing before releasing mod->list and buglist. */
synchronize_rcu();
- mutex_unlock(&module_mutex);
+ unlock_modules();
/* This may be empty, but that's OK */
module_arch_freeing_init(mod);
@@ -3504,10 +3504,10 @@ static bool finished_loading(const char *name)
* in the wait_event_interruptible(), which is harmless.
*/
sched_annotate_sleep();
- mutex_lock(&module_mutex);
+ lock_modules();
mod = find_module_all(name, strlen(name), true);
ret = !mod || mod->state == MODULE_STATE_LIVE;
- mutex_unlock(&module_mutex);
+ unlock_modules();
return ret;
}
@@ -3619,7 +3619,7 @@ static noinline int do_init_module(struct module *mod)
ftrace_free_mem(mod, mod->init_layout.base, mod->init_layout.base +
mod->init_layout.size);
- mutex_lock(&module_mutex);
+ lock_modules();
/* Drop initial reference. */
module_put(mod);
trim_init_extable(mod);
@@ -3651,7 +3651,7 @@ static noinline int do_init_module(struct module *mod)
if (llist_add(&freeinit->node, &init_free_list))
schedule_work(&init_free_wq);
- mutex_unlock(&module_mutex);
+ unlock_modules();
wake_up_all(&module_wq);
return 0;
@@ -3693,12 +3693,12 @@ static int add_unformed_module(struct module *mod)
mod->state = MODULE_STATE_UNFORMED;
again:
- mutex_lock(&module_mutex);
+ lock_modules();
old = find_module_all(mod->name, strlen(mod->name), true);
if (old != NULL) {
if (old->state != MODULE_STATE_LIVE) {
/* Wait in case it fails to load. */
- mutex_unlock(&module_mutex);
+ unlock_modules();
err = wait_event_interruptible(module_wq,
finished_loading(mod->name));
if (err)
@@ -3714,7 +3714,7 @@ static int add_unformed_module(struct module *mod)
err = 0;
out:
- mutex_unlock(&module_mutex);
+ unlock_modules();
out_unlocked:
return err;
}
@@ -3723,7 +3723,7 @@ static int complete_formation(struct module *mod, struct load_info *info)
{
int err;
- mutex_lock(&module_mutex);
+ lock_modules();
/* Find duplicate symbols (must be called under lock). */
err = verify_exported_symbols(mod);
@@ -3740,12 +3740,12 @@ static int complete_formation(struct module *mod, struct load_info *info)
/* Mark state as coming so strong_try_module_get() ignores us,
* but kallsyms etc. can see us. */
mod->state = MODULE_STATE_COMING;
- mutex_unlock(&module_mutex);
+ unlock_modules();
return 0;
out:
- mutex_unlock(&module_mutex);
+ unlock_modules();
return err;
}
@@ -3943,9 +3943,9 @@ static int load_module(struct load_info *info, const char __user *uargs,
klp_module_going(mod);
bug_cleanup:
/* module_bug_cleanup needs module_mutex protection */
- mutex_lock(&module_mutex);
+ lock_modules();
module_bug_cleanup(mod);
- mutex_unlock(&module_mutex);
+ unlock_modules();
ddebug_cleanup:
ftrace_release_mod(mod);
@@ -3959,14 +3959,14 @@ static int load_module(struct load_info *info, const char __user *uargs,
free_unload:
module_unload_free(mod);
unlink_mod:
- mutex_lock(&module_mutex);
+ lock_modules();
/* Unlink carefully: kallsyms could be walking list. */
list_del_rcu(&mod->list);
mod_tree_remove(mod);
wake_up_all(&module_wq);
/* Wait for RCU-sched synchronizing before releasing mod->list. */
synchronize_rcu();
- mutex_unlock(&module_mutex);
+ unlock_modules();
free_module:
/* Free lock-classes; relies on the preceding sync_rcu() */
lockdep_free_key_range(mod->core_layout.base, mod->core_layout.size);
@@ -4322,7 +4322,7 @@ static char *module_flags(struct module *mod, char *buf)
/* Called by the /proc file system to return a list of modules. */
static void *m_start(struct seq_file *m, loff_t *pos)
{
- mutex_lock(&module_mutex);
+ lock_modules();
return seq_list_start(&modules, *pos);
}
@@ -4333,7 +4333,7 @@ static void *m_next(struct seq_file *m, void *p, loff_t *pos)
static void m_stop(struct seq_file *m, void *p)
{
- mutex_unlock(&module_mutex);
+ unlock_modules();
}
static int m_show(struct seq_file *m, void *p)
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index aefb6065b508..710ec6a6aa8f 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -122,9 +122,9 @@ static nokprobe_inline bool trace_kprobe_module_exist(struct trace_kprobe *tk)
if (!p)
return true;
*p = '\0';
- mutex_lock(&module_mutex);
+ lock_modules();
ret = !!find_module(tk->symbol);
- mutex_unlock(&module_mutex);
+ unlock_modules();
*p = ':';
return ret;
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 3/3] kprobes: Flag out CONFIG_MODULES dependent code
2020-07-14 22:32 [PATCH v3 0/3] kprobes: Remove MODULES dependency Jarkko Sakkinen
2020-07-14 22:32 ` [PATCH v3 1/3] kprobes: Add text_alloc() and text_free() Jarkko Sakkinen
2020-07-14 22:32 ` [PATCH v3 2/3] module: Add lock_modules() and unlock_modules() Jarkko Sakkinen
@ 2020-07-14 22:32 ` Jarkko Sakkinen
2020-07-15 8:35 ` Masami Hiramatsu
2 siblings, 1 reply; 18+ messages in thread
From: Jarkko Sakkinen @ 2020-07-14 22:32 UTC (permalink / raw)
To: linux-kernel
Cc: x86, Jarkko Sakkinen, Andi Kleen, Jessica Yu, Naveen N. Rao,
Anil S Keshavamurthy, David S. Miller, Masami Hiramatsu,
Steven Rostedt, Ingo Molnar
Remove CONFIG_MODULES dependency by flagging out the dependent code. This
allows to use kprobes in a kernel without support for loadable modules,
which could be useful for a test kernel or perhaps an embedded kernel.
Cc: Andi Kleen <ak@linux.intel.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
include/linux/module.h | 14 +++++++-------
kernel/kprobes.c | 7 +++++++
kernel/trace/trace_kprobe.c | 16 +++++++++++++++-
3 files changed, 29 insertions(+), 8 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index 857b84bf9e90..eaa8ad02f75c 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -290,6 +290,13 @@ extern typeof(name) __mod_##type##__##name##_device_table \
struct notifier_block;
+enum module_state {
+ MODULE_STATE_LIVE, /* Normal state. */
+ MODULE_STATE_COMING, /* Full formed, running module_init. */
+ MODULE_STATE_GOING, /* Going away. */
+ MODULE_STATE_UNFORMED, /* Still setting it up. */
+};
+
#ifdef CONFIG_MODULES
extern int modules_disabled; /* for sysctl */
@@ -305,13 +312,6 @@ struct module_use {
struct module *source, *target;
};
-enum module_state {
- MODULE_STATE_LIVE, /* Normal state. */
- MODULE_STATE_COMING, /* Full formed, running module_init. */
- MODULE_STATE_GOING, /* Going away. */
- MODULE_STATE_UNFORMED, /* Still setting it up. */
-};
-
struct mod_tree_node {
struct module *mod;
struct latch_tree_node node;
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index b4f3c24cd2ef..3039df13d34e 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -2225,6 +2225,7 @@ int kprobe_add_area_blacklist(unsigned long start, unsigned long end)
return 0;
}
+#ifdef CONFIG_MODULES
/* Remove all symbols in given area from kprobe blacklist */
static void kprobe_remove_area_blacklist(unsigned long start, unsigned long end)
{
@@ -2242,6 +2243,7 @@ static void kprobe_remove_ksym_blacklist(unsigned long entry)
{
kprobe_remove_area_blacklist(entry, entry + 1);
}
+#endif
int __init __weak arch_populate_kprobe_blacklist(void)
{
@@ -2285,6 +2287,7 @@ static int __init populate_kprobe_blacklist(unsigned long *start,
return ret ? : arch_populate_kprobe_blacklist();
}
+#ifdef CONFIG_MODULES
static void add_module_kprobe_blacklist(struct module *mod)
{
unsigned long start, end;
@@ -2330,6 +2333,10 @@ static void remove_module_kprobe_blacklist(struct module *mod)
kprobe_remove_area_blacklist(start, end);
}
}
+#else
+static void add_module_kprobe_blacklist(struct module *mod) {}
+static void remove_module_kprobe_blacklist(struct module *mod) {}
+#endif /* CONFIG_MODULES */
/* Module notifier call back, checking kprobes on the module */
static int kprobes_module_callback(struct notifier_block *nb,
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 710ec6a6aa8f..7fcd1bf2d96e 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -103,8 +103,9 @@ static nokprobe_inline bool trace_kprobe_has_gone(struct trace_kprobe *tk)
return !!(kprobe_gone(&tk->rp.kp));
}
+#ifdef CONFIG_MODULES
static nokprobe_inline bool trace_kprobe_within_module(struct trace_kprobe *tk,
- struct module *mod)
+ struct module *mod)
{
int len = strlen(mod->name);
const char *name = trace_kprobe_symbol(tk);
@@ -129,6 +130,17 @@ static nokprobe_inline bool trace_kprobe_module_exist(struct trace_kprobe *tk)
return ret;
}
+#else
+static nokprobe_inline bool trace_kprobe_within_module(struct trace_kprobe *tk,
+ struct module *mod)
+{
+ return false;
+}
+static nokprobe_inline bool trace_kprobe_module_exist(struct trace_kprobe *tk)
+{
+ return false;
+}
+#endif
static bool trace_kprobe_is_busy(struct dyn_event *ev)
{
@@ -685,10 +697,12 @@ static int trace_kprobe_module_callback(struct notifier_block *nb,
/* Don't need to check busy - this should have gone. */
__unregister_trace_kprobe(tk);
ret = __register_trace_kprobe(tk);
+#ifdef CONFIG_MODULES
if (ret)
pr_warn("Failed to re-register probe %s on %s: %d\n",
trace_probe_name(&tk->tp),
mod->name, ret);
+#endif
}
}
mutex_unlock(&event_mutex);
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/3] kprobes: Add text_alloc() and text_free()
2020-07-14 22:32 ` [PATCH v3 1/3] kprobes: Add text_alloc() and text_free() Jarkko Sakkinen
@ 2020-07-15 0:59 ` kernel test robot
2020-07-15 8:27 ` Masami Hiramatsu
` (2 subsequent siblings)
3 siblings, 0 replies; 18+ messages in thread
From: kernel test robot @ 2020-07-15 0:59 UTC (permalink / raw)
To: Jarkko Sakkinen, linux-kernel
Cc: kbuild-all, x86, Jarkko Sakkinen, Andi Kleen, Peter Zijlstra,
Thomas Gleixner, Ingo Molnar, Borislav Petkov,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
H. Peter Anvin
[-- Attachment #1: Type: text/plain, Size: 2532 bytes --]
Hi Jarkko,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.8-rc5]
[cannot apply to tip/x86/core tip/perf/core jeyu/modules-next next-20200714]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Jarkko-Sakkinen/kprobes-Remove-MODULES-dependency/20200715-063746
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git e9919e11e219eaa5e8041b7b1a196839143e9125
config: x86_64-fedora-25 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-14) 9.3.0
reproduce (this is a W=1 build):
# save the attached .config to linux build tree
make W=1 ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
arch/x86/kernel/text.c: In function 'get_module_load_offset':
>> arch/x86/kernel/text.c:30:6: error: implicit declaration of function 'get_random_int' [-Werror=implicit-function-declaration]
30 | (get_random_int() % 1024 + 1) * PAGE_SIZE;
| ^~~~~~~~~~~~~~
arch/x86/kernel/text.c: At top level:
arch/x86/kernel/text.c:42:7: warning: no previous prototype for 'text_alloc' [-Wmissing-prototypes]
42 | void *text_alloc(unsigned long size)
| ^~~~~~~~~~
arch/x86/kernel/text.c:62:6: warning: no previous prototype for 'text_free' [-Wmissing-prototypes]
62 | void text_free(void *region)
| ^~~~~~~~~
cc1: some warnings being treated as errors
vim +/get_random_int +30 arch/x86/kernel/text.c
18
19 static unsigned long get_module_load_offset(void)
20 {
21 if (kaslr_enabled()) {
22 mutex_lock(&module_kaslr_mutex);
23 /*
24 * Calculate the module_load_offset the first time this
25 * code is called. Once calculated it stays the same until
26 * reboot.
27 */
28 if (module_load_offset == 0)
29 module_load_offset =
> 30 (get_random_int() % 1024 + 1) * PAGE_SIZE;
31 mutex_unlock(&module_kaslr_mutex);
32 }
33 return module_load_offset;
34 }
35 #else
36 static unsigned long get_module_load_offset(void)
37 {
38 return 0;
39 }
40 #endif
41
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 52107 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/3] kprobes: Add text_alloc() and text_free()
2020-07-14 22:32 ` [PATCH v3 1/3] kprobes: Add text_alloc() and text_free() Jarkko Sakkinen
2020-07-15 0:59 ` kernel test robot
@ 2020-07-15 8:27 ` Masami Hiramatsu
2020-07-15 19:38 ` Kees Cook
2020-07-16 17:32 ` Jarkko Sakkinen
2020-07-15 19:36 ` Kees Cook
2020-07-16 9:02 ` Peter Zijlstra
3 siblings, 2 replies; 18+ messages in thread
From: Masami Hiramatsu @ 2020-07-15 8:27 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-kernel, x86, Andi Kleen, Peter Zijlstra, Thomas Gleixner,
Ingo Molnar, Borislav Petkov,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
H. Peter Anvin, Naveen N. Rao, Anil S Keshavamurthy,
David S. Miller, Masami Hiramatsu, Jessica Yu, Andrew Morton,
Aneesh Kumar K.V, Kees Cook, Will Deacon, Sami Tolvanen,
Alexandre Ghiti, Masahiro Yamada, Peter Collingbourne,
Frederic Weisbecker, Krzysztof Kozlowski, Arnd Bergmann,
Stephen Boyd, Andy Lutomirski, Josh Poimboeuf, Miroslav Benes,
Babu Moger, Omar Sandoval, Nayna Jain, Marco Elver, Brian Gerst,
Jiri Kosina, Joe Lawrence, Mike Rapoport
Hi Jarkko,
On Wed, 15 Jul 2020 01:32:27 +0300
Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote:
> Introduce new API for allocating space for code generaed at run-time
> leveraging from module_alloc() and module_memfree() code. Use this to
> perform memory allocations in the kprobes code in order to loose the
> bound between kprobes and modules subsystem.
>
> Initially, use this API only with arch/x86 and define a new config
> flag CONFIG_ARCH_HAS_TEXT_ALLOC to promote the availability of the
> new API.
This might need to be extended for the text_alloc() flags as we discuss
in previous thread. (e.g. bpf on arm64)
> Cc: Andi Kleen <ak@linux.intel.com>
> Suggested-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> ---
> arch/Kconfig | 2 +-
> arch/x86/Kconfig | 3 ++
> arch/x86/kernel/Makefile | 1 +
> arch/x86/kernel/module.c | 49 ---------------------------
> arch/x86/kernel/text.c | 71 ++++++++++++++++++++++++++++++++++++++++
I think this would better be arch/x86/mm/text_alloc.c (text.c is too
generic, and this only provides memory allocation.)
> include/linux/text.h | 17 ++++++++++
"text_alloc.h" would be better, or puting the prototype in vmalloc.h
will be easier to use.
> kernel/kprobes.c | 11 +++++++
> kernel/module.c | 10 ++++++
> 8 files changed, 114 insertions(+), 50 deletions(-)
> create mode 100644 arch/x86/kernel/text.c
> create mode 100644 include/linux/text.h
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 8cc35dc556c7..e3d6b6868ccb 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -61,7 +61,7 @@ config OPROFILE_NMI_TIMER
>
> config KPROBES
> bool "Kprobes"
> - depends on MODULES
> + depends on MODULES || ARCH_HAS_TEXT_ALLOC
> depends on HAVE_KPROBES
> select KALLSYMS
> help
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 0dea7fdd7a00..a4ee5d1300f6 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -2035,6 +2035,9 @@ config KEXEC_FILE
> config ARCH_HAS_KEXEC_PURGATORY
> def_bool KEXEC_FILE
>
> +config ARCH_HAS_TEXT_ALLOC
> + def_bool y
> +
> config KEXEC_SIG
> bool "Verify kernel signature during kexec_file_load() syscall"
> depends on KEXEC_FILE
> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
> index e77261db2391..2878e4b753a0 100644
> --- a/arch/x86/kernel/Makefile
> +++ b/arch/x86/kernel/Makefile
> @@ -68,6 +68,7 @@ obj-y += tsc.o tsc_msr.o io_delay.o rtc.o
> obj-y += pci-iommu_table.o
> obj-y += resource.o
> obj-y += irqflags.o
> +obj-y += text.o
>
> obj-y += process.o
> obj-y += fpu/
> diff --git a/arch/x86/kernel/module.c b/arch/x86/kernel/module.c
> index 34b153cbd4ac..261df078f127 100644
> --- a/arch/x86/kernel/module.c
> +++ b/arch/x86/kernel/module.c
> @@ -36,55 +36,6 @@ do { \
> } while (0)
> #endif
>
> -#ifdef CONFIG_RANDOMIZE_BASE
> -static unsigned long module_load_offset;
> -
> -/* Mutex protects the module_load_offset. */
> -static DEFINE_MUTEX(module_kaslr_mutex);
> -
> -static unsigned long int get_module_load_offset(void)
> -{
> - if (kaslr_enabled()) {
> - mutex_lock(&module_kaslr_mutex);
> - /*
> - * Calculate the module_load_offset the first time this
> - * code is called. Once calculated it stays the same until
> - * reboot.
> - */
> - if (module_load_offset == 0)
> - module_load_offset =
> - (get_random_int() % 1024 + 1) * PAGE_SIZE;
> - mutex_unlock(&module_kaslr_mutex);
> - }
> - return module_load_offset;
> -}
> -#else
> -static unsigned long int get_module_load_offset(void)
> -{
> - return 0;
> -}
> -#endif
> -
> -void *module_alloc(unsigned long size)
> -{
> - void *p;
> -
> - if (PAGE_ALIGN(size) > MODULES_LEN)
> - return NULL;
> -
> - p = __vmalloc_node_range(size, MODULE_ALIGN,
> - MODULES_VADDR + get_module_load_offset(),
> - MODULES_END, GFP_KERNEL,
> - PAGE_KERNEL, 0, NUMA_NO_NODE,
> - __builtin_return_address(0));
> - if (p && (kasan_module_alloc(p, size) < 0)) {
> - vfree(p);
> - return NULL;
> - }
> -
> - return p;
> -}
Please don't touch this module_alloc() at all. Then we can
just call __vmalloc_node_range() in the text_alloc().
> -
> #ifdef CONFIG_X86_32
> int apply_relocate(Elf32_Shdr *sechdrs,
> const char *strtab,
> diff --git a/arch/x86/kernel/text.c b/arch/x86/kernel/text.c
> new file mode 100644
> index 000000000000..986b92ff1434
> --- /dev/null
> +++ b/arch/x86/kernel/text.c
> @@ -0,0 +1,71 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Kernel module help for x86.
> + * Copyright (C) 2001 Rusty Russell.
> + */
> +
> +#include <linux/kasan.h>
> +#include <linux/mm.h>
> +#include <linux/moduleloader.h>
> +#include <linux/vmalloc.h>
> +#include <asm/setup.h>
> +
> +#ifdef CONFIG_RANDOMIZE_BASE
> +static unsigned long module_load_offset;
> +
> +/* Mutex protects the module_load_offset. */
> +static DEFINE_MUTEX(module_kaslr_mutex);
> +
> +static unsigned long get_module_load_offset(void)
> +{
> + if (kaslr_enabled()) {
> + mutex_lock(&module_kaslr_mutex);
> + /*
> + * Calculate the module_load_offset the first time this
> + * code is called. Once calculated it stays the same until
> + * reboot.
> + */
> + if (module_load_offset == 0)
> + module_load_offset =
> + (get_random_int() % 1024 + 1) * PAGE_SIZE;
> + mutex_unlock(&module_kaslr_mutex);
> + }
> + return module_load_offset;
> +}
> +#else
> +static unsigned long get_module_load_offset(void)
> +{
> + return 0;
> +}
> +#endif
So as I pointed, we can ignore this for kprobes (and other
dynamic allocated trampoline code).
> +
> +void *text_alloc(unsigned long size)
> +{
> + void *p;
> +
> + if (PAGE_ALIGN(size) > MODULES_LEN)
> + return NULL;
> +
> + p = __vmalloc_node_range(size, MODULE_ALIGN,
> + MODULES_VADDR + get_module_load_offset(),
> + MODULES_END, GFP_KERNEL,
> + PAGE_KERNEL, 0, NUMA_NO_NODE,
> + __builtin_return_address(0));
> + if (p && (kasan_module_alloc(p, size) < 0)) {
> + vfree(p);
> + return NULL;
> + }
> +
> + return p;
> +}
> +
> +void text_free(void *region)
> +{
> + /*
> + * This memory may be RO, and freeing RO memory in an interrupt is not
> + * supported by vmalloc.
> + */
> + WARN_ON(in_interrupt());
> +
> + vfree(region);
> +}
> diff --git a/include/linux/text.h b/include/linux/text.h
> new file mode 100644
> index 000000000000..a27d4a42ecda
> --- /dev/null
> +++ b/include/linux/text.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef _LINUX_TEXT_H
> +#define _LINUX_TEXT_H
> +
> +/*
> + * An allocator used for allocating modules, core sections and init sections.
> + * Returns NULL on failure.
> + */
> +void *text_alloc(unsigned long size);
> +
> +/*
> + * Free memory returned from text_alloc().
> + */
> +void text_free(void *region);
Hmm, if this is this short, in this version we might better move
these to vmalloc.h.
> +
> +#endif /* _LINUX_TEXT_H */
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index 2e97febeef77..fa7687eb0c0e 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -35,6 +35,7 @@
> #include <linux/ftrace.h>
> #include <linux/cpu.h>
> #include <linux/jump_label.h>
> +#include <linux/text.h>
>
> #include <asm/sections.h>
> #include <asm/cacheflush.h>
> @@ -111,12 +112,20 @@ enum kprobe_slot_state {
>
> void __weak *alloc_insn_page(void)
> {
> +#ifdef CONFIG_ARCH_HAS_TEXT_ALLOC
> + return text_alloc(PAGE_SIZE);
> +#else
> return module_alloc(PAGE_SIZE);
> +#endif
> }
>
> void __weak free_insn_page(void *page)
> {
> +#ifdef CONFIG_ARCH_HAS_TEXT_ALLOC
> + text_free(page);
> +#else
> module_memfree(page);
> +#endif
> }
>
> struct kprobe_insn_cache kprobe_insn_slots = {
> @@ -1608,6 +1617,7 @@ static int check_kprobe_address_safe(struct kprobe *p,
> goto out;
> }
>
> +#ifdef CONFIG_MODULES
> /*
> * If the module freed .init.text, we couldn't insert
> * kprobes in there.
> @@ -1618,6 +1628,7 @@ static int check_kprobe_address_safe(struct kprobe *p,
> *probed_mod = NULL;
> ret = -ENOENT;
> }
> +#endif
This change is not related to text_alloc(). Please move this to 3/3.
> }
> out:
> preempt_enable();
> diff --git a/kernel/module.c b/kernel/module.c
> index aa183c9ac0a2..8adeb126b02c 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -56,6 +56,7 @@
> #include <linux/bsearch.h>
> #include <linux/dynamic_debug.h>
> #include <linux/audit.h>
> +#include <linux/text.h>
> #include <uapi/linux/module.h>
> #include "module-internal.h"
>
> @@ -2151,7 +2152,12 @@ void __weak module_memfree(void *module_region)
> * supported by vmalloc.
> */
> WARN_ON(in_interrupt());
> +
> +#ifdef CONFIG_ARCH_HAS_TEXT_ALLOC
> + text_free(module_region);
> +#else
> vfree(module_region);
> +#endif
> }
>
> void __weak module_arch_cleanup(struct module *mod)
> @@ -2786,9 +2792,13 @@ static void dynamic_debug_remove(struct module *mod, struct _ddebug *debug)
>
> void * __weak module_alloc(unsigned long size)
> {
> +#ifdef CONFIG_ARCH_HAS_TEXT_ALLOC
> + return text_alloc(size);
> +#else
> return __vmalloc_node_range(size, 1, VMALLOC_START, VMALLOC_END,
> GFP_KERNEL, PAGE_KERNEL_EXEC, VM_FLUSH_RESET_PERMS,
> NUMA_NO_NODE, __builtin_return_address(0));
> +#endif
> }
Please don't touch kernel/module.c too. This seems to make things complicated.
Thank you,
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 3/3] kprobes: Flag out CONFIG_MODULES dependent code
2020-07-14 22:32 ` [PATCH v3 3/3] kprobes: Flag out CONFIG_MODULES dependent code Jarkko Sakkinen
@ 2020-07-15 8:35 ` Masami Hiramatsu
2020-07-16 17:36 ` Jarkko Sakkinen
0 siblings, 1 reply; 18+ messages in thread
From: Masami Hiramatsu @ 2020-07-15 8:35 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-kernel, x86, Andi Kleen, Jessica Yu, Naveen N. Rao,
Anil S Keshavamurthy, David S. Miller, Masami Hiramatsu,
Steven Rostedt, Ingo Molnar
Hi Jarkko,
On Wed, 15 Jul 2020 01:32:29 +0300
Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote:
> Remove CONFIG_MODULES dependency by flagging out the dependent code. This
> allows to use kprobes in a kernel without support for loadable modules,
> which could be useful for a test kernel or perhaps an embedded kernel.
>
OK, looks good, I just have 2 comments below.
> Cc: Andi Kleen <ak@linux.intel.com>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> ---
> include/linux/module.h | 14 +++++++-------
> kernel/kprobes.c | 7 +++++++
> kernel/trace/trace_kprobe.c | 16 +++++++++++++++-
> 3 files changed, 29 insertions(+), 8 deletions(-)
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 857b84bf9e90..eaa8ad02f75c 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -290,6 +290,13 @@ extern typeof(name) __mod_##type##__##name##_device_table \
>
> struct notifier_block;
>
> +enum module_state {
> + MODULE_STATE_LIVE, /* Normal state. */
> + MODULE_STATE_COMING, /* Full formed, running module_init. */
> + MODULE_STATE_GOING, /* Going away. */
> + MODULE_STATE_UNFORMED, /* Still setting it up. */
> +};
> +
> #ifdef CONFIG_MODULES
>
> extern int modules_disabled; /* for sysctl */
> @@ -305,13 +312,6 @@ struct module_use {
> struct module *source, *target;
> };
>
> -enum module_state {
> - MODULE_STATE_LIVE, /* Normal state. */
> - MODULE_STATE_COMING, /* Full formed, running module_init. */
> - MODULE_STATE_GOING, /* Going away. */
> - MODULE_STATE_UNFORMED, /* Still setting it up. */
> -};
> -
> struct mod_tree_node {
> struct module *mod;
> struct latch_tree_node node;
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index b4f3c24cd2ef..3039df13d34e 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -2225,6 +2225,7 @@ int kprobe_add_area_blacklist(unsigned long start, unsigned long end)
> return 0;
> }
>
> +#ifdef CONFIG_MODULES
> /* Remove all symbols in given area from kprobe blacklist */
> static void kprobe_remove_area_blacklist(unsigned long start, unsigned long end)
> {
> @@ -2242,6 +2243,7 @@ static void kprobe_remove_ksym_blacklist(unsigned long entry)
> {
> kprobe_remove_area_blacklist(entry, entry + 1);
> }
> +#endif
>
> int __init __weak arch_populate_kprobe_blacklist(void)
> {
> @@ -2285,6 +2287,7 @@ static int __init populate_kprobe_blacklist(unsigned long *start,
> return ret ? : arch_populate_kprobe_blacklist();
> }
>
> +#ifdef CONFIG_MODULES
> static void add_module_kprobe_blacklist(struct module *mod)
> {
> unsigned long start, end;
> @@ -2330,6 +2333,10 @@ static void remove_module_kprobe_blacklist(struct module *mod)
> kprobe_remove_area_blacklist(start, end);
> }
> }
> +#else
> +static void add_module_kprobe_blacklist(struct module *mod) {}
> +static void remove_module_kprobe_blacklist(struct module *mod) {}
> +#endif /* CONFIG_MODULES */
Please feel free to move the function. I would like to see;
#ifdef CONFIG_MODULES
/* Remove all symbols in given area from kprobe blacklist */
static void kprobe_remove_area_blacklist(unsigned long start, unsigned long end)
{
...
}
static void add_module_kprobe_blacklist(struct module *mod)
{
...
}
#else
static void add_module_kprobe_blacklist(struct module *mod) {}
static void remove_module_kprobe_blacklist(struct module *mod) {}
#endif /* CONFIG_MODULES */
Rather than split #ifdefs.
>
> /* Module notifier call back, checking kprobes on the module */
> static int kprobes_module_callback(struct notifier_block *nb,
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 710ec6a6aa8f..7fcd1bf2d96e 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -103,8 +103,9 @@ static nokprobe_inline bool trace_kprobe_has_gone(struct trace_kprobe *tk)
> return !!(kprobe_gone(&tk->rp.kp));
> }
>
> +#ifdef CONFIG_MODULES
> static nokprobe_inline bool trace_kprobe_within_module(struct trace_kprobe *tk,
> - struct module *mod)
> + struct module *mod)
> {
> int len = strlen(mod->name);
> const char *name = trace_kprobe_symbol(tk);
> @@ -129,6 +130,17 @@ static nokprobe_inline bool trace_kprobe_module_exist(struct trace_kprobe *tk)
>
> return ret;
> }
> +#else
> +static nokprobe_inline bool trace_kprobe_within_module(struct trace_kprobe *tk,
> + struct module *mod)
> +{
> + return false;
> +}
> +static nokprobe_inline bool trace_kprobe_module_exist(struct trace_kprobe *tk)
> +{
> + return false;
> +}
> +#endif
>
> static bool trace_kprobe_is_busy(struct dyn_event *ev)
> {
> @@ -685,10 +697,12 @@ static int trace_kprobe_module_callback(struct notifier_block *nb,
> /* Don't need to check busy - this should have gone. */
> __unregister_trace_kprobe(tk);
> ret = __register_trace_kprobe(tk);
> +#ifdef CONFIG_MODULES
> if (ret)
> pr_warn("Failed to re-register probe %s on %s: %d\n",
> trace_probe_name(&tk->tp),
> mod->name, ret);
> +#endif
I guess this CONFIG_MODULES is for avoiding build error according to mod->name,
if so, please use module_name(mod) macro instead of this #ifdef.
> }
> }
> mutex_unlock(&event_mutex);
> --
> 2.25.1
>
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 2/3] module: Add lock_modules() and unlock_modules()
2020-07-14 22:32 ` [PATCH v3 2/3] module: Add lock_modules() and unlock_modules() Jarkko Sakkinen
@ 2020-07-15 8:39 ` Masami Hiramatsu
2020-07-16 17:37 ` Jarkko Sakkinen
0 siblings, 1 reply; 18+ messages in thread
From: Masami Hiramatsu @ 2020-07-15 8:39 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-kernel, x86, Andi Kleen, Masami Hiramatsu, Jessica Yu,
Naveen N. Rao, Anil S Keshavamurthy, David S. Miller,
Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek,
Joe Lawrence, Steven Rostedt, Ingo Molnar,
open list:LIVE PATCHING
On Wed, 15 Jul 2020 01:32:28 +0300
Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote:
> Add wrappers to take the modules "big lock" in order to encapsulate
> conditional compilation (CONFIG_MODULES) inside the wrapper.
>
> Cc: Andi Kleen <ak@linux.intel.com>
> Suggested-by: Masami Hiramatsu <mhiramat@kernel.org>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> ---
> include/linux/module.h | 15 ++++++++++
> kernel/kprobes.c | 4 +--
> kernel/livepatch/core.c | 8 ++---
> kernel/module.c | 60 ++++++++++++++++++-------------------
> kernel/trace/trace_kprobe.c | 4 +--
> 5 files changed, 53 insertions(+), 38 deletions(-)
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 2e6670860d27..857b84bf9e90 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -902,4 +902,19 @@ static inline bool module_sig_ok(struct module *module)
> }
> #endif /* CONFIG_MODULE_SIG */
>
> +#ifdef CONFIG_MODULES
> +static inline void lock_modules(void)
> +{
> + mutex_lock(&module_mutex);
> +}
> +
> +static inline void unlock_modules(void)
> +{
> + mutex_unlock(&module_mutex);
> +}
> +#else
> +static inline void lock_modules(void) { }
> +static inline void unlock_modules(void) { }
> +#endif
You don't need to add new #ifdefs. There is a room for dummy prototypes
for !CONFIG_MODULES already in modules.h.
-----
struct notifier_block;
#ifdef CONFIG_MODULES
extern int modules_disabled; /* for sysctl */
...
#else /* !CONFIG_MODULES... */
static inline struct module *__module_address(unsigned long addr)
{
-----
So you can just put those inlines in the appropriate places ;)
Thank you,
> +
> #endif /* _LINUX_MODULE_H */
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index fa7687eb0c0e..b4f3c24cd2ef 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -573,7 +573,7 @@ static void kprobe_optimizer(struct work_struct *work)
> cpus_read_lock();
> mutex_lock(&text_mutex);
> /* Lock modules while optimizing kprobes */
> - mutex_lock(&module_mutex);
> + lock_modules();
>
> /*
> * Step 1: Unoptimize kprobes and collect cleaned (unused and disarmed)
> @@ -598,7 +598,7 @@ static void kprobe_optimizer(struct work_struct *work)
> /* Step 4: Free cleaned kprobes after quiesence period */
> do_free_cleaned_kprobes();
>
> - mutex_unlock(&module_mutex);
> + unlock_modules();
> mutex_unlock(&text_mutex);
> cpus_read_unlock();
>
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index f76fdb925532..d9d9d4973e6b 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -57,7 +57,7 @@ static void klp_find_object_module(struct klp_object *obj)
> if (!klp_is_module(obj))
> return;
>
> - mutex_lock(&module_mutex);
> + lock_modules();
> /*
> * We do not want to block removal of patched modules and therefore
> * we do not take a reference here. The patches are removed by
> @@ -74,7 +74,7 @@ static void klp_find_object_module(struct klp_object *obj)
> if (mod && mod->klp_alive)
> obj->mod = mod;
>
> - mutex_unlock(&module_mutex);
> + unlock_modules();
> }
>
> static bool klp_initialized(void)
> @@ -163,12 +163,12 @@ static int klp_find_object_symbol(const char *objname, const char *name,
> .pos = sympos,
> };
>
> - mutex_lock(&module_mutex);
> + lock_modules();
> if (objname)
> module_kallsyms_on_each_symbol(klp_find_callback, &args);
> else
> kallsyms_on_each_symbol(klp_find_callback, &args);
> - mutex_unlock(&module_mutex);
> + unlock_modules();
>
> /*
> * Ensure an address was found. If sympos is 0, ensure symbol is unique;
> diff --git a/kernel/module.c b/kernel/module.c
> index 8adeb126b02c..3fd8686b1637 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -896,7 +896,7 @@ static void module_unload_free(struct module *mod)
> {
> struct module_use *use, *tmp;
>
> - mutex_lock(&module_mutex);
> + lock_modules();
> list_for_each_entry_safe(use, tmp, &mod->target_list, target_list) {
> struct module *i = use->target;
> pr_debug("%s unusing %s\n", mod->name, i->name);
> @@ -905,7 +905,7 @@ static void module_unload_free(struct module *mod)
> list_del(&use->target_list);
> kfree(use);
> }
> - mutex_unlock(&module_mutex);
> + unlock_modules();
> }
>
> #ifdef CONFIG_MODULE_FORCE_UNLOAD
> @@ -1025,7 +1025,7 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
> if (ret != 0)
> goto out;
>
> - mutex_unlock(&module_mutex);
> + unlock_modules();
> /* Final destruction now no one is using it. */
> if (mod->exit != NULL)
> mod->exit();
> @@ -1044,7 +1044,7 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
> wake_up_all(&module_wq);
> return 0;
> out:
> - mutex_unlock(&module_mutex);
> + unlock_modules();
> return ret;
> }
>
> @@ -1449,7 +1449,7 @@ static const struct kernel_symbol *resolve_symbol(struct module *mod,
> * in the wait_event_interruptible(), which is harmless.
> */
> sched_annotate_sleep();
> - mutex_lock(&module_mutex);
> + lock_modules();
> sym = find_symbol(name, &owner, &crc,
> !(mod->taints & (1 << TAINT_PROPRIETARY_MODULE)), true);
> if (!sym)
> @@ -1476,7 +1476,7 @@ static const struct kernel_symbol *resolve_symbol(struct module *mod,
> /* We must make copy under the lock if we failed to get ref. */
> strncpy(ownername, module_name(owner), MODULE_NAME_LEN);
> unlock:
> - mutex_unlock(&module_mutex);
> + unlock_modules();
> return sym;
> }
>
> @@ -1731,10 +1731,10 @@ static void del_usage_links(struct module *mod)
> #ifdef CONFIG_MODULE_UNLOAD
> struct module_use *use;
>
> - mutex_lock(&module_mutex);
> + lock_modules();
> list_for_each_entry(use, &mod->target_list, target_list)
> sysfs_remove_link(use->target->holders_dir, mod->name);
> - mutex_unlock(&module_mutex);
> + unlock_modules();
> #endif
> }
>
> @@ -1744,14 +1744,14 @@ static int add_usage_links(struct module *mod)
> #ifdef CONFIG_MODULE_UNLOAD
> struct module_use *use;
>
> - mutex_lock(&module_mutex);
> + lock_modules();
> list_for_each_entry(use, &mod->target_list, target_list) {
> ret = sysfs_create_link(use->target->holders_dir,
> &mod->mkobj.kobj, mod->name);
> if (ret)
> break;
> }
> - mutex_unlock(&module_mutex);
> + unlock_modules();
> if (ret)
> del_usage_links(mod);
> #endif
> @@ -2177,9 +2177,9 @@ static void free_module(struct module *mod)
>
> /* We leave it in list to prevent duplicate loads, but make sure
> * that noone uses it while it's being deconstructed. */
> - mutex_lock(&module_mutex);
> + lock_modules();
> mod->state = MODULE_STATE_UNFORMED;
> - mutex_unlock(&module_mutex);
> + unlock_modules();
>
> /* Remove dynamic debug info */
> ddebug_remove_module(mod->name);
> @@ -2197,7 +2197,7 @@ static void free_module(struct module *mod)
> free_module_elf(mod);
>
> /* Now we can delete it from the lists */
> - mutex_lock(&module_mutex);
> + lock_modules();
> /* Unlink carefully: kallsyms could be walking list. */
> list_del_rcu(&mod->list);
> mod_tree_remove(mod);
> @@ -2205,7 +2205,7 @@ static void free_module(struct module *mod)
> module_bug_cleanup(mod);
> /* Wait for RCU-sched synchronizing before releasing mod->list and buglist. */
> synchronize_rcu();
> - mutex_unlock(&module_mutex);
> + unlock_modules();
>
> /* This may be empty, but that's OK */
> module_arch_freeing_init(mod);
> @@ -3504,10 +3504,10 @@ static bool finished_loading(const char *name)
> * in the wait_event_interruptible(), which is harmless.
> */
> sched_annotate_sleep();
> - mutex_lock(&module_mutex);
> + lock_modules();
> mod = find_module_all(name, strlen(name), true);
> ret = !mod || mod->state == MODULE_STATE_LIVE;
> - mutex_unlock(&module_mutex);
> + unlock_modules();
>
> return ret;
> }
> @@ -3619,7 +3619,7 @@ static noinline int do_init_module(struct module *mod)
>
> ftrace_free_mem(mod, mod->init_layout.base, mod->init_layout.base +
> mod->init_layout.size);
> - mutex_lock(&module_mutex);
> + lock_modules();
> /* Drop initial reference. */
> module_put(mod);
> trim_init_extable(mod);
> @@ -3651,7 +3651,7 @@ static noinline int do_init_module(struct module *mod)
> if (llist_add(&freeinit->node, &init_free_list))
> schedule_work(&init_free_wq);
>
> - mutex_unlock(&module_mutex);
> + unlock_modules();
> wake_up_all(&module_wq);
>
> return 0;
> @@ -3693,12 +3693,12 @@ static int add_unformed_module(struct module *mod)
> mod->state = MODULE_STATE_UNFORMED;
>
> again:
> - mutex_lock(&module_mutex);
> + lock_modules();
> old = find_module_all(mod->name, strlen(mod->name), true);
> if (old != NULL) {
> if (old->state != MODULE_STATE_LIVE) {
> /* Wait in case it fails to load. */
> - mutex_unlock(&module_mutex);
> + unlock_modules();
> err = wait_event_interruptible(module_wq,
> finished_loading(mod->name));
> if (err)
> @@ -3714,7 +3714,7 @@ static int add_unformed_module(struct module *mod)
> err = 0;
>
> out:
> - mutex_unlock(&module_mutex);
> + unlock_modules();
> out_unlocked:
> return err;
> }
> @@ -3723,7 +3723,7 @@ static int complete_formation(struct module *mod, struct load_info *info)
> {
> int err;
>
> - mutex_lock(&module_mutex);
> + lock_modules();
>
> /* Find duplicate symbols (must be called under lock). */
> err = verify_exported_symbols(mod);
> @@ -3740,12 +3740,12 @@ static int complete_formation(struct module *mod, struct load_info *info)
> /* Mark state as coming so strong_try_module_get() ignores us,
> * but kallsyms etc. can see us. */
> mod->state = MODULE_STATE_COMING;
> - mutex_unlock(&module_mutex);
> + unlock_modules();
>
> return 0;
>
> out:
> - mutex_unlock(&module_mutex);
> + unlock_modules();
> return err;
> }
>
> @@ -3943,9 +3943,9 @@ static int load_module(struct load_info *info, const char __user *uargs,
> klp_module_going(mod);
> bug_cleanup:
> /* module_bug_cleanup needs module_mutex protection */
> - mutex_lock(&module_mutex);
> + lock_modules();
> module_bug_cleanup(mod);
> - mutex_unlock(&module_mutex);
> + unlock_modules();
>
> ddebug_cleanup:
> ftrace_release_mod(mod);
> @@ -3959,14 +3959,14 @@ static int load_module(struct load_info *info, const char __user *uargs,
> free_unload:
> module_unload_free(mod);
> unlink_mod:
> - mutex_lock(&module_mutex);
> + lock_modules();
> /* Unlink carefully: kallsyms could be walking list. */
> list_del_rcu(&mod->list);
> mod_tree_remove(mod);
> wake_up_all(&module_wq);
> /* Wait for RCU-sched synchronizing before releasing mod->list. */
> synchronize_rcu();
> - mutex_unlock(&module_mutex);
> + unlock_modules();
> free_module:
> /* Free lock-classes; relies on the preceding sync_rcu() */
> lockdep_free_key_range(mod->core_layout.base, mod->core_layout.size);
> @@ -4322,7 +4322,7 @@ static char *module_flags(struct module *mod, char *buf)
> /* Called by the /proc file system to return a list of modules. */
> static void *m_start(struct seq_file *m, loff_t *pos)
> {
> - mutex_lock(&module_mutex);
> + lock_modules();
> return seq_list_start(&modules, *pos);
> }
>
> @@ -4333,7 +4333,7 @@ static void *m_next(struct seq_file *m, void *p, loff_t *pos)
>
> static void m_stop(struct seq_file *m, void *p)
> {
> - mutex_unlock(&module_mutex);
> + unlock_modules();
> }
>
> static int m_show(struct seq_file *m, void *p)
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index aefb6065b508..710ec6a6aa8f 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -122,9 +122,9 @@ static nokprobe_inline bool trace_kprobe_module_exist(struct trace_kprobe *tk)
> if (!p)
> return true;
> *p = '\0';
> - mutex_lock(&module_mutex);
> + lock_modules();
> ret = !!find_module(tk->symbol);
> - mutex_unlock(&module_mutex);
> + unlock_modules();
> *p = ':';
>
> return ret;
> --
> 2.25.1
>
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/3] kprobes: Add text_alloc() and text_free()
2020-07-14 22:32 ` [PATCH v3 1/3] kprobes: Add text_alloc() and text_free() Jarkko Sakkinen
2020-07-15 0:59 ` kernel test robot
2020-07-15 8:27 ` Masami Hiramatsu
@ 2020-07-15 19:36 ` Kees Cook
2020-07-15 19:49 ` Mike Rapoport
2020-07-23 1:39 ` Jarkko Sakkinen
2020-07-16 9:02 ` Peter Zijlstra
3 siblings, 2 replies; 18+ messages in thread
From: Kees Cook @ 2020-07-15 19:36 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-kernel, x86, Andi Kleen, Peter Zijlstra, Thomas Gleixner,
Ingo Molnar, Borislav Petkov,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
H. Peter Anvin, Naveen N. Rao, Anil S Keshavamurthy,
David S. Miller, Masami Hiramatsu, Jessica Yu, Andrew Morton,
Aneesh Kumar K.V, Will Deacon, Sami Tolvanen, Alexandre Ghiti,
Masahiro Yamada, Peter Collingbourne, Frederic Weisbecker,
Krzysztof Kozlowski, Arnd Bergmann, Stephen Boyd,
Andy Lutomirski, Josh Poimboeuf, Miroslav Benes, Babu Moger,
Omar Sandoval, Nayna Jain, Marco Elver, Brian Gerst, Jiri Kosina,
Joe Lawrence, Mike Rapoport
On Wed, Jul 15, 2020 at 01:32:27AM +0300, Jarkko Sakkinen wrote:
> Introduce new API for allocating space for code generaed at run-time
> leveraging from module_alloc() and module_memfree() code. Use this to
> perform memory allocations in the kprobes code in order to loose the
> bound between kprobes and modules subsystem.
>
> Initially, use this API only with arch/x86 and define a new config
> flag CONFIG_ARCH_HAS_TEXT_ALLOC to promote the availability of the
> new API.
> [...]
> diff --git a/include/linux/text.h b/include/linux/text.h
> new file mode 100644
> index 000000000000..a27d4a42ecda
> --- /dev/null
> +++ b/include/linux/text.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef _LINUX_TEXT_H
> +#define _LINUX_TEXT_H
> +
> +/*
> + * An allocator used for allocating modules, core sections and init sections.
> + * Returns NULL on failure.
> + */
> +void *text_alloc(unsigned long size);
> +
> +/*
> + * Free memory returned from text_alloc().
> + */
> +void text_free(void *region);
> +
> +#endif /* _LINUX_TEXT_H */
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index 2e97febeef77..fa7687eb0c0e 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -35,6 +35,7 @@
> #include <linux/ftrace.h>
> #include <linux/cpu.h>
> #include <linux/jump_label.h>
> +#include <linux/text.h>
>
> #include <asm/sections.h>
> #include <asm/cacheflush.h>
> @@ -111,12 +112,20 @@ enum kprobe_slot_state {
>
> void __weak *alloc_insn_page(void)
> {
> +#ifdef CONFIG_ARCH_HAS_TEXT_ALLOC
> + return text_alloc(PAGE_SIZE);
> +#else
> return module_alloc(PAGE_SIZE);
> +#endif
This seems like it shouldn't be needed. I think text_alloc() should
always be visible. In the ARCH_HAS... case it will call the arch
implementation, and without it should just call module_alloc():
For example:
void *text_alloc(unsigned long size)
{
#ifdef CONFIG_ARCH_HAS_TEXT_ALLOC
return arch_text_alloc(size);
#else
return module_alloc(size);
#endif
}
--
Kees Cook
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/3] kprobes: Add text_alloc() and text_free()
2020-07-15 8:27 ` Masami Hiramatsu
@ 2020-07-15 19:38 ` Kees Cook
2020-07-16 17:32 ` Jarkko Sakkinen
1 sibling, 0 replies; 18+ messages in thread
From: Kees Cook @ 2020-07-15 19:38 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Jarkko Sakkinen, linux-kernel, x86, Andi Kleen, Peter Zijlstra,
Thomas Gleixner, Ingo Molnar, Borislav Petkov,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
H. Peter Anvin, Naveen N. Rao, Anil S Keshavamurthy,
David S. Miller, Jessica Yu, Andrew Morton, Aneesh Kumar K.V,
Will Deacon, Sami Tolvanen, Alexandre Ghiti, Masahiro Yamada,
Peter Collingbourne, Frederic Weisbecker, Krzysztof Kozlowski,
Arnd Bergmann, Stephen Boyd, Andy Lutomirski, Josh Poimboeuf,
Miroslav Benes, Babu Moger, Omar Sandoval, Nayna Jain,
Marco Elver, Brian Gerst, Jiri Kosina, Joe Lawrence,
Mike Rapoport
On Wed, Jul 15, 2020 at 05:27:32PM +0900, Masami Hiramatsu wrote:
>
> On Wed, 15 Jul 2020 01:32:27 +0300
> Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote:
> > [...]
> > -void *module_alloc(unsigned long size)
> > -{
> > - void *p;
> > -
> > - if (PAGE_ALIGN(size) > MODULES_LEN)
> > - return NULL;
> > -
> > - p = __vmalloc_node_range(size, MODULE_ALIGN,
> > - MODULES_VADDR + get_module_load_offset(),
> > - MODULES_END, GFP_KERNEL,
> > - PAGE_KERNEL, 0, NUMA_NO_NODE,
> > - __builtin_return_address(0));
> > - if (p && (kasan_module_alloc(p, size) < 0)) {
> > - vfree(p);
> > - return NULL;
> > - }
> > -
> > - return p;
> > -}
>
> Please don't touch this module_alloc() at all. Then we can
> just call __vmalloc_node_range() in the text_alloc().
Hm? I thought the requirement was that trampolines needed to stay within
a certain distance of kernel text and that the module_alloc() enforced
that?
--
Kees Cook
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/3] kprobes: Add text_alloc() and text_free()
2020-07-15 19:36 ` Kees Cook
@ 2020-07-15 19:49 ` Mike Rapoport
2020-07-15 19:51 ` Kees Cook
2020-07-23 1:39 ` Jarkko Sakkinen
1 sibling, 1 reply; 18+ messages in thread
From: Mike Rapoport @ 2020-07-15 19:49 UTC (permalink / raw)
To: Kees Cook
Cc: Jarkko Sakkinen, linux-kernel, x86, Andi Kleen, Peter Zijlstra,
Thomas Gleixner, Ingo Molnar, Borislav Petkov,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
H. Peter Anvin, Naveen N. Rao, Anil S Keshavamurthy,
David S. Miller, Masami Hiramatsu, Jessica Yu, Andrew Morton,
Aneesh Kumar K.V, Will Deacon, Sami Tolvanen, Alexandre Ghiti,
Masahiro Yamada, Peter Collingbourne, Frederic Weisbecker,
Krzysztof Kozlowski, Arnd Bergmann, Stephen Boyd,
Andy Lutomirski, Josh Poimboeuf, Miroslav Benes, Babu Moger,
Omar Sandoval, Nayna Jain, Marco Elver, Brian Gerst, Jiri Kosina,
Joe Lawrence
On Wed, Jul 15, 2020 at 12:36:02PM -0700, Kees Cook wrote:
> On Wed, Jul 15, 2020 at 01:32:27AM +0300, Jarkko Sakkinen wrote:
> > Introduce new API for allocating space for code generaed at run-time
> > leveraging from module_alloc() and module_memfree() code. Use this to
> > perform memory allocations in the kprobes code in order to loose the
> > bound between kprobes and modules subsystem.
> >
> > Initially, use this API only with arch/x86 and define a new config
> > flag CONFIG_ARCH_HAS_TEXT_ALLOC to promote the availability of the
> > new API.
> > [...]
> > diff --git a/include/linux/text.h b/include/linux/text.h
> > new file mode 100644
> > index 000000000000..a27d4a42ecda
> > --- /dev/null
> > +++ b/include/linux/text.h
> > @@ -0,0 +1,17 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +
> > +#ifndef _LINUX_TEXT_H
> > +#define _LINUX_TEXT_H
> > +
> > +/*
> > + * An allocator used for allocating modules, core sections and init sections.
> > + * Returns NULL on failure.
> > + */
> > +void *text_alloc(unsigned long size);
> > +
> > +/*
> > + * Free memory returned from text_alloc().
> > + */
> > +void text_free(void *region);
> > +
> > +#endif /* _LINUX_TEXT_H */
> > diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> > index 2e97febeef77..fa7687eb0c0e 100644
> > --- a/kernel/kprobes.c
> > +++ b/kernel/kprobes.c
> > @@ -35,6 +35,7 @@
> > #include <linux/ftrace.h>
> > #include <linux/cpu.h>
> > #include <linux/jump_label.h>
> > +#include <linux/text.h>
> >
> > #include <asm/sections.h>
> > #include <asm/cacheflush.h>
> > @@ -111,12 +112,20 @@ enum kprobe_slot_state {
> >
> > void __weak *alloc_insn_page(void)
> > {
> > +#ifdef CONFIG_ARCH_HAS_TEXT_ALLOC
> > + return text_alloc(PAGE_SIZE);
> > +#else
> > return module_alloc(PAGE_SIZE);
> > +#endif
>
> This seems like it shouldn't be needed. I think text_alloc() should
> always be visible. In the ARCH_HAS... case it will call the arch
> implementation, and without it should just call module_alloc():
>
> For example:
> void *text_alloc(unsigned long size)
> {
> #ifdef CONFIG_ARCH_HAS_TEXT_ALLOC
> return arch_text_alloc(size);
> #else
> return module_alloc(size);
> #endif
> }
This inverts the dependcy chain, IMHO, module_alloc() is a special case
of text_alloc() and not vice versa.
The addition of #ifdefs to alloc_insn_page() is not needed because it is
anyway overriden on x86, so I don't see a point in added #ifdefs until
another arch will enable ARCH_HAS_TEXT_ALLOC.
> --
> Kees Cook
--
Sincerely yours,
Mike.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/3] kprobes: Add text_alloc() and text_free()
2020-07-15 19:49 ` Mike Rapoport
@ 2020-07-15 19:51 ` Kees Cook
0 siblings, 0 replies; 18+ messages in thread
From: Kees Cook @ 2020-07-15 19:51 UTC (permalink / raw)
To: Mike Rapoport
Cc: Jarkko Sakkinen, linux-kernel, x86, Andi Kleen, Peter Zijlstra,
Thomas Gleixner, Ingo Molnar, Borislav Petkov,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
H. Peter Anvin, Naveen N. Rao, Anil S Keshavamurthy,
David S. Miller, Masami Hiramatsu, Jessica Yu, Andrew Morton,
Aneesh Kumar K.V, Will Deacon, Sami Tolvanen, Alexandre Ghiti,
Masahiro Yamada, Peter Collingbourne, Frederic Weisbecker,
Krzysztof Kozlowski, Arnd Bergmann, Stephen Boyd,
Andy Lutomirski, Josh Poimboeuf, Miroslav Benes, Babu Moger,
Omar Sandoval, Nayna Jain, Marco Elver, Brian Gerst, Jiri Kosina,
Joe Lawrence
On Wed, Jul 15, 2020 at 10:49:33PM +0300, Mike Rapoport wrote:
> On Wed, Jul 15, 2020 at 12:36:02PM -0700, Kees Cook wrote:
> > On Wed, Jul 15, 2020 at 01:32:27AM +0300, Jarkko Sakkinen wrote:
> > > Introduce new API for allocating space for code generaed at run-time
> > > leveraging from module_alloc() and module_memfree() code. Use this to
> > > perform memory allocations in the kprobes code in order to loose the
> > > bound between kprobes and modules subsystem.
> > >
> > > Initially, use this API only with arch/x86 and define a new config
> > > flag CONFIG_ARCH_HAS_TEXT_ALLOC to promote the availability of the
> > > new API.
> > > [...]
> > > diff --git a/include/linux/text.h b/include/linux/text.h
> > > new file mode 100644
> > > index 000000000000..a27d4a42ecda
> > > --- /dev/null
> > > +++ b/include/linux/text.h
> > > @@ -0,0 +1,17 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-only */
> > > +
> > > +#ifndef _LINUX_TEXT_H
> > > +#define _LINUX_TEXT_H
> > > +
> > > +/*
> > > + * An allocator used for allocating modules, core sections and init sections.
> > > + * Returns NULL on failure.
> > > + */
> > > +void *text_alloc(unsigned long size);
> > > +
> > > +/*
> > > + * Free memory returned from text_alloc().
> > > + */
> > > +void text_free(void *region);
> > > +
> > > +#endif /* _LINUX_TEXT_H */
> > > diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> > > index 2e97febeef77..fa7687eb0c0e 100644
> > > --- a/kernel/kprobes.c
> > > +++ b/kernel/kprobes.c
> > > @@ -35,6 +35,7 @@
> > > #include <linux/ftrace.h>
> > > #include <linux/cpu.h>
> > > #include <linux/jump_label.h>
> > > +#include <linux/text.h>
> > >
> > > #include <asm/sections.h>
> > > #include <asm/cacheflush.h>
> > > @@ -111,12 +112,20 @@ enum kprobe_slot_state {
> > >
> > > void __weak *alloc_insn_page(void)
> > > {
> > > +#ifdef CONFIG_ARCH_HAS_TEXT_ALLOC
> > > + return text_alloc(PAGE_SIZE);
> > > +#else
> > > return module_alloc(PAGE_SIZE);
> > > +#endif
> >
> > This seems like it shouldn't be needed. I think text_alloc() should
> > always be visible. In the ARCH_HAS... case it will call the arch
> > implementation, and without it should just call module_alloc():
> >
> > For example:
> > void *text_alloc(unsigned long size)
> > {
> > #ifdef CONFIG_ARCH_HAS_TEXT_ALLOC
> > return arch_text_alloc(size);
> > #else
> > return module_alloc(size);
> > #endif
> > }
>
> This inverts the dependcy chain, IMHO, module_alloc() is a special case
> of text_alloc() and not vice versa.
Okay, sure. That's fine too. Whatever the case is, I want to make sure
we keep the KASLR offset though. I don't think that should be unique to
the modules logic.
--
Kees Cook
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/3] kprobes: Add text_alloc() and text_free()
2020-07-14 22:32 ` [PATCH v3 1/3] kprobes: Add text_alloc() and text_free() Jarkko Sakkinen
` (2 preceding siblings ...)
2020-07-15 19:36 ` Kees Cook
@ 2020-07-16 9:02 ` Peter Zijlstra
2020-07-23 1:49 ` Jarkko Sakkinen
3 siblings, 1 reply; 18+ messages in thread
From: Peter Zijlstra @ 2020-07-16 9:02 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-kernel, x86, Andi Kleen, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
H. Peter Anvin, Naveen N. Rao, Anil S Keshavamurthy,
David S. Miller, Masami Hiramatsu, Jessica Yu, Andrew Morton,
Aneesh Kumar K.V, Kees Cook, Will Deacon, Sami Tolvanen,
Alexandre Ghiti, Masahiro Yamada, Peter Collingbourne,
Frederic Weisbecker, Krzysztof Kozlowski, Arnd Bergmann,
Stephen Boyd, Andy Lutomirski, Josh Poimboeuf, Miroslav Benes,
Babu Moger, Omar Sandoval, Nayna Jain, Marco Elver, Brian Gerst,
Jiri Kosina, Joe Lawrence, Mike Rapoport
On Wed, Jul 15, 2020 at 01:32:27AM +0300, Jarkko Sakkinen wrote:
> +void *text_alloc(unsigned long size)
> +{
> + void *p;
> +
> + if (PAGE_ALIGN(size) > MODULES_LEN)
> + return NULL;
> +
> + p = __vmalloc_node_range(size, MODULE_ALIGN,
> + MODULES_VADDR + get_module_load_offset(),
> + MODULES_END, GFP_KERNEL,
> + PAGE_KERNEL, 0, NUMA_NO_NODE,
> + __builtin_return_address(0));
> + if (p && (kasan_module_alloc(p, size) < 0)) {
> + vfree(p);
> + return NULL;
> + }
> +
> + return p;
> +}
> +
> +void text_free(void *region)
> +{
> + /*
> + * This memory may be RO, and freeing RO memory in an interrupt is not
> + * supported by vmalloc.
> + */
> + WARN_ON(in_interrupt());
I think that wants to be:
lockdep_assert_irqs_enabled();
in_interrupt() isn't sufficient, interrupts must also not be disabled
when issuesing TLB invalidations.
> +
> + vfree(region);
> +}
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/3] kprobes: Add text_alloc() and text_free()
2020-07-15 8:27 ` Masami Hiramatsu
2020-07-15 19:38 ` Kees Cook
@ 2020-07-16 17:32 ` Jarkko Sakkinen
1 sibling, 0 replies; 18+ messages in thread
From: Jarkko Sakkinen @ 2020-07-16 17:32 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: linux-kernel, x86, Andi Kleen, Peter Zijlstra, Thomas Gleixner,
Ingo Molnar, Borislav Petkov,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
H. Peter Anvin, Naveen N. Rao, Anil S Keshavamurthy,
David S. Miller, Jessica Yu, Andrew Morton, Aneesh Kumar K.V,
Kees Cook, Will Deacon, Sami Tolvanen, Alexandre Ghiti,
Masahiro Yamada, Peter Collingbourne, Frederic Weisbecker,
Krzysztof Kozlowski, Arnd Bergmann, Stephen Boyd,
Andy Lutomirski, Josh Poimboeuf, Miroslav Benes, Babu Moger,
Omar Sandoval, Nayna Jain, Marco Elver, Brian Gerst, Jiri Kosina,
Joe Lawrence, Mike Rapoport
On Wed, Jul 15, 2020 at 05:27:32PM +0900, Masami Hiramatsu wrote:
> Hi Jarkko,
>
> On Wed, 15 Jul 2020 01:32:27 +0300
> Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote:
>
> > Introduce new API for allocating space for code generaed at run-time
> > leveraging from module_alloc() and module_memfree() code. Use this to
> > perform memory allocations in the kprobes code in order to loose the
> > bound between kprobes and modules subsystem.
> >
> > Initially, use this API only with arch/x86 and define a new config
> > flag CONFIG_ARCH_HAS_TEXT_ALLOC to promote the availability of the
> > new API.
>
> This might need to be extended for the text_alloc() flags as we discuss
> in previous thread. (e.g. bpf on arm64)
>
> > Cc: Andi Kleen <ak@linux.intel.com>
> > Suggested-by: Peter Zijlstra <peterz@infradead.org>
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > ---
> > arch/Kconfig | 2 +-
> > arch/x86/Kconfig | 3 ++
> > arch/x86/kernel/Makefile | 1 +
> > arch/x86/kernel/module.c | 49 ---------------------------
> > arch/x86/kernel/text.c | 71 ++++++++++++++++++++++++++++++++++++++++
>
> I think this would better be arch/x86/mm/text_alloc.c (text.c is too
> generic, and this only provides memory allocation.)
Agreed.
>
> > include/linux/text.h | 17 ++++++++++
>
> "text_alloc.h" would be better, or puting the prototype in vmalloc.h
> will be easier to use.
>
> > kernel/kprobes.c | 11 +++++++
> > kernel/module.c | 10 ++++++
> > 8 files changed, 114 insertions(+), 50 deletions(-)
> > create mode 100644 arch/x86/kernel/text.c
> > create mode 100644 include/linux/text.h
> >
> > diff --git a/arch/Kconfig b/arch/Kconfig
> > index 8cc35dc556c7..e3d6b6868ccb 100644
> > --- a/arch/Kconfig
> > +++ b/arch/Kconfig
> > @@ -61,7 +61,7 @@ config OPROFILE_NMI_TIMER
> >
> > config KPROBES
> > bool "Kprobes"
> > - depends on MODULES
> > + depends on MODULES || ARCH_HAS_TEXT_ALLOC
> > depends on HAVE_KPROBES
> > select KALLSYMS
> > help
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > index 0dea7fdd7a00..a4ee5d1300f6 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -2035,6 +2035,9 @@ config KEXEC_FILE
> > config ARCH_HAS_KEXEC_PURGATORY
> > def_bool KEXEC_FILE
> >
> > +config ARCH_HAS_TEXT_ALLOC
> > + def_bool y
> > +
> > config KEXEC_SIG
> > bool "Verify kernel signature during kexec_file_load() syscall"
> > depends on KEXEC_FILE
> > diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
> > index e77261db2391..2878e4b753a0 100644
> > --- a/arch/x86/kernel/Makefile
> > +++ b/arch/x86/kernel/Makefile
> > @@ -68,6 +68,7 @@ obj-y += tsc.o tsc_msr.o io_delay.o rtc.o
> > obj-y += pci-iommu_table.o
> > obj-y += resource.o
> > obj-y += irqflags.o
> > +obj-y += text.o
> >
> > obj-y += process.o
> > obj-y += fpu/
> > diff --git a/arch/x86/kernel/module.c b/arch/x86/kernel/module.c
> > index 34b153cbd4ac..261df078f127 100644
> > --- a/arch/x86/kernel/module.c
> > +++ b/arch/x86/kernel/module.c
> > @@ -36,55 +36,6 @@ do { \
> > } while (0)
> > #endif
> >
> > -#ifdef CONFIG_RANDOMIZE_BASE
> > -static unsigned long module_load_offset;
> > -
> > -/* Mutex protects the module_load_offset. */
> > -static DEFINE_MUTEX(module_kaslr_mutex);
> > -
> > -static unsigned long int get_module_load_offset(void)
> > -{
> > - if (kaslr_enabled()) {
> > - mutex_lock(&module_kaslr_mutex);
> > - /*
> > - * Calculate the module_load_offset the first time this
> > - * code is called. Once calculated it stays the same until
> > - * reboot.
> > - */
> > - if (module_load_offset == 0)
> > - module_load_offset =
> > - (get_random_int() % 1024 + 1) * PAGE_SIZE;
> > - mutex_unlock(&module_kaslr_mutex);
> > - }
> > - return module_load_offset;
> > -}
> > -#else
> > -static unsigned long int get_module_load_offset(void)
> > -{
> > - return 0;
> > -}
> > -#endif
> > -
> > -void *module_alloc(unsigned long size)
> > -{
> > - void *p;
> > -
> > - if (PAGE_ALIGN(size) > MODULES_LEN)
> > - return NULL;
> > -
> > - p = __vmalloc_node_range(size, MODULE_ALIGN,
> > - MODULES_VADDR + get_module_load_offset(),
> > - MODULES_END, GFP_KERNEL,
> > - PAGE_KERNEL, 0, NUMA_NO_NODE,
> > - __builtin_return_address(0));
> > - if (p && (kasan_module_alloc(p, size) < 0)) {
> > - vfree(p);
> > - return NULL;
> > - }
> > -
> > - return p;
> > -}
>
> Please don't touch this module_alloc() at all. Then we can
> just call __vmalloc_node_range() in the text_alloc().
This is my mistake, not intentional. Had a fixup that by mistake did not
end up to the final patch set. Sorry about that.
>
> > -
> > #ifdef CONFIG_X86_32
> > int apply_relocate(Elf32_Shdr *sechdrs,
> > const char *strtab,
> > diff --git a/arch/x86/kernel/text.c b/arch/x86/kernel/text.c
> > new file mode 100644
> > index 000000000000..986b92ff1434
> > --- /dev/null
> > +++ b/arch/x86/kernel/text.c
> > @@ -0,0 +1,71 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Kernel module help for x86.
> > + * Copyright (C) 2001 Rusty Russell.
> > + */
> > +
> > +#include <linux/kasan.h>
> > +#include <linux/mm.h>
> > +#include <linux/moduleloader.h>
> > +#include <linux/vmalloc.h>
> > +#include <asm/setup.h>
> > +
> > +#ifdef CONFIG_RANDOMIZE_BASE
> > +static unsigned long module_load_offset;
> > +
> > +/* Mutex protects the module_load_offset. */
> > +static DEFINE_MUTEX(module_kaslr_mutex);
> > +
> > +static unsigned long get_module_load_offset(void)
> > +{
> > + if (kaslr_enabled()) {
> > + mutex_lock(&module_kaslr_mutex);
> > + /*
> > + * Calculate the module_load_offset the first time this
> > + * code is called. Once calculated it stays the same until
> > + * reboot.
> > + */
> > + if (module_load_offset == 0)
> > + module_load_offset =
> > + (get_random_int() % 1024 + 1) * PAGE_SIZE;
> > + mutex_unlock(&module_kaslr_mutex);
> > + }
> > + return module_load_offset;
> > +}
> > +#else
> > +static unsigned long get_module_load_offset(void)
> > +{
> > + return 0;
> > +}
> > +#endif
>
> So as I pointed, we can ignore this for kprobes (and other
> dynamic allocated trampoline code).
>
> > +
> > +void *text_alloc(unsigned long size)
> > +{
> > + void *p;
> > +
> > + if (PAGE_ALIGN(size) > MODULES_LEN)
> > + return NULL;
> > +
> > + p = __vmalloc_node_range(size, MODULE_ALIGN,
> > + MODULES_VADDR + get_module_load_offset(),
> > + MODULES_END, GFP_KERNEL,
> > + PAGE_KERNEL, 0, NUMA_NO_NODE,
> > + __builtin_return_address(0));
> > + if (p && (kasan_module_alloc(p, size) < 0)) {
> > + vfree(p);
> > + return NULL;
> > + }
> > +
> > + return p;
> > +}
> > +
> > +void text_free(void *region)
> > +{
> > + /*
> > + * This memory may be RO, and freeing RO memory in an interrupt is not
> > + * supported by vmalloc.
> > + */
> > + WARN_ON(in_interrupt());
> > +
> > + vfree(region);
> > +}
> > diff --git a/include/linux/text.h b/include/linux/text.h
> > new file mode 100644
> > index 000000000000..a27d4a42ecda
> > --- /dev/null
> > +++ b/include/linux/text.h
> > @@ -0,0 +1,17 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +
> > +#ifndef _LINUX_TEXT_H
> > +#define _LINUX_TEXT_H
> > +
> > +/*
> > + * An allocator used for allocating modules, core sections and init sections.
> > + * Returns NULL on failure.
> > + */
> > +void *text_alloc(unsigned long size);
> > +
> > +/*
> > + * Free memory returned from text_alloc().
> > + */
> > +void text_free(void *region);
>
> Hmm, if this is this short, in this version we might better move
> these to vmalloc.h.
>
> > +
> > +#endif /* _LINUX_TEXT_H */
> > diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> > index 2e97febeef77..fa7687eb0c0e 100644
> > --- a/kernel/kprobes.c
> > +++ b/kernel/kprobes.c
> > @@ -35,6 +35,7 @@
> > #include <linux/ftrace.h>
> > #include <linux/cpu.h>
> > #include <linux/jump_label.h>
> > +#include <linux/text.h>
> >
> > #include <asm/sections.h>
> > #include <asm/cacheflush.h>
> > @@ -111,12 +112,20 @@ enum kprobe_slot_state {
> >
> > void __weak *alloc_insn_page(void)
> > {
> > +#ifdef CONFIG_ARCH_HAS_TEXT_ALLOC
> > + return text_alloc(PAGE_SIZE);
> > +#else
> > return module_alloc(PAGE_SIZE);
> > +#endif
> > }
> >
> > void __weak free_insn_page(void *page)
> > {
> > +#ifdef CONFIG_ARCH_HAS_TEXT_ALLOC
> > + text_free(page);
> > +#else
> > module_memfree(page);
> > +#endif
> > }
> >
> > struct kprobe_insn_cache kprobe_insn_slots = {
> > @@ -1608,6 +1617,7 @@ static int check_kprobe_address_safe(struct kprobe *p,
> > goto out;
> > }
> >
> > +#ifdef CONFIG_MODULES
> > /*
> > * If the module freed .init.text, we couldn't insert
> > * kprobes in there.
> > @@ -1618,6 +1628,7 @@ static int check_kprobe_address_safe(struct kprobe *p,
> > *probed_mod = NULL;
> > ret = -ENOENT;
> > }
> > +#endif
>
> This change is not related to text_alloc(). Please move this to 3/3.
>
> > }
> > out:
> > preempt_enable();
> > diff --git a/kernel/module.c b/kernel/module.c
> > index aa183c9ac0a2..8adeb126b02c 100644
> > --- a/kernel/module.c
> > +++ b/kernel/module.c
> > @@ -56,6 +56,7 @@
> > #include <linux/bsearch.h>
> > #include <linux/dynamic_debug.h>
> > #include <linux/audit.h>
> > +#include <linux/text.h>
> > #include <uapi/linux/module.h>
> > #include "module-internal.h"
> >
> > @@ -2151,7 +2152,12 @@ void __weak module_memfree(void *module_region)
> > * supported by vmalloc.
> > */
> > WARN_ON(in_interrupt());
> > +
> > +#ifdef CONFIG_ARCH_HAS_TEXT_ALLOC
> > + text_free(module_region);
> > +#else
> > vfree(module_region);
> > +#endif
> > }
> >
> > void __weak module_arch_cleanup(struct module *mod)
> > @@ -2786,9 +2792,13 @@ static void dynamic_debug_remove(struct module *mod, struct _ddebug *debug)
> >
> > void * __weak module_alloc(unsigned long size)
> > {
> > +#ifdef CONFIG_ARCH_HAS_TEXT_ALLOC
> > + return text_alloc(size);
> > +#else
> > return __vmalloc_node_range(size, 1, VMALLOC_START, VMALLOC_END,
> > GFP_KERNEL, PAGE_KERNEL_EXEC, VM_FLUSH_RESET_PERMS,
> > NUMA_NO_NODE, __builtin_return_address(0));
> > +#endif
> > }
>
> Please don't touch kernel/module.c too. This seems to make things complicated.
>
> Thank you,
>
> --
> Masami Hiramatsu <mhiramat@kernel.org>
Thanks for the review. Agree with your remarks.
/Jarkko
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 3/3] kprobes: Flag out CONFIG_MODULES dependent code
2020-07-15 8:35 ` Masami Hiramatsu
@ 2020-07-16 17:36 ` Jarkko Sakkinen
0 siblings, 0 replies; 18+ messages in thread
From: Jarkko Sakkinen @ 2020-07-16 17:36 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: linux-kernel, x86, Andi Kleen, Jessica Yu, Naveen N. Rao,
Anil S Keshavamurthy, David S. Miller, Steven Rostedt,
Ingo Molnar
On Wed, Jul 15, 2020 at 05:35:24PM +0900, Masami Hiramatsu wrote:
> Hi Jarkko,
>
> On Wed, 15 Jul 2020 01:32:29 +0300
> Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote:
>
> > Remove CONFIG_MODULES dependency by flagging out the dependent code. This
> > allows to use kprobes in a kernel without support for loadable modules,
> > which could be useful for a test kernel or perhaps an embedded kernel.
> >
>
> OK, looks good, I just have 2 comments below.
>
> > Cc: Andi Kleen <ak@linux.intel.com>
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > ---
> > include/linux/module.h | 14 +++++++-------
> > kernel/kprobes.c | 7 +++++++
> > kernel/trace/trace_kprobe.c | 16 +++++++++++++++-
> > 3 files changed, 29 insertions(+), 8 deletions(-)
> >
> > diff --git a/include/linux/module.h b/include/linux/module.h
> > index 857b84bf9e90..eaa8ad02f75c 100644
> > --- a/include/linux/module.h
> > +++ b/include/linux/module.h
> > @@ -290,6 +290,13 @@ extern typeof(name) __mod_##type##__##name##_device_table \
> >
> > struct notifier_block;
> >
> > +enum module_state {
> > + MODULE_STATE_LIVE, /* Normal state. */
> > + MODULE_STATE_COMING, /* Full formed, running module_init. */
> > + MODULE_STATE_GOING, /* Going away. */
> > + MODULE_STATE_UNFORMED, /* Still setting it up. */
> > +};
> > +
> > #ifdef CONFIG_MODULES
> >
> > extern int modules_disabled; /* for sysctl */
> > @@ -305,13 +312,6 @@ struct module_use {
> > struct module *source, *target;
> > };
> >
> > -enum module_state {
> > - MODULE_STATE_LIVE, /* Normal state. */
> > - MODULE_STATE_COMING, /* Full formed, running module_init. */
> > - MODULE_STATE_GOING, /* Going away. */
> > - MODULE_STATE_UNFORMED, /* Still setting it up. */
> > -};
> > -
> > struct mod_tree_node {
> > struct module *mod;
> > struct latch_tree_node node;
> > diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> > index b4f3c24cd2ef..3039df13d34e 100644
> > --- a/kernel/kprobes.c
> > +++ b/kernel/kprobes.c
> > @@ -2225,6 +2225,7 @@ int kprobe_add_area_blacklist(unsigned long start, unsigned long end)
> > return 0;
> > }
> >
> > +#ifdef CONFIG_MODULES
> > /* Remove all symbols in given area from kprobe blacklist */
> > static void kprobe_remove_area_blacklist(unsigned long start, unsigned long end)
> > {
> > @@ -2242,6 +2243,7 @@ static void kprobe_remove_ksym_blacklist(unsigned long entry)
> > {
> > kprobe_remove_area_blacklist(entry, entry + 1);
> > }
> > +#endif
> >
> > int __init __weak arch_populate_kprobe_blacklist(void)
> > {
> > @@ -2285,6 +2287,7 @@ static int __init populate_kprobe_blacklist(unsigned long *start,
> > return ret ? : arch_populate_kprobe_blacklist();
> > }
> >
> > +#ifdef CONFIG_MODULES
> > static void add_module_kprobe_blacklist(struct module *mod)
> > {
> > unsigned long start, end;
> > @@ -2330,6 +2333,10 @@ static void remove_module_kprobe_blacklist(struct module *mod)
> > kprobe_remove_area_blacklist(start, end);
> > }
> > }
> > +#else
> > +static void add_module_kprobe_blacklist(struct module *mod) {}
> > +static void remove_module_kprobe_blacklist(struct module *mod) {}
> > +#endif /* CONFIG_MODULES */
>
> Please feel free to move the function. I would like to see;
>
> #ifdef CONFIG_MODULES
> /* Remove all symbols in given area from kprobe blacklist */
> static void kprobe_remove_area_blacklist(unsigned long start, unsigned long end)
> {
> ...
> }
> static void add_module_kprobe_blacklist(struct module *mod)
> {
> ...
> }
> #else
> static void add_module_kprobe_blacklist(struct module *mod) {}
> static void remove_module_kprobe_blacklist(struct module *mod) {}
> #endif /* CONFIG_MODULES */
>
> Rather than split #ifdefs.
>
> >
> > /* Module notifier call back, checking kprobes on the module */
> > static int kprobes_module_callback(struct notifier_block *nb,
> > diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> > index 710ec6a6aa8f..7fcd1bf2d96e 100644
> > --- a/kernel/trace/trace_kprobe.c
> > +++ b/kernel/trace/trace_kprobe.c
> > @@ -103,8 +103,9 @@ static nokprobe_inline bool trace_kprobe_has_gone(struct trace_kprobe *tk)
> > return !!(kprobe_gone(&tk->rp.kp));
> > }
> >
> > +#ifdef CONFIG_MODULES
> > static nokprobe_inline bool trace_kprobe_within_module(struct trace_kprobe *tk,
> > - struct module *mod)
> > + struct module *mod)
> > {
> > int len = strlen(mod->name);
> > const char *name = trace_kprobe_symbol(tk);
> > @@ -129,6 +130,17 @@ static nokprobe_inline bool trace_kprobe_module_exist(struct trace_kprobe *tk)
> >
> > return ret;
> > }
> > +#else
> > +static nokprobe_inline bool trace_kprobe_within_module(struct trace_kprobe *tk,
> > + struct module *mod)
> > +{
> > + return false;
> > +}
> > +static nokprobe_inline bool trace_kprobe_module_exist(struct trace_kprobe *tk)
> > +{
> > + return false;
> > +}
> > +#endif
> >
> > static bool trace_kprobe_is_busy(struct dyn_event *ev)
> > {
> > @@ -685,10 +697,12 @@ static int trace_kprobe_module_callback(struct notifier_block *nb,
> > /* Don't need to check busy - this should have gone. */
> > __unregister_trace_kprobe(tk);
> > ret = __register_trace_kprobe(tk);
> > +#ifdef CONFIG_MODULES
> > if (ret)
> > pr_warn("Failed to re-register probe %s on %s: %d\n",
> > trace_probe_name(&tk->tp),
> > mod->name, ret);
> > +#endif
>
> I guess this CONFIG_MODULES is for avoiding build error according to mod->name,
> if so, please use module_name(mod) macro instead of this #ifdef.
>
> > }
> > }
> > mutex_unlock(&event_mutex);
> > --
> > 2.25.1
> >
>
>
> --
> Masami Hiramatsu <mhiramat@kernel.org>
Thanks, agree with the remarks.
/Jarkko
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 2/3] module: Add lock_modules() and unlock_modules()
2020-07-15 8:39 ` Masami Hiramatsu
@ 2020-07-16 17:37 ` Jarkko Sakkinen
0 siblings, 0 replies; 18+ messages in thread
From: Jarkko Sakkinen @ 2020-07-16 17:37 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: linux-kernel, x86, Andi Kleen, Jessica Yu, Naveen N. Rao,
Anil S Keshavamurthy, David S. Miller, Josh Poimboeuf,
Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence,
Steven Rostedt, Ingo Molnar, open list:LIVE PATCHING
On Wed, Jul 15, 2020 at 05:39:39PM +0900, Masami Hiramatsu wrote:
> On Wed, 15 Jul 2020 01:32:28 +0300
> Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote:
>
> > Add wrappers to take the modules "big lock" in order to encapsulate
> > conditional compilation (CONFIG_MODULES) inside the wrapper.
> >
> > Cc: Andi Kleen <ak@linux.intel.com>
> > Suggested-by: Masami Hiramatsu <mhiramat@kernel.org>
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > ---
> > include/linux/module.h | 15 ++++++++++
> > kernel/kprobes.c | 4 +--
> > kernel/livepatch/core.c | 8 ++---
> > kernel/module.c | 60 ++++++++++++++++++-------------------
> > kernel/trace/trace_kprobe.c | 4 +--
> > 5 files changed, 53 insertions(+), 38 deletions(-)
> >
> > diff --git a/include/linux/module.h b/include/linux/module.h
> > index 2e6670860d27..857b84bf9e90 100644
> > --- a/include/linux/module.h
> > +++ b/include/linux/module.h
> > @@ -902,4 +902,19 @@ static inline bool module_sig_ok(struct module *module)
> > }
> > #endif /* CONFIG_MODULE_SIG */
> >
> > +#ifdef CONFIG_MODULES
> > +static inline void lock_modules(void)
> > +{
> > + mutex_lock(&module_mutex);
> > +}
> > +
> > +static inline void unlock_modules(void)
> > +{
> > + mutex_unlock(&module_mutex);
> > +}
> > +#else
> > +static inline void lock_modules(void) { }
> > +static inline void unlock_modules(void) { }
> > +#endif
>
> You don't need to add new #ifdefs. There is a room for dummy prototypes
> for !CONFIG_MODULES already in modules.h.
>
> -----
> struct notifier_block;
>
> #ifdef CONFIG_MODULES
>
> extern int modules_disabled; /* for sysctl */
>
> ...
> #else /* !CONFIG_MODULES... */
>
> static inline struct module *__module_address(unsigned long addr)
> {
> -----
>
> So you can just put those inlines in the appropriate places ;)
>
> Thank you,
Rrright.
/Jarkko
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/3] kprobes: Add text_alloc() and text_free()
2020-07-15 19:36 ` Kees Cook
2020-07-15 19:49 ` Mike Rapoport
@ 2020-07-23 1:39 ` Jarkko Sakkinen
1 sibling, 0 replies; 18+ messages in thread
From: Jarkko Sakkinen @ 2020-07-23 1:39 UTC (permalink / raw)
To: Kees Cook
Cc: linux-kernel, x86, Andi Kleen, Peter Zijlstra, Thomas Gleixner,
Ingo Molnar, Borislav Petkov,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
H. Peter Anvin, Naveen N. Rao, Anil S Keshavamurthy,
David S. Miller, Masami Hiramatsu, Jessica Yu, Andrew Morton,
Aneesh Kumar K.V, Will Deacon, Sami Tolvanen, Alexandre Ghiti,
Masahiro Yamada, Peter Collingbourne, Frederic Weisbecker,
Krzysztof Kozlowski, Arnd Bergmann, Stephen Boyd,
Andy Lutomirski, Josh Poimboeuf, Miroslav Benes, Babu Moger,
Omar Sandoval, Nayna Jain, Marco Elver, Brian Gerst, Jiri Kosina,
Joe Lawrence, Mike Rapoport
On Wed, Jul 15, 2020 at 12:36:02PM -0700, Kees Cook wrote:
> On Wed, Jul 15, 2020 at 01:32:27AM +0300, Jarkko Sakkinen wrote:
> > Introduce new API for allocating space for code generaed at run-time
> > leveraging from module_alloc() and module_memfree() code. Use this to
> > perform memory allocations in the kprobes code in order to loose the
> > bound between kprobes and modules subsystem.
> >
> > Initially, use this API only with arch/x86 and define a new config
> > flag CONFIG_ARCH_HAS_TEXT_ALLOC to promote the availability of the
> > new API.
> > [...]
> > diff --git a/include/linux/text.h b/include/linux/text.h
> > new file mode 100644
> > index 000000000000..a27d4a42ecda
> > --- /dev/null
> > +++ b/include/linux/text.h
> > @@ -0,0 +1,17 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +
> > +#ifndef _LINUX_TEXT_H
> > +#define _LINUX_TEXT_H
> > +
> > +/*
> > + * An allocator used for allocating modules, core sections and init sections.
> > + * Returns NULL on failure.
> > + */
> > +void *text_alloc(unsigned long size);
> > +
> > +/*
> > + * Free memory returned from text_alloc().
> > + */
> > +void text_free(void *region);
> > +
> > +#endif /* _LINUX_TEXT_H */
> > diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> > index 2e97febeef77..fa7687eb0c0e 100644
> > --- a/kernel/kprobes.c
> > +++ b/kernel/kprobes.c
> > @@ -35,6 +35,7 @@
> > #include <linux/ftrace.h>
> > #include <linux/cpu.h>
> > #include <linux/jump_label.h>
> > +#include <linux/text.h>
> >
> > #include <asm/sections.h>
> > #include <asm/cacheflush.h>
> > @@ -111,12 +112,20 @@ enum kprobe_slot_state {
> >
> > void __weak *alloc_insn_page(void)
> > {
> > +#ifdef CONFIG_ARCH_HAS_TEXT_ALLOC
> > + return text_alloc(PAGE_SIZE);
> > +#else
> > return module_alloc(PAGE_SIZE);
> > +#endif
>
> This seems like it shouldn't be needed. I think text_alloc() should
> always be visible. In the ARCH_HAS... case it will call the arch
> implementation, and without it should just call module_alloc():
>
> For example:
> void *text_alloc(unsigned long size)
> {
> #ifdef CONFIG_ARCH_HAS_TEXT_ALLOC
> return arch_text_alloc(size);
> #else
> return module_alloc(size);
> #endif
> }
>
> --
> Kees Cook
I fully agree with your comments and I posted a follow-up series where I
think these should be resolved. Please check it because before I got
this review, It was already posted.
Thank you.
/Jarkko
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/3] kprobes: Add text_alloc() and text_free()
2020-07-16 9:02 ` Peter Zijlstra
@ 2020-07-23 1:49 ` Jarkko Sakkinen
0 siblings, 0 replies; 18+ messages in thread
From: Jarkko Sakkinen @ 2020-07-23 1:49 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel, x86, Andi Kleen, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
H. Peter Anvin, Naveen N. Rao, Anil S Keshavamurthy,
David S. Miller, Masami Hiramatsu, Jessica Yu, Andrew Morton,
Aneesh Kumar K.V, Kees Cook, Will Deacon, Sami Tolvanen,
Alexandre Ghiti, Masahiro Yamada, Peter Collingbourne,
Frederic Weisbecker, Krzysztof Kozlowski, Arnd Bergmann,
Stephen Boyd, Andy Lutomirski, Josh Poimboeuf, Miroslav Benes,
Babu Moger, Omar Sandoval, Nayna Jain, Marco Elver, Brian Gerst,
Jiri Kosina, Joe Lawrence, Mike Rapoport
On Thu, Jul 16, 2020 at 11:02:53AM +0200, Peter Zijlstra wrote:
> On Wed, Jul 15, 2020 at 01:32:27AM +0300, Jarkko Sakkinen wrote:
> > +void *text_alloc(unsigned long size)
> > +{
> > + void *p;
> > +
> > + if (PAGE_ALIGN(size) > MODULES_LEN)
> > + return NULL;
> > +
> > + p = __vmalloc_node_range(size, MODULE_ALIGN,
> > + MODULES_VADDR + get_module_load_offset(),
> > + MODULES_END, GFP_KERNEL,
> > + PAGE_KERNEL, 0, NUMA_NO_NODE,
> > + __builtin_return_address(0));
> > + if (p && (kasan_module_alloc(p, size) < 0)) {
> > + vfree(p);
> > + return NULL;
> > + }
> > +
> > + return p;
> > +}
> > +
> > +void text_free(void *region)
> > +{
> > + /*
> > + * This memory may be RO, and freeing RO memory in an interrupt is not
> > + * supported by vmalloc.
> > + */
> > + WARN_ON(in_interrupt());
>
> I think that wants to be:
>
> lockdep_assert_irqs_enabled();
>
> in_interrupt() isn't sufficient, interrupts must also not be disabled
> when issuesing TLB invalidations.
Shouldn't it be then also fixed in the module_memfree() fallback
implementation (kernel/module.c)?
/Jarkko
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2020-07-23 1:49 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-14 22:32 [PATCH v3 0/3] kprobes: Remove MODULES dependency Jarkko Sakkinen
2020-07-14 22:32 ` [PATCH v3 1/3] kprobes: Add text_alloc() and text_free() Jarkko Sakkinen
2020-07-15 0:59 ` kernel test robot
2020-07-15 8:27 ` Masami Hiramatsu
2020-07-15 19:38 ` Kees Cook
2020-07-16 17:32 ` Jarkko Sakkinen
2020-07-15 19:36 ` Kees Cook
2020-07-15 19:49 ` Mike Rapoport
2020-07-15 19:51 ` Kees Cook
2020-07-23 1:39 ` Jarkko Sakkinen
2020-07-16 9:02 ` Peter Zijlstra
2020-07-23 1:49 ` Jarkko Sakkinen
2020-07-14 22:32 ` [PATCH v3 2/3] module: Add lock_modules() and unlock_modules() Jarkko Sakkinen
2020-07-15 8:39 ` Masami Hiramatsu
2020-07-16 17:37 ` Jarkko Sakkinen
2020-07-14 22:32 ` [PATCH v3 3/3] kprobes: Flag out CONFIG_MODULES dependent code Jarkko Sakkinen
2020-07-15 8:35 ` Masami Hiramatsu
2020-07-16 17:36 ` Jarkko Sakkinen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).