> On March 26, 2020 9:42 AM Christophe Leroy <christophe.le...@c-s.fr> 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 <christophe.le...@c-s.fr> > --- > 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. > 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
