Thread (15 messages) 15 messages, 2 authors, 2014-08-13

[PATCH v3 3/7] arm: use fixmap for text patching when text is RO

From: Stephen Boyd <hidden>
Date: 2014-08-13 00:27:52
Also in: lkml
Subsystem: arm port, the rest · Maintainers: Russell King, Linus Torvalds

On 08/12/14 14:47, Kees Cook wrote:
On Tue, Aug 12, 2014 at 2:39 PM, Stephen Boyd [off-list ref] wrote:
quoted
On 08/12/14 11:24, Kees Cook wrote:
quoted
diff --git a/arch/arm/kernel/patch.c b/arch/arm/kernel/patch.c
index 07314af47733..03dd4e39c833 100644
--- a/arch/arm/kernel/patch.c
+++ b/arch/arm/kernel/patch.c
@@ -13,21 +16,69 @@ struct patch {
      unsigned int insn;
 };

-void __kprobes __patch_text(void *addr, unsigned int insn)
+static DEFINE_SPINLOCK(patch_lock);
+
+static void __kprobes *patch_map(void *addr, int fixmap, unsigned long *flags)
+{
+     unsigned int uintaddr = (uintptr_t) addr;
+     bool module = !core_kernel_text(uintaddr);
+     struct page *page;
+
+     if (module && IS_ENABLED(CONFIG_DEBUG_SET_MODULE_RONX))
+             page = vmalloc_to_page(addr);
+     else if (!module && IS_ENABLED(CONFIG_DEBUG_RODATA))
+             page = virt_to_page(addr);
+     else
+             return addr;
+
+     if (flags)
+             spin_lock_irqsave(&patch_lock, *flags);
+
+     set_fixmap(fixmap, page_to_phys(page));
+
+     return (void *) (__fix_to_virt(fixmap) + (uintaddr & ~PAGE_MASK));
+}
+
+static void __kprobes patch_unmap(int fixmap, unsigned long *flags)
+{
+     clear_fixmap(fixmap);
+
+     if (flags)
+             spin_unlock_irqrestore(&patch_lock, *flags);
+}
Has the kbuildbot complained about this one yet?

  CHECK  arch/arm/kernel/patch.c
  arch/arm/kernel/patch.c:47:39: warning: context imbalance in
'patch_unmap' - unexpected unlock

I guess we're going to ignore it.
No, nothing yet from buildbot, let me do a sparse run -- I think we
can just add annotation and we'll be okay.
Ok. I tried to move code around but sparse still complains because of
conditional locking.

---8<---
diff --git a/arch/arm/kernel/patch.c b/arch/arm/kernel/patch.c
index 03dd4e39c833..f6d4de3826a0 100644
--- a/arch/arm/kernel/patch.c
+++ b/arch/arm/kernel/patch.c
@@ -18,33 +18,17 @@ struct patch {
 
 static DEFINE_SPINLOCK(patch_lock);
 
-static void __kprobes *patch_map(void *addr, int fixmap, unsigned long *flags)
+static struct page __kprobes *patch_page(void *addr)
 {
 	unsigned int uintaddr = (uintptr_t) addr;
 	bool module = !core_kernel_text(uintaddr);
-	struct page *page;
 
 	if (module && IS_ENABLED(CONFIG_DEBUG_SET_MODULE_RONX))
-		page = vmalloc_to_page(addr);
+		return vmalloc_to_page(addr);
 	else if (!module && IS_ENABLED(CONFIG_DEBUG_RODATA))
-		page = virt_to_page(addr);
-	else
-		return addr;
+		return virt_to_page(addr);
 
-	if (flags)
-		spin_lock_irqsave(&patch_lock, *flags);
-
-	set_fixmap(fixmap, page_to_phys(page));
-
-	return (void *) (__fix_to_virt(fixmap) + (uintaddr & ~PAGE_MASK));
-}
-
-static void __kprobes patch_unmap(int fixmap, unsigned long *flags)
-{
-	clear_fixmap(fixmap);
-
-	if (flags)
-		spin_unlock_irqrestore(&patch_lock, *flags);
+	return NULL;
 }
 
 void __kprobes __patch_text_real(void *addr, unsigned int insn, bool remap)
@@ -54,10 +38,18 @@ void __kprobes __patch_text_real(void *addr, unsigned int insn, bool remap)
 	bool twopage = false;
 	unsigned long flags;
 	void *waddr = addr;
+	struct page *page = NULL;
 	int size;
 
-	if (remap)
-		waddr = patch_map(addr, FIX_TEXT_POKE0, &flags);
+	if (remap) {
+		page = patch_page(addr);
+		if (page) {
+			spin_lock_irqsave(&patch_lock, flags);
+			set_fixmap(FIX_TEXT_POKE0, page_to_phys(page));
+			waddr = (void *)(__fix_to_virt(FIX_TEXT_POKE0) +
+				(uintaddr & ~PAGE_MASK));
+		}
+	}
 
 	if (thumb2 && __opcode_is_thumb16(insn)) {
 		*(u16 *)waddr = __opcode_to_mem_thumb16(insn);
@@ -69,15 +61,25 @@ void __kprobes __patch_text_real(void *addr, unsigned int insn, bool remap)
 		u16 *addrh1 = waddr + 2;
 
 		twopage = (uintaddr & ~PAGE_MASK) == PAGE_SIZE - 2;
-		if (twopage && remap)
-			addrh1 = patch_map(addr + 2, FIX_TEXT_POKE1, NULL);
+		if (twopage && remap) {
+			struct page *page2;
+
+			page2 = patch_page(addr + 2);
+			if (page2) {
+				set_fixmap(FIX_TEXT_POKE1, page_to_phys(page));
+				addrh1 = (u16 *)(__fix_to_virt(FIX_TEXT_POKE1) +
+						(uintaddr & ~PAGE_MASK));
+			} else {
+				addrh1 = addr + 2;
+			}
+		}
 
 		*addrh0 = __opcode_to_mem_thumb16(first);
 		*addrh1 = __opcode_to_mem_thumb16(second);
 
 		if (twopage && addrh1 != addr + 2) {
 			flush_kernel_vmap_range(addrh1, 2);
-			patch_unmap(FIX_TEXT_POKE1, NULL);
+			clear_fixmap(FIX_TEXT_POKE1);
 		}
 
 		size = sizeof(u32);
@@ -91,9 +93,10 @@ void __kprobes __patch_text_real(void *addr, unsigned int insn, bool remap)
 		size = sizeof(u32);
 	}
 
-	if (waddr != addr) {
+	if (page) {
 		flush_kernel_vmap_range(waddr, twopage ? size / 2 : size);
-		patch_unmap(FIX_TEXT_POKE0, &flags);
+		clear_fixmap(FIX_TEXT_POKE0);
+		spin_unlock_irqrestore(&patch_lock, flags);
 	}
 
 	flush_icache_range((uintptr_t)(addr),
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help