[PATCH] arm64: patching: avoid early page_to_phys()

Mark Rutland mark.rutland at arm.com
Mon Dec 2 09:03:59 PST 2024


When arm64 is configured with CONFIG_DEBUG_VIRTUAL=y, a warning is
printed from the patching code because patch_map(), e.g.

| ------------[ cut here ]------------
| WARNING: CPU: 0 PID: 0 at arch/arm64/kernel/patching.c:45 patch_map.constprop.0+0x120/0xd00
| CPU: 0 UID: 0 PID: 0 Comm: swapper Not tainted 6.13.0-rc1-00002-ge1a5d6c6be55 #1
| Hardware name: linux,dummy-virt (DT)
| pstate: 800003c5 (Nzcv DAIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
| pc : patch_map.constprop.0+0x120/0xd00
| lr : patch_map.constprop.0+0x120/0xd00
| sp : ffffa9bb312a79a0
| x29: ffffa9bb312a79a0 x28: 0000000000000001 x27: 0000000000000001
| x26: 0000000000000000 x25: 0000000000000000 x24: 00000000000402e8
| x23: ffffa9bb2c94c1c8 x22: ffffa9bb2c94c000 x21: ffffa9bb222e883c
| x20: 0000000000000002 x19: ffffc1ffc100ba40 x18: ffffa9bb2cf0f21c
| x17: 0000000000000006 x16: 0000000000000000 x15: 0000000000000004
| x14: 1ffff5376625b4ac x13: ffff753766a67fb8 x12: ffff753766919cd1
| x11: 0000000000000003 x10: 1ffff5376625b4c3 x9 : 1ffff5376625b4af
| x8 : ffff753766254f0a x7 : 0000000041b58ab3 x6 : ffff753766254f18
| x5 : ffffa9bb312d9bc0 x4 : 0000000000000000 x3 : ffffa9bb29bd90e4
| x2 : 0000000000000002 x1 : ffffa9bb312d9bc0 x0 : 0000000000000000
| Call trace:
|  patch_map.constprop.0+0x120/0xd00 (P)
|  patch_map.constprop.0+0x120/0xd00 (L)
|  __aarch64_insn_write+0xa8/0x120
|  aarch64_insn_patch_text_nosync+0x4c/0xb8
|  arch_jump_label_transform_queue+0x7c/0x100
|  jump_label_update+0x154/0x460
|  static_key_enable_cpuslocked+0x1d8/0x280
|  static_key_enable+0x2c/0x48
|  early_randomize_kstack_offset+0x104/0x168
|  do_early_param+0xe4/0x148
|  parse_args+0x3a4/0x838
|  parse_early_options+0x50/0x68
|  parse_early_param+0x58/0xe0
|  setup_arch+0x78/0x1f0
|  start_kernel+0xa0/0x530
|  __primary_switched+0x8c/0xa0
| irq event stamp: 0
| hardirqs last  enabled at (0): [<0000000000000000>] 0x0
| hardirqs last disabled at (0): [<0000000000000000>] 0x0
| softirqs last  enabled at (0): [<0000000000000000>] 0x0
| softirqs last disabled at (0): [<0000000000000000>] 0x0
| ---[ end trace 0000000000000000 ]---

The warning has been produced since commit:

  3e25d5a49f99b75b ("asm-generic: add an optional pfn_valid check to page_to_phys")

... which added a pfn_valid() check into page_to_phys(), and at this
point in boot pfn_valid() will always return false because the vmemmap
has not yet been initialized and there are no valid mem_sections yet.

Before that commit, the arithmetic performed by page_to_phys() would
give the expected physical address, though it is somewhat dubious to use
vmemmap addresses before the vmemmap has been initialized.

For historical reasons, the structure of patch_map() is more complicated
than necessary and can be simplified. For kernel image addresses it's
sufficient to use __pa_symbol() directly without converting this to a
page address and back. Aside from kernel image addresses, all executable
code should be allocated from execmem (where all allocations will fall
within the vmalloc area), and the vmalloc area), and so there's no need
for the fallback case when case when CONFIG_EXECMEM=n.

Simplify patch_map() accordingly, directly converting kernel image
addresses and removing the redundant fallback case.

Fixes: 3e25d5a49f99b75b ("asm-generic: add an optional pfn_valid check to page_to_phys")
Signed-off-by: Mark Rutland <mark.rutland at arm.com>
Cc: Arnd Bergmann <arnd at arndb.de>
Cc: Catalin Marinas <catalin.marinas at arm.com>
Cc: Christoph Hellwig <hch at lst.de>
Cc: Mike Rapoport <rppt at kernel.org>
Cc: Thomas Huth <thuth at redhat.com>
Cc: Will Deacon <will at kernel.org>
---
 arch/arm64/kernel/patching.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

Catalin, Will, I wasn't sure whether we'd prefer this or an explicit
check that the address is a vmalloc addr, e.g.

	if (is_image_text(...)) {
		phys = ...;
	} else if (is_vmalloc_addr(...)) {
		phys = ...;
	} else {
		BUG();
	}

I went for the style below because it was marginally simpler, I'm more
than happy to respin as above if that's preferable.

Mark.

diff --git a/arch/arm64/kernel/patching.c b/arch/arm64/kernel/patching.c
index 7f99723fbb8c4..1041bc67a3eee 100644
--- a/arch/arm64/kernel/patching.c
+++ b/arch/arm64/kernel/patching.c
@@ -30,20 +30,17 @@ static bool is_image_text(unsigned long addr)
 
 static void __kprobes *patch_map(void *addr, int fixmap)
 {
-	unsigned long uintaddr = (uintptr_t) addr;
-	bool image = is_image_text(uintaddr);
-	struct page *page;
-
-	if (image)
-		page = phys_to_page(__pa_symbol(addr));
-	else if (IS_ENABLED(CONFIG_EXECMEM))
-		page = vmalloc_to_page(addr);
-	else
-		return addr;
-
-	BUG_ON(!page);
-	return (void *)set_fixmap_offset(fixmap, page_to_phys(page) +
-			(uintaddr & ~PAGE_MASK));
+	phys_addr_t phys;
+
+	if (is_image_text((unsigned long)addr)) {
+		phys = __pa_symbol(addr);
+	} else {
+		struct page *page = vmalloc_to_page(addr);
+		BUG_ON(!page);
+		phys = page_to_phys(page) + offset_in_page(addr);
+	}
+
+	return (void *)set_fixmap_offset(fixmap, phys);
 }
 
 static void __kprobes patch_unmap(int fixmap)
-- 
2.30.2




More information about the linux-arm-kernel mailing list