Re: [RFC PATCH] powerpc/lib: Fixing use a temporary mm for code patching
From: Christopher M Riedl <hidden>
Date: 2020-04-15 05:25:00
Also in:
lkml
quoted hunk ↗ jump to hunk
On March 26, 2020 9:42 AM Christophe Leroy [off-list ref] wrote: This patch fixes the RFC series identified below. It fixes three points: - Failure with CONFIG_PPC_KUAP - Failure to write do to lack of DIRTY bit set on the 8xx - Inadequaly complex WARN post verification However, it has an impact on the CPU load. Here is the time needed on an 8xx to run the ftrace selftests without and with this series: - Without CONFIG_STRICT_KERNEL_RWX ==> 38 seconds - With CONFIG_STRICT_KERNEL_RWX ==> 40 seconds - With CONFIG_STRICT_KERNEL_RWX + this series ==> 43 seconds Link: https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=166003 Signed-off-by: Christophe Leroy <redacted> --- arch/powerpc/lib/code-patching.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c index f156132e8975..4ccff427592e 100644 --- a/arch/powerpc/lib/code-patching.c +++ b/arch/powerpc/lib/code-patching.c@@ -97,6 +97,7 @@ static int map_patch(const void *addr, struct patch_mapping *patch_mapping) } pte = mk_pte(page, pgprot); + pte = pte_mkdirty(pte); set_pte_at(patching_mm, patching_addr, ptep, pte); init_temp_mm(&patch_mapping->temp_mm, patching_mm);@@ -168,7 +169,9 @@ static int do_patch_instruction(unsigned int *addr, unsigned int instr) (offset_in_page((unsigned long)addr) / sizeof(unsigned int)); + allow_write_to_user(patch_addr, sizeof(instr)); __patch_instruction(addr, instr, patch_addr); + prevent_write_to_user(patch_addr, sizeof(instr));
On radix we can map the page with PAGE_KERNEL protection which ends up setting EAA[0] in the radix PTE. This means the KUAP (AMR) protection is ignored (ISA v3.0b Fig. 35) since we are accessing the page from MSR[PR]=0. Can we employ a similar approach on the 8xx? I would prefer *not* to wrap the __patch_instruction() with the allow_/prevent_write_to_user() KUAP things because this is a temporary kernel mapping which really isn't userspace in the usual sense.
quoted hunk ↗ jump to hunk
err = unmap_patch(&patch_mapping); if (err)@@ -179,7 +182,7 @@ static int do_patch_instruction(unsigned int *addr, unsigned int instr) * think we just wrote. * XXX: BUG_ON() instead? */ - WARN_ON(memcmp(addr, &instr, sizeof(instr))); + WARN_ON(*addr != instr); out: local_irq_restore(flags);-- 2.25.0