| Message ID | 20191002094935.48848-5-james.morse@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: errata: Workaround Neoverse-N1 #1542419 | expand |
On Wed, Oct 02, 2019 at 10:49:35AM +0100, James Morse wrote: > CPUs affected by Neoverse-N1 #1542419 may execute a stale instruction if > it was recently modified. The affected sequence requires freshly written > instructions to be executable before a branch to them is updated. > > There are very few places in the kernel that modify executable text, > all but one come with sufficient synchronisation: > * The module loader's flush_module_icache() calls flush_icache_range(), > which does a kick_all_cpus_sync() > * bpf_int_jit_compile() calls flush_icache_range(). > * Kprobes calls aarch64_insn_patch_text(), which does its work in > stop_machine(). > * static keys and ftrace both patch between nops and branches to > existing kernel code (not generated code). > > The affected sequence is the interaction between ftrace and modules. > The module PLT is cleaned using __flush_icache_range() as the trampoline > shouldn't be executable until we update the branch to it. > > Drop the double-underscore so that this path runs kick_all_cpus_sync() > too. > > Signed-off-by: James Morse <james.morse@arm.com> > --- > arch/arm64/kernel/ftrace.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) I'll take this one as a fix, since it's not reliant on a firmware update. One other thing -- please can you update silicon-errata.rst as part of the other patches? Will
Hi Will, On 04/10/2019 11:33, Will Deacon wrote: > On Wed, Oct 02, 2019 at 10:49:35AM +0100, James Morse wrote: >> CPUs affected by Neoverse-N1 #1542419 may execute a stale instruction if >> it was recently modified. The affected sequence requires freshly written >> instructions to be executable before a branch to them is updated. >> >> There are very few places in the kernel that modify executable text, >> all but one come with sufficient synchronisation: >> * The module loader's flush_module_icache() calls flush_icache_range(), >> which does a kick_all_cpus_sync() >> * bpf_int_jit_compile() calls flush_icache_range(). >> * Kprobes calls aarch64_insn_patch_text(), which does its work in >> stop_machine(). >> * static keys and ftrace both patch between nops and branches to >> existing kernel code (not generated code). >> >> The affected sequence is the interaction between ftrace and modules. >> The module PLT is cleaned using __flush_icache_range() as the trampoline >> shouldn't be executable until we update the branch to it. >> >> Drop the double-underscore so that this path runs kick_all_cpus_sync() >> too. > I'll take this one as a fix, since it's not reliant on a firmware update. > One other thing -- please can you update silicon-errata.rst as part of the > other patches? Gah, bother. I'll fix that and repost this for rc2, noting its for v5.5. Thanks, James
diff --git a/arch/arm64/kernel/ftrace.c b/arch/arm64/kernel/ftrace.c index 171773257974..06e56b470315 100644 --- a/arch/arm64/kernel/ftrace.c +++ b/arch/arm64/kernel/ftrace.c @@ -121,10 +121,16 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr) /* * Ensure updated trampoline is visible to instruction - * fetch before we patch in the branch. + * fetch before we patch in the branch. Although the + * architecture doesn't require an IPI in this case, + * Neoverse-N1 erratum #1542419 does require one + * if the TLB maintenance in module_enable_ro() is + * skipped due to rodata_enabled. It doesn't seem worth + * it to make it conditional given that this is + * certainly not a fast-path. */ - __flush_icache_range((unsigned long)&dst[0], - (unsigned long)&dst[1]); + flush_icache_range((unsigned long)&dst[0], + (unsigned long)&dst[1]); } addr = (unsigned long)dst; #else /* CONFIG_ARM64_MODULE_PLTS */
CPUs affected by Neoverse-N1 #1542419 may execute a stale instruction if it was recently modified. The affected sequence requires freshly written instructions to be executable before a branch to them is updated. There are very few places in the kernel that modify executable text, all but one come with sufficient synchronisation: * The module loader's flush_module_icache() calls flush_icache_range(), which does a kick_all_cpus_sync() * bpf_int_jit_compile() calls flush_icache_range(). * Kprobes calls aarch64_insn_patch_text(), which does its work in stop_machine(). * static keys and ftrace both patch between nops and branches to existing kernel code (not generated code). The affected sequence is the interaction between ftrace and modules. The module PLT is cleaned using __flush_icache_range() as the trampoline shouldn't be executable until we update the branch to it. Drop the double-underscore so that this path runs kick_all_cpus_sync() too. Signed-off-by: James Morse <james.morse@arm.com> --- arch/arm64/kernel/ftrace.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)