From: Andi Kleen <ak@suse.de>
To: mathieu.desnoyers@polymtl.ca, jbeulich@novell.com,
"S. P. Prasanna" <prasanna@in.ibm.com>
Cc: linux-kernel@vger.kernel.org, patches@x86-64.org
Subject: new text patching for review
Date: Thu, 19 Jul 2007 11:05:43 +0200 [thread overview]
Message-ID: <200707191105.44056.ak@suse.de> (raw)
Hallo,
I had some second thoughts about the text patching of DEBUG_RODATA kernels
using change_page_attr(). Change_page_attr is intrusive and slow and using
a separate mapping is a little more gentle. I came up with this patch.
For your review and testing pleasure.
The main quirk is that it doesn't fully follow the cross-modifying code
recommendations; but i'm not sure that's really needed.
Also I admit nop_out is quite inefficient, but that's a very slow path
and it was easier to do it this way than handle all the corner cases
explicitely.
Comments,
-Andi
x86: Fix alternatives and kprobes to remap write-protected kernel text
Signed-off-by: Andi Kleen <ak@suse.de>
---
arch/i386/kernel/alternative.c | 33 +++++++++++++++++++++++++++++++--
arch/i386/kernel/kprobes.c | 9 +++------
arch/i386/mm/init.c | 14 +++-----------
arch/x86_64/kernel/kprobes.c | 10 +++-------
arch/x86_64/mm/init.c | 10 ----------
arch/x86_64/mm/pageattr.c | 2 +-
include/asm-i386/alternative.h | 2 ++
include/asm-x86_64/alternative.h | 2 ++
include/asm-x86_64/pgtable.h | 2 ++
9 files changed, 47 insertions(+), 37 deletions(-)
Index: linux/arch/x86_64/kernel/kprobes.c
===================================================================
--- linux.orig/arch/x86_64/kernel/kprobes.c
+++ linux/arch/x86_64/kernel/kprobes.c
@@ -39,9 +39,9 @@
#include <linux/module.h>
#include <linux/kdebug.h>
-#include <asm/cacheflush.h>
#include <asm/pgtable.h>
#include <asm/uaccess.h>
+#include <asm/alternative.h>
void jprobe_return_end(void);
static void __kprobes arch_copy_kprobe(struct kprobe *p);
@@ -209,16 +209,12 @@ static void __kprobes arch_copy_kprobe(s
void __kprobes arch_arm_kprobe(struct kprobe *p)
{
- *p->addr = BREAKPOINT_INSTRUCTION;
- flush_icache_range((unsigned long) p->addr,
- (unsigned long) p->addr + sizeof(kprobe_opcode_t));
+ text_poke(p->addr, BREAKPOINT_INSTRUCTION);
}
void __kprobes arch_disarm_kprobe(struct kprobe *p)
{
- *p->addr = p->opcode;
- flush_icache_range((unsigned long) p->addr,
- (unsigned long) p->addr + sizeof(kprobe_opcode_t));
+ text_poke(p->addr, p->opcode);
}
void __kprobes arch_remove_kprobe(struct kprobe *p)
Index: linux/arch/i386/kernel/alternative.c
===================================================================
--- linux.orig/arch/i386/kernel/alternative.c
+++ linux/arch/i386/kernel/alternative.c
@@ -2,8 +2,12 @@
#include <linux/sched.h>
#include <linux/spinlock.h>
#include <linux/list.h>
+#include <linux/kprobes.h>
+#include <linux/mm.h>
+#include <linux/vmalloc.h>
#include <asm/alternative.h>
#include <asm/sections.h>
+#include <asm/pgtable.h>
#ifdef CONFIG_HOTPLUG_CPU
static int smp_alt_once;
@@ -145,12 +149,15 @@ static unsigned char** find_nop_table(vo
static void nop_out(void *insns, unsigned int len)
{
unsigned char **noptable = find_nop_table();
+ int i;
while (len > 0) {
unsigned int noplen = len;
if (noplen > ASM_NOP_MAX)
noplen = ASM_NOP_MAX;
- memcpy(insns, noptable[noplen], noplen);
+ /* Very inefficient */
+ for (i = 0; i < noplen; i++)
+ text_poke(insns + i, noptable[noplen][i]);
insns += noplen;
len -= noplen;
}
@@ -202,7 +209,7 @@ static void alternatives_smp_lock(u8 **s
continue;
if (*ptr > text_end)
continue;
- **ptr = 0xf0; /* lock prefix */
+ text_poke(ptr, 0xf0); /* lock prefix */
};
}
@@ -406,3 +413,25 @@ void __init alternative_instructions(voi
apply_paravirt(__parainstructions, __parainstructions_end);
local_irq_restore(flags);
}
+
+/*
+ * RED-PEN Intel recommends stopping the other CPUs for such
+ * "cross-modifying code".
+ */
+void __kprobes text_poke(void *oaddr, u8 opcode)
+{
+ u8 *addr = oaddr;
+ if (!pte_write(*lookup_address((unsigned long)addr))) {
+ struct page *p = virt_to_page(addr);
+ addr = vmap(&p, 1, VM_MAP, PAGE_KERNEL);
+ if (!addr)
+ return;
+ addr += ((unsigned long)oaddr) % PAGE_SIZE;
+ }
+ *addr = opcode;
+ /* Not strictly needed, but can speed CPU recovery up */
+ if (cpu_has_clflush)
+ asm("clflush (%0) " :: "r" (addr) : "memory");
+ if (addr != oaddr)
+ vunmap(addr);
+}
Index: linux/arch/x86_64/mm/pageattr.c
===================================================================
--- linux.orig/arch/x86_64/mm/pageattr.c
+++ linux/arch/x86_64/mm/pageattr.c
@@ -13,7 +13,7 @@
#include <asm/tlbflush.h>
#include <asm/io.h>
-static inline pte_t *lookup_address(unsigned long address)
+pte_t *lookup_address(unsigned long address)
{
pgd_t *pgd = pgd_offset_k(address);
pud_t *pud;
Index: linux/include/asm-i386/alternative.h
===================================================================
--- linux.orig/include/asm-i386/alternative.h
+++ linux/include/asm-i386/alternative.h
@@ -149,4 +149,6 @@ apply_paravirt(struct paravirt_patch_sit
#define __parainstructions_end NULL
#endif
+extern void text_poke(void *addr, u8 opcode);
+
#endif /* _I386_ALTERNATIVE_H */
Index: linux/include/asm-x86_64/alternative.h
===================================================================
--- linux.orig/include/asm-x86_64/alternative.h
+++ linux/include/asm-x86_64/alternative.h
@@ -154,4 +154,6 @@ apply_paravirt(struct paravirt_patch *st
#define __parainstructions_end NULL
#endif
+extern void text_poke(void *addr, u8 opcode);
+
#endif /* _X86_64_ALTERNATIVE_H */
Index: linux/include/asm-x86_64/pgtable.h
===================================================================
--- linux.orig/include/asm-x86_64/pgtable.h
+++ linux/include/asm-x86_64/pgtable.h
@@ -403,6 +403,8 @@ extern struct list_head pgd_list;
extern int kern_addr_valid(unsigned long addr);
+pte_t *lookup_address(unsigned long addr);
+
#define io_remap_pfn_range(vma, vaddr, pfn, size, prot) \
remap_pfn_range(vma, vaddr, pfn, size, prot)
Index: linux/arch/i386/kernel/kprobes.c
===================================================================
--- linux.orig/arch/i386/kernel/kprobes.c
+++ linux/arch/i386/kernel/kprobes.c
@@ -35,6 +35,7 @@
#include <asm/cacheflush.h>
#include <asm/desc.h>
#include <asm/uaccess.h>
+#include <asm/alternative.h>
void jprobe_return_end(void);
@@ -169,16 +170,12 @@ int __kprobes arch_prepare_kprobe(struct
void __kprobes arch_arm_kprobe(struct kprobe *p)
{
- *p->addr = BREAKPOINT_INSTRUCTION;
- flush_icache_range((unsigned long) p->addr,
- (unsigned long) p->addr + sizeof(kprobe_opcode_t));
+ text_poke(p->addr, BREAKPOINT_INSTRUCTION);
}
void __kprobes arch_disarm_kprobe(struct kprobe *p)
{
- *p->addr = p->opcode;
- flush_icache_range((unsigned long) p->addr,
- (unsigned long) p->addr + sizeof(kprobe_opcode_t));
+ text_poke(p->addr, p->opcode);
}
void __kprobes arch_remove_kprobe(struct kprobe *p)
Index: linux/arch/i386/mm/init.c
===================================================================
--- linux.orig/arch/i386/mm/init.c
+++ linux/arch/i386/mm/init.c
@@ -801,17 +801,9 @@ void mark_rodata_ro(void)
unsigned long start = PFN_ALIGN(_text);
unsigned long size = PFN_ALIGN(_etext) - start;
-#ifndef CONFIG_KPROBES
-#ifdef CONFIG_HOTPLUG_CPU
- /* It must still be possible to apply SMP alternatives. */
- if (num_possible_cpus() <= 1)
-#endif
- {
- change_page_attr(virt_to_page(start),
- size >> PAGE_SHIFT, PAGE_KERNEL_RX);
- printk("Write protecting the kernel text: %luk\n", size >> 10);
- }
-#endif
+ change_page_attr(virt_to_page(start),
+ size >> PAGE_SHIFT, PAGE_KERNEL_RX);
+ printk("Write protecting the kernel text: %luk\n", size >> 10);
start += size;
size = (unsigned long)__end_rodata - start;
change_page_attr(virt_to_page(start),
Index: linux/arch/x86_64/mm/init.c
===================================================================
--- linux.orig/arch/x86_64/mm/init.c
+++ linux/arch/x86_64/mm/init.c
@@ -600,16 +600,6 @@ void mark_rodata_ro(void)
{
unsigned long start = (unsigned long)_stext, end;
-#ifdef CONFIG_HOTPLUG_CPU
- /* It must still be possible to apply SMP alternatives. */
- if (num_possible_cpus() > 1)
- start = (unsigned long)_etext;
-#endif
-
-#ifdef CONFIG_KPROBES
- start = (unsigned long)__start_rodata;
-#endif
-
end = (unsigned long)__end_rodata;
start = (start + PAGE_SIZE - 1) & PAGE_MASK;
end &= PAGE_MASK;
next reply other threads:[~2007-07-19 9:06 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-07-19 9:05 Andi Kleen [this message]
2007-07-19 13:38 ` Mathieu Desnoyers
2007-07-19 13:46 ` Andi Kleen
2007-07-19 17:35 ` Mathieu Desnoyers
2007-07-19 21:14 ` Andi Kleen
2007-07-19 20:30 ` Jeremy Fitzhardinge
2007-07-19 20:46 ` [patches] " Andi Kleen
2007-07-19 20:51 ` Jeremy Fitzhardinge
2007-07-19 21:06 ` Andi Kleen
2007-07-19 21:08 ` Jeremy Fitzhardinge
2007-07-19 23:53 ` Mathieu Desnoyers
2007-08-20 0:55 ` Non atomic unaligned writes Mathieu Desnoyers
2007-08-20 5:03 ` Arjan van de Ven
2007-08-20 10:23 ` Andi Kleen
2007-07-19 23:51 ` new text patching for review Mathieu Desnoyers
2007-07-19 23:49 ` Mathieu Desnoyers
2007-07-20 1:15 ` Zachary Amsden
2007-07-20 7:37 ` Andi Kleen
2007-07-20 15:17 ` Mathieu Desnoyers
2007-07-21 6:19 ` Andi Kleen
2007-07-20 8:28 ` Andi Kleen
2007-07-20 14:36 ` Mathieu Desnoyers
2007-07-20 0:37 ` Zachary Amsden
2007-07-20 8:23 ` Andi Kleen
2007-08-10 19:00 ` Mathieu Desnoyers
2007-07-19 12:25 Jan Beulich
2007-07-19 12:41 ` Andi Kleen
Reply instructions:
You may reply publicly 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=200707191105.44056.ak@suse.de \
--to=ak@suse.de \
--cc=jbeulich@novell.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mathieu.desnoyers@polymtl.ca \
--cc=patches@x86-64.org \
--cc=prasanna@in.ibm.com \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome