From mboxrd@z Thu Jan 1 00:00:00 1970 From: mark.rutland@arm.com (Mark Rutland) Date: Tue, 25 Nov 2014 17:04:45 +0000 Subject: [PATCHv6 6/8] arm64: use fixmap for text patching when text is RO In-Reply-To: <1416606645-25633-7-git-send-email-lauraa@codeaurora.org> References: <1416606645-25633-1-git-send-email-lauraa@codeaurora.org> <1416606645-25633-7-git-send-email-lauraa@codeaurora.org> Message-ID: <20141125170445.GB21525@leverpostej> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Laura, It looks like "early" means before we've set up the strict page permissions, though as far as I can tell the fixmap will already be available before we do any patching. Could we not always use the fixmap for patching? Then we only need the patch_{map,unmap} additions, and not the changes to distinguish the early cases. >>From testing on Juno with defconfig, all of the early patches were avoidable NOP -> NOP changes as part of static key initialisation, which I think we can skip similarly to x86 (I'll send a patch shortly). All other patches were not early and went via the fixmap. Even with the avoidable NOP -> NOP patching I did not see a noticeable boot time difference from forcing the use of the fixmap. Thanks, Mark. On Fri, Nov 21, 2014 at 09:50:43PM +0000, Laura Abbott wrote: > When kernel text is marked as read only, it cannot be modified directly. > Use a fixmap to modify the text instead in a similar manner to > x86 and arm. > > Reviewed-by: Kees Cook > Tested-by: Kees Cook > Signed-off-by: Laura Abbott > --- > arch/arm64/include/asm/fixmap.h | 1 + > arch/arm64/include/asm/insn.h | 2 ++ > arch/arm64/kernel/insn.c | 72 +++++++++++++++++++++++++++++++++++++++-- > arch/arm64/kernel/jump_label.c | 2 +- > 4 files changed, 73 insertions(+), 4 deletions(-) > > diff --git a/arch/arm64/include/asm/fixmap.h b/arch/arm64/include/asm/fixmap.h > index db26a2f2..2cd4b0d 100644 > --- a/arch/arm64/include/asm/fixmap.h > +++ b/arch/arm64/include/asm/fixmap.h > @@ -48,6 +48,7 @@ enum fixed_addresses { > > FIX_BTMAP_END = __end_of_permanent_fixed_addresses, > FIX_BTMAP_BEGIN = FIX_BTMAP_END + TOTAL_FIX_BTMAPS - 1, > + FIX_TEXT_POKE0, > __end_of_fixed_addresses > }; > > diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h > index 56a9e63..f66853b 100644 > --- a/arch/arm64/include/asm/insn.h > +++ b/arch/arm64/include/asm/insn.h > @@ -282,6 +282,7 @@ bool aarch64_insn_is_nop(u32 insn); > > int aarch64_insn_read(void *addr, u32 *insnp); > int aarch64_insn_write(void *addr, u32 insn); > +int aarch64_insn_write_early(void *addr, u32 insn); > enum aarch64_insn_encoding_class aarch64_get_insn_class(u32 insn); > u32 aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type, > u32 insn, u64 imm); > @@ -352,6 +353,7 @@ u32 aarch64_insn_gen_logical_shifted_reg(enum aarch64_insn_register dst, > bool aarch64_insn_hotpatch_safe(u32 old_insn, u32 new_insn); > > int aarch64_insn_patch_text_nosync(void *addr, u32 insn); > +int __aarch64_insn_patch_text_nosync(void *addr, u32 insn, bool early); > int aarch64_insn_patch_text_sync(void *addrs[], u32 insns[], int cnt); > int aarch64_insn_patch_text(void *addrs[], u32 insns[], int cnt); > #endif /* __ASSEMBLY__ */ > diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c > index 8cd27fe..b2cad38 100644 > --- a/arch/arm64/kernel/insn.c > +++ b/arch/arm64/kernel/insn.c > @@ -19,12 +19,15 @@ > #include > #include > #include > +#include > #include > +#include > #include > #include > > #include > #include > +#include > #include > > #define AARCH64_INSN_SF_BIT BIT(31) > @@ -72,6 +75,36 @@ bool __kprobes aarch64_insn_is_nop(u32 insn) > } > } > > +static DEFINE_SPINLOCK(patch_lock); > + > +static void __kprobes *patch_map(void *addr, int fixmap, unsigned long *flags) > +{ > + unsigned long 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); > +} > /* > * In ARMv8-A, A64 instructions have a fixed length of 32 bits and are always > * little-endian. > @@ -88,10 +121,34 @@ int __kprobes aarch64_insn_read(void *addr, u32 *insnp) > return ret; > } > > +static int __kprobes __aarch64_insn_write(void *addr, u32 insn, bool patch) > +{ > + void *waddr = addr; > + unsigned long flags; > + int ret; > + > + if (patch) > + waddr = patch_map(addr, FIX_TEXT_POKE0, &flags); > + > + ret = probe_kernel_write(waddr, &insn, AARCH64_INSN_SIZE); > + > + if (waddr != addr) > + patch_unmap(FIX_TEXT_POKE0, &flags); > + > + return ret; > +} > + > int __kprobes aarch64_insn_write(void *addr, u32 insn) > { > insn = cpu_to_le32(insn); > - return probe_kernel_write(addr, &insn, AARCH64_INSN_SIZE); > + return __aarch64_insn_write(addr, insn, true); > +} > + > +int __kprobes aarch64_insn_write_early(void *addr, u32 insn) > +{ > + insn = cpu_to_le32(insn); > + return __aarch64_insn_write(addr, insn, false); > + > } > > static bool __kprobes __aarch64_insn_hotpatch_safe(u32 insn) > @@ -124,7 +181,7 @@ bool __kprobes aarch64_insn_hotpatch_safe(u32 old_insn, u32 new_insn) > __aarch64_insn_hotpatch_safe(new_insn); > } > > -int __kprobes aarch64_insn_patch_text_nosync(void *addr, u32 insn) > +int __kprobes __aarch64_insn_patch_text_nosync(void *addr, u32 insn, bool early) > { > u32 *tp = addr; > int ret; > @@ -133,7 +190,11 @@ int __kprobes aarch64_insn_patch_text_nosync(void *addr, u32 insn) > if ((uintptr_t)tp & 0x3) > return -EINVAL; > > - ret = aarch64_insn_write(tp, insn); > + if (early) > + ret = aarch64_insn_write_early(tp, insn); > + else > + ret = aarch64_insn_write(tp, insn); > + > if (ret == 0) > flush_icache_range((uintptr_t)tp, > (uintptr_t)tp + AARCH64_INSN_SIZE); > @@ -141,6 +202,11 @@ int __kprobes aarch64_insn_patch_text_nosync(void *addr, u32 insn) > return ret; > } > > +int __kprobes aarch64_insn_patch_text_nosync(void *addr, u32 insn) > +{ > + return __aarch64_insn_patch_text_nosync(addr, insn, false); > +} > + > struct aarch64_insn_patch { > void **text_addrs; > u32 *new_insns; > diff --git a/arch/arm64/kernel/jump_label.c b/arch/arm64/kernel/jump_label.c > index 263a166..9ac30bb 100644 > --- a/arch/arm64/kernel/jump_label.c > +++ b/arch/arm64/kernel/jump_label.c > @@ -38,7 +38,7 @@ static void __arch_jump_label_transform(struct jump_entry *entry, > } > > if (is_static) > - aarch64_insn_patch_text_nosync(addr, insn); > + __aarch64_insn_patch_text_nosync(addr, insn, true); > else > aarch64_insn_patch_text(&addr, &insn, 1); > } > -- > Qualcomm Innovation Center, Inc. > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project > >