Linux-Security-Module Archive on lore.kernel.org
 help / color / Atom feed
From: Nadav Amit <namit@vmware.com>
To: Ingo Molnar <mingo@redhat.com>
Cc: <linux-kernel@vger.kernel.org>, <x86@kernel.org>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Borislav Petkov <bp@alien8.de>, Andy Lutomirski <luto@kernel.org>,
	Nadav Amit <nadav.amit@gmail.com>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Peter Zijlstra <peterz@infradead.org>, <linux_dti@icloud.com>,
	<linux-integrity@vger.kernel.org>,
	<linux-security-module@vger.kernel.org>,
	Nadav Amit <namit@vmware.com>,
	Steven Rostedt <rostedt@goodmis.org>
Subject: [PATCH v7 08/14] x86/ftrace: Use text_poke_*() infrastructure
Date: Tue, 4 Dec 2018 17:34:02 -0800
Message-ID: <20181205013408.47725-9-namit@vmware.com> (raw)
In-Reply-To: <20181205013408.47725-1-namit@vmware.com>

A following patch is going to make module allocated memory
non-executable. This requires to modify ftrace and make the memory
executable again after it is configured.

In addition, this patch makes ftrace use the general text poking
infrastructure instead ftrace's homegrown text patching. This provides
the advantages of having slightly "safer" code patching and avoiding
races with module removal or other mechanisms that patch the kernel
code.

Cc: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Nadav Amit <namit@vmware.com>
---
 arch/x86/kernel/ftrace.c | 74 +++++++++++++---------------------------
 1 file changed, 23 insertions(+), 51 deletions(-)

diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 01ebcb6f263e..f05a0f9e2837 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -22,6 +22,7 @@
 #include <linux/init.h>
 #include <linux/list.h>
 #include <linux/module.h>
+#include <linux/memory.h>
 
 #include <trace/syscall.h>
 
@@ -29,23 +30,10 @@
 #include <asm/kprobes.h>
 #include <asm/ftrace.h>
 #include <asm/nops.h>
+#include <asm/text-patching.h>
 
 #ifdef CONFIG_DYNAMIC_FTRACE
 
-int ftrace_arch_code_modify_prepare(void)
-{
-	set_kernel_text_rw();
-	set_all_modules_text_rw();
-	return 0;
-}
-
-int ftrace_arch_code_modify_post_process(void)
-{
-	set_all_modules_text_ro();
-	set_kernel_text_ro();
-	return 0;
-}
-
 union ftrace_code_union {
 	char code[MCOUNT_INSN_SIZE];
 	struct {
@@ -79,22 +67,6 @@ within(unsigned long addr, unsigned long start, unsigned long end)
 	return addr >= start && addr < end;
 }
 
-static unsigned long text_ip_addr(unsigned long ip)
-{
-	/*
-	 * On x86_64, kernel text mappings are mapped read-only, so we use
-	 * the kernel identity mapping instead of the kernel text mapping
-	 * to modify the kernel text.
-	 *
-	 * For 32bit kernels, these mappings are same and we can use
-	 * kernel identity mapping to modify code.
-	 */
-	if (within(ip, (unsigned long)_text, (unsigned long)_etext))
-		ip = (unsigned long)__va(__pa_symbol(ip));
-
-	return ip;
-}
-
 static const unsigned char *ftrace_nop_replace(void)
 {
 	return ideal_nops[NOP_ATOMIC5];
@@ -124,13 +96,8 @@ ftrace_modify_code_direct(unsigned long ip, unsigned const char *old_code,
 	if (memcmp(replaced, old_code, MCOUNT_INSN_SIZE) != 0)
 		return -EINVAL;
 
-	ip = text_ip_addr(ip);
-
 	/* replace the text with the new text */
-	if (probe_kernel_write((void *)ip, new_code, MCOUNT_INSN_SIZE))
-		return -EPERM;
-
-	sync_core();
+	text_poke_early((void *)ip, new_code, MCOUNT_INSN_SIZE);
 
 	return 0;
 }
@@ -302,10 +269,7 @@ int ftrace_int3_handler(struct pt_regs *regs)
 
 static int ftrace_write(unsigned long ip, const char *val, int size)
 {
-	ip = text_ip_addr(ip);
-
-	if (probe_kernel_write((void *)ip, val, size))
-		return -EPERM;
+	text_poke((void *)ip, val, size);
 
 	return 0;
 }
@@ -653,9 +617,11 @@ void arch_ftrace_update_code(int command)
 {
 	/* See comment above by declaration of modifying_ftrace_code */
 	atomic_inc(&modifying_ftrace_code);
+	mutex_lock(&text_mutex);
 
 	ftrace_modify_all_code(command);
 
+	mutex_unlock(&text_mutex);
 	atomic_dec(&modifying_ftrace_code);
 }
 
@@ -741,6 +707,7 @@ create_trampoline(struct ftrace_ops *ops, unsigned int *tramp_size)
 	unsigned long end_offset;
 	unsigned long op_offset;
 	unsigned long offset;
+	unsigned long npages;
 	unsigned long size;
 	unsigned long ip;
 	unsigned long *ptr;
@@ -748,7 +715,6 @@ create_trampoline(struct ftrace_ops *ops, unsigned int *tramp_size)
 	/* 48 8b 15 <offset> is movq <offset>(%rip), %rdx */
 	unsigned const char op_ref[] = { 0x48, 0x8b, 0x15 };
 	union ftrace_op_code_union op_ptr;
-	int ret;
 
 	if (ops->flags & FTRACE_OPS_FL_SAVE_REGS) {
 		start_offset = (unsigned long)ftrace_regs_caller;
@@ -772,19 +738,16 @@ create_trampoline(struct ftrace_ops *ops, unsigned int *tramp_size)
 		return 0;
 
 	*tramp_size = size + MCOUNT_INSN_SIZE + sizeof(void *);
+	npages = DIV_ROUND_UP(*tramp_size, PAGE_SIZE);
 
 	/* Copy ftrace_caller onto the trampoline memory */
-	ret = probe_kernel_read(trampoline, (void *)start_offset, size);
-	if (WARN_ON(ret < 0)) {
-		tramp_free(trampoline, *tramp_size);
-		return 0;
-	}
+	text_poke_early(trampoline, (void *)start_offset, size);
 
 	ip = (unsigned long)trampoline + size;
 
 	/* The trampoline ends with a jmp to ftrace_epilogue */
 	jmp = ftrace_jmp_replace(ip, (unsigned long)ftrace_epilogue);
-	memcpy(trampoline + size, jmp, MCOUNT_INSN_SIZE);
+	text_poke_early(trampoline + size, jmp, MCOUNT_INSN_SIZE);
 
 	/*
 	 * The address of the ftrace_ops that is used for this trampoline
@@ -813,11 +776,19 @@ create_trampoline(struct ftrace_ops *ops, unsigned int *tramp_size)
 	op_ptr.offset = offset;
 
 	/* put in the new offset to the ftrace_ops */
-	memcpy(trampoline + op_offset, &op_ptr, OP_REF_SIZE);
+	text_poke_early(trampoline + op_offset, &op_ptr, OP_REF_SIZE);
 
 	/* ALLOC_TRAMP flags lets us know we created it */
 	ops->flags |= FTRACE_OPS_FL_ALLOC_TRAMP;
 
+	set_memory_ro((unsigned long)trampoline, npages);
+
+	/*
+	 * TODO: Once we have better code (and page-table) protection
+	 * mechanisms, ensure that the code has not been tampered before.
+	 */
+	set_memory_x((unsigned long)trampoline, npages);
+
 	return (unsigned long)trampoline;
 }
 
@@ -853,8 +824,6 @@ void arch_ftrace_update_trampoline(struct ftrace_ops *ops)
 		 */
 		if (!(ops->flags & FTRACE_OPS_FL_ALLOC_TRAMP))
 			return;
-		npages = PAGE_ALIGN(ops->trampoline_size) >> PAGE_SHIFT;
-		set_memory_rw(ops->trampoline, npages);
 	} else {
 		ops->trampoline = create_trampoline(ops, &size);
 		if (!ops->trampoline)
@@ -863,6 +832,8 @@ void arch_ftrace_update_trampoline(struct ftrace_ops *ops)
 		npages = PAGE_ALIGN(size) >> PAGE_SHIFT;
 	}
 
+	mutex_lock(&text_mutex);
+
 	offset = calc_trampoline_call_offset(ops->flags & FTRACE_OPS_FL_SAVE_REGS);
 	ip = ops->trampoline + offset;
 
@@ -871,7 +842,8 @@ void arch_ftrace_update_trampoline(struct ftrace_ops *ops)
 	/* Do a safe modify in case the trampoline is executing */
 	new = ftrace_call_replace(ip, (unsigned long)func);
 	ret = update_ftrace_func(ip, new);
-	set_memory_ro(ops->trampoline, npages);
+
+	mutex_unlock(&text_mutex);
 
 	/* The update should never fail */
 	WARN_ON(ret);
-- 
2.17.1


  parent reply index

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-05  1:33 [PATCH v7 00/14] x86/alternative: text_poke() enhancements Nadav Amit
2018-12-05  1:33 ` [PATCH v7 01/14] Fix "x86/alternatives: Lockdep-enforce text_mutex in text_poke*()" Nadav Amit
2018-12-05  1:33 ` [PATCH v7 02/14] x86/jump_label: Use text_poke_early() during early init Nadav Amit
2018-12-05  1:33 ` [PATCH v7 03/14] x86/mm: temporary mm struct Nadav Amit
2018-12-05  1:33 ` [PATCH v7 04/14] fork: provide a function for copying init_mm Nadav Amit
2018-12-05  1:33 ` [PATCH v7 05/14] x86/alternative: initializing temporary mm for patching Nadav Amit
2018-12-05  1:34 ` [PATCH v7 06/14] x86/alternative: use temporary mm for text poking Nadav Amit
2018-12-05  1:34 ` [PATCH v7 07/14] x86/kgdb: avoid redundant comparison of patched code Nadav Amit
2018-12-05  1:34 ` Nadav Amit [this message]
2018-12-06  0:06   ` [PATCH v7 08/14] x86/ftrace: Use text_poke_*() infrastructure Nadav Amit
2018-12-06 16:28     ` Ingo Molnar
2018-12-05  1:34 ` [PATCH v7 09/14] x86/kprobes: Instruction pages initialization enhancements Nadav Amit
2018-12-06 13:09   ` Masami Hiramatsu
2018-12-05  1:34 ` [PATCH v7 10/14] x86: avoid W^X being broken during modules loading Nadav Amit
2018-12-05  1:34 ` [PATCH v7 11/14] x86/jump-label: remove support for custom poker Nadav Amit
2018-12-05  1:34 ` [PATCH v7 12/14] x86/alternative: Remove the return value of text_poke_*() Nadav Amit
2018-12-05  1:34 ` [PATCH v7 13/14] module: Do not set nx for module memory before freeing Nadav Amit
2018-12-06  9:57   ` Peter Zijlstra
2018-12-06 17:28     ` Nadav Amit
2018-12-06 11:13   ` Andrea Parri
2018-12-06 18:52   ` Andy Lutomirski
2018-12-06 18:56     ` Nadav Amit
2018-12-06 20:21     ` Edgecombe, Rick P
2018-12-06 20:29       ` Nadav Amit
2018-12-13 14:10   ` Jessica Yu
2018-12-13 17:25     ` Nadav Amit
2018-12-05  1:34 ` [PATCH v7 14/14] module: Prevent module removal racing with text_poke() Nadav Amit
2018-12-06 10:01   ` Peter Zijlstra
2018-12-06 10:03 ` [PATCH v7 00/14] x86/alternative: text_poke() enhancements Peter Zijlstra
2018-12-10  1:06   ` Nadav Amit

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181205013408.47725-9-namit@vmware.com \
    --to=namit@vmware.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=linux_dti@icloud.com \
    --cc=luto@kernel.org \
    --cc=mingo@redhat.com \
    --cc=nadav.amit@gmail.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Security-Module Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-security-module/0 linux-security-module/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-security-module linux-security-module/ https://lore.kernel.org/linux-security-module \
		linux-security-module@vger.kernel.org linux-security-module@archiver.kernel.org
	public-inbox-index linux-security-module


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-security-module


AGPL code for this site: git clone https://public-inbox.org/ public-inbox