diff mbox series

[3/4] kprobes/treewide: Explicitly override alloc/free functions

Message ID 20240326163624.3253157-4-mark.rutland@arm.com (mailing list archive)
State New, archived
Headers show
Series kprobes: permit use without modules | expand

Commit Message

Mark Rutland March 26, 2024, 4:36 p.m. UTC 
Currently architectures can override kprobes_alloc_insn_page(), but
kprobes_free_insn_page() is always implemented using module_memfree(),
which might not be what an architecture needs, especially as we'd like
to make it possible to use kprobes without requiring MODULES.

It would be nicer if architectures either:

(a) Used only the generic kprobes_alloc_insn_page() and
    kprobes_free_insn_page(), implicitly depending on MODULES.

(b) Provided their own implementation of both kprobes_alloc_insn_page()
    and kprobes_free_insn_page(), handling the relevant dependencies
    themselves.

This patch applies that split treewide:

(a) Architectures using the generic kprobes_free_insn_page() and
    kprobes_free_insn_page() are left as-is. The __weak annotation is
    removed from the generic implementations so that accidental
    overrides/misuse can be detected easily.

(b) Architectures which provide their own kprobes_free_insn_page() are
    given a matching implementation of kprobes_free_insn_page(), and
    select HAVE_KPROBES_ALLOC.

    This new Kconfig symbol will allow subsequent patches to relax the
    dependency on MODULES to (MODULES || HAVE_KPROBES_ALLOC) once other
    module dependencies in the core kprobes code are cleaned up.

    Architectures which use module_alloc() are given an implementation
    using module_memfree() along with an explicit dependency on MODULES.

    Architectures using __vmalloc_node_range() are given an
    implementation using vfree(). This loses the warning for
    in_interrupt(), but vfree() can handle this via vfree_atomic(), so
    the warning isn't necessary.

    On riscv, the allocator depends on !XIP_KERNEL, which is already a
    dependency for HAVE_KPROBES in arch/riscv/Kconfig.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Jarkko Sakkinen <jarkko@kernel.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Naveen N. Rao <naveen.n.rao@linux.ibm.com>
---
 arch/Kconfig                       | 3 +++
 arch/arm64/Kconfig                 | 1 +
 arch/arm64/kernel/probes/kprobes.c | 5 +++++
 arch/powerpc/Kconfig               | 3 ++-
 arch/powerpc/kernel/kprobes.c      | 5 +++++
 arch/riscv/Kconfig                 | 1 +
 arch/riscv/kernel/probes/kprobes.c | 5 +++++
 arch/s390/Kconfig                  | 3 ++-
 arch/s390/kernel/kprobes.c         | 5 +++++
 arch/x86/Kconfig                   | 3 ++-
 arch/x86/kernel/kprobes/core.c     | 5 +++++
 include/linux/kprobes.h            | 1 +
 kernel/kprobes.c                   | 6 ++++--
 13 files changed, 41 insertions(+), 5 deletions(-)

Comments

Alexander Gordeev April 13, 2024, 7:22 a.m. UTC | #1 
On Tue, Mar 26, 2024 at 04:36:23PM +0000, Mark Rutland wrote:
Hi Mark,

...
> (a) Architectures using the generic kprobes_free_insn_page() and
                                      kprobes_alloc_insn_page()?
>     kprobes_free_insn_page() are left as-is. The __weak annotation is
>     removed from the generic implementations so that accidental
>     overrides/misuse can be detected easily.
> 
> (b) Architectures which provide their own kprobes_free_insn_page() are
                                            kprobes_alloc_insn_page()?
>     given a matching implementation of kprobes_free_insn_page(), and
>     select HAVE_KPROBES_ALLOC.
..
>  arch/s390/Kconfig                  | 3 ++-
>  arch/s390/kernel/kprobes.c         | 5 +++++

I tried the repo:
git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git    kprobes/without-modules

For s390:
Acked-by: Alexander Gordeev <agordeev@linux.ibm.com>

Thanks!
diff mbox series

Patch

diff --git a/arch/Kconfig b/arch/Kconfig
index 9f066785bb71d..85bb59f7b8c07 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -206,6 +206,9 @@  config HAVE_IOREMAP_PROT
 config HAVE_KPROBES
 	bool
 
+config HAVE_KPROBES_ALLOC
+	bool
+
 config HAVE_KRETPROBES
 	bool
 
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 7b11c98b3e84b..bda7913d6c9b8 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -233,6 +233,7 @@  config ARM64
 	select HAVE_STACKPROTECTOR
 	select HAVE_SYSCALL_TRACEPOINTS
 	select HAVE_KPROBES
+	select HAVE_KPROBES_ALLOC
 	select HAVE_KRETPROBES
 	select HAVE_GENERIC_VDSO
 	select HOTPLUG_CORE_SYNC_DEAD if HOTPLUG_CPU
diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
index 4b6ab7b1fa211..69d19a390cd48 100644
--- a/arch/arm64/kernel/probes/kprobes.c
+++ b/arch/arm64/kernel/probes/kprobes.c
@@ -136,6 +136,11 @@  void *kprobes_alloc_insn_page(void)
 			NUMA_NO_NODE, __builtin_return_address(0));
 }
 
+void kprobes_free_insn_page(void *page)
+{
+	vfree(page);
+}
+
 /* arm kprobe: install breakpoint in text */
 void __kprobes arch_arm_kprobe(struct kprobe *p)
 {
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 1c4be33736860..13e0fc51dcdcf 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -254,7 +254,8 @@  config PPC
 	select HAVE_KERNEL_LZMA			if DEFAULT_UIMAGE
 	select HAVE_KERNEL_LZO			if DEFAULT_UIMAGE
 	select HAVE_KERNEL_XZ			if PPC_BOOK3S || 44x
-	select HAVE_KPROBES
+	select HAVE_KPROBES			if MODULES
+	select HAVE_KPROBES_ALLOC
 	select HAVE_KPROBES_ON_FTRACE
 	select HAVE_KRETPROBES
 	select HAVE_LD_DEAD_CODE_DATA_ELIMINATION if HAVE_OBJTOOL_MCOUNT && (!ARCH_USING_PATCHABLE_FUNCTION_ENTRY || (!CC_IS_GCC || GCC_VERSION >= 110100))
diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
index 0b297718d5de6..d0332aaebab09 100644
--- a/arch/powerpc/kernel/kprobes.c
+++ b/arch/powerpc/kernel/kprobes.c
@@ -146,6 +146,11 @@  void *kprobes_alloc_insn_page(void)
 	return NULL;
 }
 
+void kprobes_free_insn_page(void *page)
+{
+	module_memfree(page);
+}
+
 int arch_prepare_kprobe(struct kprobe *p)
 {
 	int ret = 0;
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index be09c8836d56b..4e22549a522a5 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -139,6 +139,7 @@  config RISCV
 	select HAVE_GENERIC_VDSO if MMU && 64BIT
 	select HAVE_IRQ_TIME_ACCOUNTING
 	select HAVE_KPROBES if !XIP_KERNEL
+	select HAVE_KPROBES_ALLOC
 	select HAVE_KPROBES_ON_FTRACE if !XIP_KERNEL
 	select HAVE_KRETPROBES if !XIP_KERNEL
 	# https://github.com/ClangBuiltLinux/linux/issues/1881
diff --git a/arch/riscv/kernel/probes/kprobes.c b/arch/riscv/kernel/probes/kprobes.c
index 75201ce721057..37fdfa952d999 100644
--- a/arch/riscv/kernel/probes/kprobes.c
+++ b/arch/riscv/kernel/probes/kprobes.c
@@ -112,6 +112,11 @@  void *kprobes_alloc_insn_page(void)
 				     VM_FLUSH_RESET_PERMS, NUMA_NO_NODE,
 				     __builtin_return_address(0));
 }
+
+void kprobes_free_insn_page(void *page)
+{
+	vfree(page);
+}
 #endif
 
 /* install breakpoint in text */
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 8f01ada6845e3..635eddc3fce80 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -193,7 +193,8 @@  config S390
 	select HAVE_KERNEL_UNCOMPRESSED
 	select HAVE_KERNEL_XZ
 	select HAVE_KERNEL_ZSTD
-	select HAVE_KPROBES
+	select HAVE_KPROBES		if MODULES
+	select HAVE_KPROBES_ALLOC
 	select HAVE_KPROBES_ON_FTRACE
 	select HAVE_KRETPROBES
 	select HAVE_LIVEPATCH
diff --git a/arch/s390/kernel/kprobes.c b/arch/s390/kernel/kprobes.c
index 91ca4d501d4ef..a5b142b8eb0f7 100644
--- a/arch/s390/kernel/kprobes.c
+++ b/arch/s390/kernel/kprobes.c
@@ -45,6 +45,11 @@  void *kprobes_alloc_insn_page(void)
 	return page;
 }
 
+void kprobes_free_insn_page(void *page)
+{
+	module_memfree(page);
+}
+
 static void *alloc_s390_insn_page(void)
 {
 	if (xchg(&insn_page_in_use, 1) == 1)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 39886bab943a8..bdd327b0124e2 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -240,7 +240,8 @@  config X86
 	select HAVE_KERNEL_LZO
 	select HAVE_KERNEL_XZ
 	select HAVE_KERNEL_ZSTD
-	select HAVE_KPROBES
+	select HAVE_KPROBES			if MODULES
+	select HAVE_KPROBES_ALLOC
 	select HAVE_KPROBES_ON_FTRACE
 	select HAVE_FUNCTION_ERROR_INJECTION
 	select HAVE_KRETPROBES
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index 7f01bbbfa9e2a..5f093b94d9b40 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -508,6 +508,11 @@  void *kprobes_alloc_insn_page(void)
 	return page;
 }
 
+void kprobes_free_insn_page(void *page)
+{
+	module_memfree(page);
+}
+
 /* Kprobe x86 instruction emulation - only regs->ip or IF flag modifiers */
 
 static void kprobe_emulate_ifmodifiers(struct kprobe *p, struct pt_regs *regs)
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index ad4b561100f9e..651c807727bea 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -431,6 +431,7 @@  int enable_kprobe(struct kprobe *kp);
 void dump_kprobe(struct kprobe *kp);
 
 void *kprobes_alloc_insn_page(void);
+void kprobes_free_insn_page(void *page);
 
 void *kprobes_alloc_optinsn_page(void);
 void kprobes_free_optinsn_page(void *page);
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 35adf56430c9b..fa2ee4e59eca2 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -110,7 +110,8 @@  enum kprobe_slot_state {
 	SLOT_USED = 2,
 };
 
-void __weak *kprobes_alloc_insn_page(void)
+#ifndef CONFIG_HAVE_KPROBES_ALLOC
+void *kprobes_alloc_insn_page(void)
 {
 	/*
 	 * Use module_alloc() so this page is within +/- 2GB of where the
@@ -121,10 +122,11 @@  void __weak *kprobes_alloc_insn_page(void)
 	return module_alloc(PAGE_SIZE);
 }
 
-static void kprobes_free_insn_page(void *page)
+void kprobes_free_insn_page(void *page)
 {
 	module_memfree(page);
 }
+#endif
 
 struct kprobe_insn_cache kprobe_insn_slots = {
 	.mutex = __MUTEX_INITIALIZER(kprobe_insn_slots.mutex),