x86 Fix text_poke for vmalloced pages

The shadow vmap for DEBUG_RODATA kernel text modification uses virt_to_page to get the pages from the pointer address.

However, I think vmalloc_to_page would be required in case the page is used for modules.

Since only the core kernel text is marked read-only, use core_kernel_text() to make sure we only shadow map the core kernel text, not modules.

This is an incremental change to make the DEBUG_RODATA and text_poke play together nicely. A future step will be to make the module text read-only too, which will require changes to load module, module free and text_poke. The idea is to fix the current x86 git tree quickly.

arch/x86/kernel/alternative.c +1 -0

+	if (core_kernel_text((unsigned long)addr)) {
 		struct page *pages[2] = { virt_to_page(addr),
 			virt_to_page(addr + PAGE_SIZE) };
 		if (!pages[1])

Quoted 20 lines · Mathieu Desnoyers

The shadow vmap for DEBUG_RODATA kernel text modification uses virt_to_page to get the pages from the pointer address.

However, I think vmalloc_to_page would be required in case the page is used for modules.

Since only the core kernel text is marked read-only, use core_kernel_text() to make sure we only shadow map the core kernel text, not modules.

This is an incremental change to make the DEBUG_RODATA and text_poke play together nicely. A future step will be to make the module text read-only too, which will require changes to load module, module free and text_poke. The idea is to fix the current x86 git tree quickly.

(quoted diff) +1 -0

+	if (core_kernel_text((unsigned long)addr)) {
 		struct page *pages[2] = { virt_to_page(addr),
 			virt_to_page(addr + PAGE_SIZE) };
 		if (!pages[1])

Review void *__kprobes text_poke(void *addr, co · line 531

Quoted 10 lines

(quoted diff) +7 -0

@@ -522,6 +522,13 @@ void *__kprobes text_poke(void *addr, co
 		memcpy(&vaddr[(unsigned long)addr & ~PAGE_MASK], opcode, len);
 		local_irq_restore(flags);
 		vunmap(vaddr);
+	} else {
+		/*
+		 * modules are in vmalloc'ed memory, always writable.
+		 */
+		local_irq_save(flags);
+		memcpy(addr, opcode, len);
+		local_irq_restore(flags);

hm, this looks ugly, and the whole text_poke() function looks ugly. For example why the extra code block + indentation here:

(quoted diff) +2 -0

+void *__kprobes text_poke(void *addr, const void *opcode, size_t len)
+{

also, more fundamentally - why not introduce a proper, generic "look up kernel text struct page *" method, instead of open-coding various assumptions about which kernel text is readonly and which isnt?

Ingo