[patch 3/3] Text Edit Lock - kprobes i386

[patch 3/3] Text Edit Lock - kprobes i386

[Mathieu Desnoyers]

[-- Attachment #1: text-edit-lock-kprobes-i386.patch --]
[-- Type: text/plain, Size: 1413 bytes --]

Kprobes can use the text edit lock to insure mutual exclusion when edition the
code and make sure the pages are writable.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
---
 arch/i386/kernel/kprobes.c |    4 ++++
 1 file changed, 4 insertions(+)

Index: linux-2.6-lttng/arch/i386/kernel/kprobes.c
===================================================================
--- linux-2.6-lttng.orig/arch/i386/kernel/kprobes.c	2007-06-18 17:41:27.000000000 -0400
+++ linux-2.6-lttng/arch/i386/kernel/kprobes.c	2007-06-18 17:41:31.000000000 -0400
@@ -169,14 +169,18 @@
 
 void __kprobes arch_arm_kprobe(struct kprobe *p)
 {
+	kernel_text_lock((unsigned long)p->addr, sizeof(kprobe_opcode_t));
 	*p->addr = BREAKPOINT_INSTRUCTION;
+	kernel_text_unlock((unsigned long)p->addr, sizeof(kprobe_opcode_t));
 	flush_icache_range((unsigned long) p->addr,
 			   (unsigned long) p->addr + sizeof(kprobe_opcode_t));
 }
 
 void __kprobes arch_disarm_kprobe(struct kprobe *p)
 {
+	kernel_text_lock((unsigned long)p->addr, sizeof(kprobe_opcode_t));
 	*p->addr = p->opcode;
+	kernel_text_unlock((unsigned long)p->addr, sizeof(kprobe_opcode_t));
 	flush_icache_range((unsigned long) p->addr,
 			   (unsigned long) p->addr + sizeof(kprobe_opcode_t));
 }

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

[patch 0/3] Text Edit Lock (i386)

[Mathieu Desnoyers]

Hi,

This is the updated text edit lock, which protects the text segment
modifications (paravirt, kprobes, immediate values).

Is applies on 2.6.22-rc6-mm1. It depends on the cpa fixes I sent previously.

Mathieu

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

[patch 1/3] Text Edit Lock - i386

[Mathieu Desnoyers]

[-- Attachment #1: text-edit-lock-i386.patch --]
[-- Type: text/plain, Size: 4007 bytes --]

Interface to use for code patching : uses a mutex to insure mutual edit
exclusion and makes sure the page is writable.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
---
 arch/i386/mm/init.c           |   69 ++++++++++++++++++++++++++++++++++--------
 include/asm-i386/cacheflush.h |    4 ++
 2 files changed, 61 insertions(+), 12 deletions(-)

Index: linux-2.6-lttng/arch/i386/mm/init.c
===================================================================
--- linux-2.6-lttng.orig/arch/i386/mm/init.c	2007-06-29 14:15:39.000000000 -0400
+++ linux-2.6-lttng/arch/i386/mm/init.c	2007-06-29 14:22:02.000000000 -0400
@@ -31,6 +31,7 @@
 #include <linux/memory_hotplug.h>
 #include <linux/initrd.h>
 #include <linux/cpumask.h>
+#include <linux/mutex.h>
 
 #include <asm/processor.h>
 #include <asm/system.h>
@@ -53,6 +54,13 @@
 
 static int noinline do_test_wp_bit(void);
 
+/* spin lock protecting text section modification (dynamic code patching) */
+static DEFINE_SPINLOCK(text_lock);
+
+#ifdef CONFIG_DEBUG_RODATA
+static int rodata_marked;
+#endif
+
 /*
  * Creates a middle page table and puts a pointer to it in the
  * given global directory entry. This only returns the gd entry
@@ -802,18 +810,11 @@
 	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);
-		kernel_text_is_ro = 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);
+	kernel_text_is_ro = 1;
+
 	start += size;
 	size = (unsigned long)__end_rodata - start;
 	change_page_attr(virt_to_page(start),
@@ -828,8 +829,52 @@
 	 * of who is the culprit.
 	 */
 	global_flush_tlb();
+	rodata_marked = 1;
+}
+#endif
+
+/*
+ * Lock the kernel text for mutual write exclusion.
+ * Make sure the pages are writable.
+ */
+void kernel_text_lock(unsigned long address, size_t len)
+{
+	spin_lock(&text_lock);
+#if defined(CONFIG_DEBUG_RODATA)
+	if (rodata_marked && address >= PFN_ALIGN(_text)
+			&& (address + len) <= PFN_ALIGN(_etext)) {
+		unsigned long nr_pages;
+		nr_pages = ((address + len) >> PAGE_SHIFT)
+				- (address >> PAGE_SHIFT) + 1;
+		change_page_attr(virt_to_page(address), nr_pages,
+				PAGE_KERNEL_EXEC);
+		mb();
+		global_flush_tlb();
+		mb();
+	}
+#endif
 }
+EXPORT_SYMBOL_GPL(kernel_text_lock);
+
+void kernel_text_unlock(unsigned long address, size_t len)
+{
+#if defined(CONFIG_DEBUG_RODATA)
+	if (rodata_marked && address >= PFN_ALIGN(_text)
+			&& (address + len) <= PFN_ALIGN(_etext)) {
+		unsigned long nr_pages;
+		wmb();
+		nr_pages = ((address + len) >> PAGE_SHIFT)
+				- (address >> PAGE_SHIFT) + 1;
+		mb();
+		change_page_attr(virt_to_page(address), nr_pages,
+				PAGE_KERNEL_RX);
+		mb();
+		global_flush_tlb();
+	}
 #endif
+	spin_unlock(&text_lock);
+}
+EXPORT_SYMBOL_GPL(kernel_text_unlock);
 
 void free_init_pages(char *what, unsigned long begin, unsigned long end)
 {
Index: linux-2.6-lttng/include/asm-i386/cacheflush.h
===================================================================
--- linux-2.6-lttng.orig/include/asm-i386/cacheflush.h	2007-06-29 10:21:47.000000000 -0400
+++ linux-2.6-lttng/include/asm-i386/cacheflush.h	2007-06-29 14:20:29.000000000 -0400
@@ -36,4 +36,8 @@
 void mark_rodata_ro(void);
 #endif
 
+/* lock kernel text and mark pages writable */
+extern void kernel_text_lock(unsigned long address, size_t len);
+extern void kernel_text_unlock(unsigned long address, size_t len);
+
 #endif /* _I386_CACHEFLUSH_H */

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

[patch 2/3] Text Edit Lock - Alternative i386

[Mathieu Desnoyers]

[-- Attachment #1: text-edit-lock-alternative-i386.patch --]
[-- Type: text/plain, Size: 1664 bytes --]

alternative.c can use the text edit lock to remove CPU HOTPLUG special cases.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
---
 arch/i386/kernel/alternative.c |    5 +++++
 1 file changed, 5 insertions(+)

Index: linux-2.6-lttng/arch/i386/kernel/alternative.c
===================================================================
--- linux-2.6-lttng.orig/arch/i386/kernel/alternative.c	2007-06-18 17:38:41.000000000 -0400
+++ linux-2.6-lttng/arch/i386/kernel/alternative.c	2007-06-18 17:42:30.000000000 -0400
@@ -4,6 +4,7 @@
 #include <linux/list.h>
 #include <asm/alternative.h>
 #include <asm/sections.h>
+#include <asm/cacheflush.h>
 
 static int noreplace_smp     = 0;
 static int smp_alt_once      = 0;
@@ -179,9 +180,11 @@
 				__FUNCTION__, a->instr, instr);
 		}
 #endif
+		kernel_text_lock((unsigned long)instr, a->instrlen);
 		memcpy(instr, a->replacement, a->replacementlen);
 		diff = a->instrlen - a->replacementlen;
 		nop_out(instr + a->replacementlen, diff);
+		kernel_text_unlock((unsigned long)instr, a->instrlen);
 	}
 }
 
@@ -346,6 +349,7 @@
 	for (p = start; p < end; p++) {
 		unsigned int used;
 
+		kernel_text_lock((unsigned long)p->instr, p->len);
 		used = paravirt_ops.patch(p->instrtype, p->clobbers, p->instr,
 					  p->len);
 
@@ -353,6 +357,7 @@
 
 		/* Pad the rest with nops */
 		nop_out(p->instr + used, p->len - used);
+		kernel_text_unlock((unsigned long)p->instr, p->len);
 	}
 
 	/* Sync to be conservative, in case we patched following

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

[patch 3/3] Text Edit Lock - kprobes i386

[Mathieu Desnoyers]

[-- Attachment #1: text-edit-lock-kprobes-i386.patch --]
[-- Type: text/plain, Size: 2282 bytes --]

Kprobes can use the text edit lock to insure mutual exclusion when edition the
code and make sure the pages are writable.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
CC: prasanna@in.ibm.com
CC: ananth@in.ibm.com
CC: anil.s.keshavamurthy@intel.com
CC: davem@davemloft.net
---
 arch/i386/kernel/kprobes.c |   30 ++++--------------------------
 1 file changed, 4 insertions(+), 26 deletions(-)

Index: linux-2.6-lttng/arch/i386/kernel/kprobes.c
===================================================================
--- linux-2.6-lttng.orig/arch/i386/kernel/kprobes.c	2007-06-29 14:15:38.000000000 -0400
+++ linux-2.6-lttng/arch/i386/kernel/kprobes.c	2007-06-29 14:25:08.000000000 -0400
@@ -169,42 +169,20 @@
 
 void __kprobes arch_arm_kprobe(struct kprobe *p)
 {
-	unsigned long addr = (unsigned long) p->addr;
-	int page_readonly = 0;
-
-	if (kernel_readonly_text(addr)) {
-		page_readonly = 1;
-		change_page_attr(virt_to_page(addr), 1, PAGE_KERNEL_RWX);
-		global_flush_tlb();
-	}
-
+	kernel_text_lock((unsigned long)p->addr, sizeof(kprobe_opcode_t));
 	*p->addr = BREAKPOINT_INSTRUCTION;
+	kernel_text_unlock((unsigned long)p->addr, sizeof(kprobe_opcode_t));
 	flush_icache_range((unsigned long) p->addr,
 			   (unsigned long) p->addr + sizeof(kprobe_opcode_t));
-
-	if (page_readonly) {
-		change_page_attr(virt_to_page(addr), 1, PAGE_KERNEL_RX);
-		global_flush_tlb();
-	}
 }
 
 void __kprobes arch_disarm_kprobe(struct kprobe *p)
 {
-	unsigned long addr = (unsigned long) p->addr;
-	int page_readonly = 0;
-
-	if (kernel_readonly_text(addr)) {
-		page_readonly = 1;
-		change_page_attr(virt_to_page(addr), 1, PAGE_KERNEL_RWX);
-		global_flush_tlb();
-	}
+	kernel_text_lock((unsigned long)p->addr, sizeof(kprobe_opcode_t));
 	*p->addr = p->opcode;
+	kernel_text_unlock((unsigned long)p->addr, sizeof(kprobe_opcode_t));
 	flush_icache_range((unsigned long) p->addr,
 			   (unsigned long) p->addr + sizeof(kprobe_opcode_t));
-	if (page_readonly) {
-		change_page_attr(virt_to_page(addr), 1, PAGE_KERNEL_RX);
-		global_flush_tlb();
-	}
 }
 
 void __kprobes arch_remove_kprobe(struct kprobe *p)

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

Re: [patch 3/3] Text Edit Lock - kprobes i386

[S. P. Prasanna]

On Tue, Jul 03, 2007 at 12:38:22PM -0400, Mathieu Desnoyers wrote:
> Kprobes can use the text edit lock to insure mutual exclusion when edition the
> code and make sure the pages are writable.

Linus suggested for splitting ro-data and ro-text; And allow ro-text
only if kprobes is not configured.
Please see the discussion thread, URL given below
http://lkml.org/lkml/2007/6/20/436

This patch below allows to configure and mark the kernel text and
kernel data as read-only separately. Also kernel text
is configured read-only if kprobes is not configured.

Thanks
Prasanna

This patch allows to configure and mark the kernel text and
kernel data as read-only separately.

Signed-off-by: Prasanna S P. <prasanna@in.ibm.com>


 arch/i386/Kconfig.debug |    8 ++++++++
 arch/i386/mm/init.c     |   22 ++++++++++++++++------
 2 files changed, 24 insertions(+), 6 deletions(-)

diff -puN arch/i386/Kconfig.debug~mark-kernel-text-data-ro-seperately-i386 arch/i386/Kconfig.debug
--- linux-2.6.22-rc6/arch/i386/Kconfig.debug~mark-kernel-text-data-ro-seperately-i386	2007-07-04 13:45:24.000000000 +0530
+++ linux-2.6.22-rc6-prasanna/arch/i386/Kconfig.debug	2007-07-04 13:52:31.000000000 +0530
@@ -56,6 +56,14 @@ config DEBUG_RODATA
 	  portion of the kernel code won't be covered by a 2MB TLB anymore.
 	  If in doubt, say "N".
 
+config DEBUG_ROTEXT
+	bool "Write protect kernel text"
+	depends on DEBUG_RODATA && !KPROBES
+	help
+	  Mark the kernel text as write-protected in the pagetables.
+	  Only allow this if kprobes is not configured.
+	  If in doubt, say "N".
+
 config 4KSTACKS
 	bool "Use 4Kb for kernel stacks instead of 8Kb"
 	depends on DEBUG_KERNEL
diff -puN arch/i386/mm/init.c~mark-kernel-text-data-ro-seperately-i386 arch/i386/mm/init.c
--- linux-2.6.22-rc6/arch/i386/mm/init.c~mark-kernel-text-data-ro-seperately-i386	2007-07-04 13:45:24.000000000 +0530
+++ linux-2.6.22-rc6-prasanna/arch/i386/mm/init.c	2007-07-04 13:51:39.000000000 +0530
@@ -792,14 +792,11 @@ static int noinline do_test_wp_bit(void)
 	return flag;
 }
 
-#ifdef CONFIG_DEBUG_RODATA
-
-void mark_rodata_ro(void)
+static inline void mark_rwtext_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)
@@ -809,9 +806,22 @@ void mark_rodata_ro(void)
 		                 size >> PAGE_SHIFT, PAGE_KERNEL_RX);
 		printk("Write protecting the kernel text: %luk\n", size >> 10);
 	}
+
+	/*
+	 * global_flush_tlb() will be called after marking the data as readonly.
+	 */
+}
+
+#ifdef CONFIG_DEBUG_RODATA
+
+void mark_rodata_ro(void)
+{
+	unsigned long start = PFN_ALIGN(_etext);
+	unsigned long size = (unsigned long)__end_rodata - start;
+
+#ifdef CONFIG_DEBUG_ROTEXT
+	mark_rwtext_ro();
 #endif
-	start += size;
-	size = (unsigned long)__end_rodata - start;
 	change_page_attr(virt_to_page(start),
 	                 size >> PAGE_SHIFT, PAGE_KERNEL_RO);
 	printk("Write protecting the kernel read-only data: %luk\n",

_
-- 
Prasanna S.P.
Linux Technology Center
India Software Labs, IBM Bangalore
Email: prasanna@in.ibm.com
Ph: 91-80-41776329

Re: [patch 3/3] Text Edit Lock - kprobes i386

[Mathieu Desnoyers]

This suggestion looks more like a workaround to get rid of a bigger
issue.

If we do that, we have to use CONFIG_* exclusivity for either 
(ro-data protection) or (paravirt, kprobes, immediate values).

One big advantage of my patch is that we can remove weird exclusivity
cases involving paravirt and cpu hotplug, like this one:

-#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);
-               kernel_text_is_ro = 1;
-       }

Mathieu

* S. P. Prasanna (prasanna@in.ibm.com) wrote:
> On Tue, Jul 03, 2007 at 12:38:22PM -0400, Mathieu Desnoyers wrote:
> > Kprobes can use the text edit lock to insure mutual exclusion when edition the
> > code and make sure the pages are writable.
> 
> Linus suggested for splitting ro-data and ro-text; And allow ro-text
> only if kprobes is not configured.
> Please see the discussion thread, URL given below
> http://lkml.org/lkml/2007/6/20/436
> 
> This patch below allows to configure and mark the kernel text and
> kernel data as read-only separately. Also kernel text
> is configured read-only if kprobes is not configured.
> 
> Thanks
> Prasanna
> 
> This patch allows to configure and mark the kernel text and
> kernel data as read-only separately.
> 
> Signed-off-by: Prasanna S P. <prasanna@in.ibm.com>
> 
> 
>  arch/i386/Kconfig.debug |    8 ++++++++
>  arch/i386/mm/init.c     |   22 ++++++++++++++++------
>  2 files changed, 24 insertions(+), 6 deletions(-)
> 
> diff -puN arch/i386/Kconfig.debug~mark-kernel-text-data-ro-seperately-i386 arch/i386/Kconfig.debug
> --- linux-2.6.22-rc6/arch/i386/Kconfig.debug~mark-kernel-text-data-ro-seperately-i386	2007-07-04 13:45:24.000000000 +0530
> +++ linux-2.6.22-rc6-prasanna/arch/i386/Kconfig.debug	2007-07-04 13:52:31.000000000 +0530
> @@ -56,6 +56,14 @@ config DEBUG_RODATA
>  	  portion of the kernel code won't be covered by a 2MB TLB anymore.
>  	  If in doubt, say "N".
>  
> +config DEBUG_ROTEXT
> +	bool "Write protect kernel text"
> +	depends on DEBUG_RODATA && !KPROBES
> +	help
> +	  Mark the kernel text as write-protected in the pagetables.
> +	  Only allow this if kprobes is not configured.
> +	  If in doubt, say "N".
> +
>  config 4KSTACKS
>  	bool "Use 4Kb for kernel stacks instead of 8Kb"
>  	depends on DEBUG_KERNEL
> diff -puN arch/i386/mm/init.c~mark-kernel-text-data-ro-seperately-i386 arch/i386/mm/init.c
> --- linux-2.6.22-rc6/arch/i386/mm/init.c~mark-kernel-text-data-ro-seperately-i386	2007-07-04 13:45:24.000000000 +0530
> +++ linux-2.6.22-rc6-prasanna/arch/i386/mm/init.c	2007-07-04 13:51:39.000000000 +0530
> @@ -792,14 +792,11 @@ static int noinline do_test_wp_bit(void)
>  	return flag;
>  }
>  
> -#ifdef CONFIG_DEBUG_RODATA
> -
> -void mark_rodata_ro(void)
> +static inline void mark_rwtext_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)
> @@ -809,9 +806,22 @@ void mark_rodata_ro(void)
>  		                 size >> PAGE_SHIFT, PAGE_KERNEL_RX);
>  		printk("Write protecting the kernel text: %luk\n", size >> 10);
>  	}
> +
> +	/*
> +	 * global_flush_tlb() will be called after marking the data as readonly.
> +	 */
> +}
> +
> +#ifdef CONFIG_DEBUG_RODATA
> +
> +void mark_rodata_ro(void)
> +{
> +	unsigned long start = PFN_ALIGN(_etext);
> +	unsigned long size = (unsigned long)__end_rodata - start;
> +
> +#ifdef CONFIG_DEBUG_ROTEXT
> +	mark_rwtext_ro();
>  #endif
> -	start += size;
> -	size = (unsigned long)__end_rodata - start;
>  	change_page_attr(virt_to_page(start),
>  	                 size >> PAGE_SHIFT, PAGE_KERNEL_RO);
>  	printk("Write protecting the kernel read-only data: %luk\n",
> 
> _
> -- 
> Prasanna S.P.
> Linux Technology Center
> India Software Labs, IBM Bangalore
> Email: prasanna@in.ibm.com
> Ph: 91-80-41776329

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

Re: [patch 2/3] Text Edit Lock - Alternative i386

[Andrew Morton]

On Tue, 03 Jul 2007 12:38:21 -0400
Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca> wrote:

> alternative.c can use the text edit lock to remove CPU HOTPLUG special cases.
> 
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> ---
>  arch/i386/kernel/alternative.c |    5 +++++

x86_64 uses this file as well, so the patchset breaks the x86-64 build.

Look, x86 is just a minefield.  please compile-test your changes much
better than you have been doing.  That means *at least* allnoconfig,
allyesconfig and allmodconfig on i386 and x86_64.

Thanks.