Skip to contentSkip to conversation

arm64/kernel: get rid of GCC large model code

PATCH New Ard Biesheuvel 3 participants 13 messages Mar 6, 2018 – Mar 8, 2018 arm64

lore
Share & tools
lore
#0 lore raw

v4 cover arm64/kernel: get rid of GCC large model code

GCC's large model uses literal pools to emit cross object symbol references rather than movz/movk sequences, resulting in data items mixed in the with executable code in modules' .text segments, reducing cache utilization, but also potentially resulting in the creation of code gadgets that are exploitable under speculative execution.

We are using GCC's large model for two separate reasons, both of which can be worked around rather easily:
- KASLR uses it to move modules and the kernel very far apart, which is not really needed,
- the Cortex-A53 erratum code uses it to avoid ADRP instruction altogether, which can be replaced by selective patching of only the ADRP instructions that are affected by the erratum

v4:
- dropped RFC prefix
- added patch #1 and #4
- use _AC() for new SIZE_4G constant with ULL prefix (#2)
- improve comments, get rid of BUG() when running out of PLT entries (#3)
- rely on new REVIDR checking code (#5)

Ard Biesheuvel (5):
arm64: module: don't BUG when exceeding preallocated PLT count arm64/kernel: kaslr: reduce module randomization range to 4 GB arm64/kernel: don't ban ADRP to work around Cortex-A53 erratum #843419 arm64/errata: add REVIDR handling to framework arm64/kernel: enable A53 erratum #8434319 handling at runtime

 arch/arm64/Kconfig                  | 18 ++--
 arch/arm64/Makefile                 |  5 --
 arch/arm64/include/asm/cpucaps.h    |  3 +-
 arch/arm64/include/asm/cpufeature.h |  4 +
 arch/arm64/include/asm/module.h     |  2 +
 arch/arm64/kernel/cpu_errata.c      | 31 ++++++-
 arch/arm64/kernel/kaslr.c           | 20 +++--
 arch/arm64/kernel/module-plts.c     | 90 +++++++++++++++++++-
 arch/arm64/kernel/module.c          | 42 +++++++--
 arch/arm64/kernel/reloc_test_core.c |  4 +-
 arch/arm64/kernel/reloc_test_syms.S | 12 ++-
 include/linux/sizes.h               |  4 +
 12 files changed, 191 insertions(+), 44 deletions(-)
Signature · 1 line
2.11.0
#1 lore raw

PATCH v4 1/5 arm64: module: don't BUG when exceeding preallocated PLT count

When PLTs are emitted at relocation time, we really should not exceed the number that we counted when parsing the relocation tables, and so currently, we BUG() on this condition. However, even though this is a clear bug in this particular piece of code, we can easily recover by failing to load the module.

So instead, return 0 from module_emit_plt_entry() if this condition occurs, which is not a valid kernel address, and can hence serve as a flag value that makes the relocation routine bail out.

---
 arch/arm64/kernel/module-plts.c | 3 ++-
 arch/arm64/kernel/module.c      | 2 ++
 2 files changed, 4 insertions(+), 1 deletion(-)
arch/arm64/kernel/module-plts.c +2 -1
index ea640f92fe5a..6bf07c602bd4 100644
@@ -36,7 +36,8 @@ u64 module_emit_plt_entry(struct module *mod, void *loc, const Elf64_Rela *rela,
 		return (u64)&plt[i - 1];
 
 	pltsec->plt_num_entries++;
-	BUG_ON(pltsec->plt_num_entries > pltsec->plt_max_entries);
+	if (WARN_ON(pltsec->plt_num_entries > pltsec->plt_max_entries))
+		return 0;
 
 	return (u64)&plt[i];
 }
arch/arm64/kernel/module.c +2 -0
index f469e0435903..c8c6c2828b79 100644
@@ -386,6 +386,8 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
 			if (IS_ENABLED(CONFIG_ARM64_MODULE_PLTS) &&
 			    ovf == -ERANGE) {
 				val = module_emit_plt_entry(me, loc, &rel[i], sym);
+				if (!val)
+					return -ENOEXEC;
 				ovf = reloc_insn_imm(RELOC_OP_PREL, loc, val, 2,
 						     26, AARCH64_INSN_IMM_26);
 			}
Signature · 1 line
2.11.0
#2 lore raw

PATCH v4 2/5 arm64/kernel: kaslr: reduce module randomization range to 4 GB

In reply to Ard Biesheuvel

We currently have to rely on the GCC large code model for KASLR for two distinct but related reasons:
- if we enable full randomization, modules will be loaded very far away from the core kernel, where they are out of range for ADRP instructions,
- even without full randomization, the fact that the 128 MB module region is now no longer fully reserved for kernel modules means that there is a very low likelihood that the normal bottom-up allocation of other vmalloc regions may collide, and use up the range for other things.

Large model code is suboptimal, given that each symbol reference involves a literal load that goes through the D-cache, reducing cache utilization. But more importantly, literals are not instructions but part of .text nonetheless, and hence mapped with executable permissions.

So let's get rid of our dependency on the large model for KASLR, by:
- reducing the full randomization range to 4 GB, thereby ensuring that ADRP references between modules and the kernel are always in range,
- reduce the spillover range to 4 GB as well, so that we fallback to a region that is still guaranteed to be in range
- move the randomization window of the core kernel to the middle of the VMALLOC space

Note that KASAN always uses the module region outside of the vmalloc space, so keep the kernel close to that if KASAN is enabled.

---
 arch/arm64/Kconfig         |  7 +++----
 arch/arm64/kernel/kaslr.c  | 20 ++++++++++++--------
 arch/arm64/kernel/module.c |  7 ++++---
 include/linux/sizes.h      |  4 ++++
 4 files changed, 23 insertions(+), 15 deletions(-)
arch/arm64/Kconfig +3 -4
index 7381eeb7ef8e..ae7d3d4c0bbe 100644
@@ -1109,7 +1109,6 @@ config ARM64_MODULE_CMODEL_LARGE
 
 config ARM64_MODULE_PLTS
 	bool
-	select ARM64_MODULE_CMODEL_LARGE
 	select HAVE_MOD_ARCH_SPECIFIC
 
 config RELOCATABLE
@@ -1143,12 +1142,12 @@ config RANDOMIZE_BASE
 	  If unsure, say N.
 
 config RANDOMIZE_MODULE_REGION_FULL
-	bool "Randomize the module region independently from the core kernel"
+	bool "Randomize the module region over a 4 GB range"
 	depends on RANDOMIZE_BASE
 	default y
 	help
-	  Randomizes the location of the module region without considering the
-	  location of the core kernel. This way, it is impossible for modules
+	  Randomizes the location of the module region inside a 4 GB window
+	  covering the core kernel. This way, it is less likely for modules
 	  to leak information about the location of core kernel data structures
 	  but it does imply that function calls between modules and the core
 	  kernel will need to be resolved via veneers in the module PLT.
arch/arm64/kernel/kaslr.c +12 -8
index 47080c49cc7e..17dbdb055314 100644
@@ -117,13 +117,15 @@ u64 __init kaslr_early_init(u64 dt_phys)
 	/*
 	 * OK, so we are proceeding with KASLR enabled. Calculate a suitable
 	 * kernel image offset from the seed. Let's place the kernel in the
-	 * lower half of the VMALLOC area (VA_BITS - 2).
+	 * middle half of the VMALLOC area (VA_BITS - 2), and stay clear of
+	 * the lower and upper quarters to avoid colliding with other
+	 * allocations.
 	 * Even if we could randomize at page granularity for 16k and 64k pages,
 	 * let's always round to 2 MB so we don't interfere with the ability to
 	 * map using contiguous PTEs
 	 */
 	mask = ((1UL << (VA_BITS - 2)) - 1) & ~(SZ_2M - 1);
-	offset = seed & mask;
+	offset = BIT(VA_BITS - 3) + (seed & mask);
 
 	/* use the top 16 bits to randomize the linear region */
 	memstart_offset_seed = seed >> 48;
@@ -149,21 +151,23 @@ u64 __init kaslr_early_init(u64 dt_phys)
 		 * vmalloc region, since shadow memory is allocated for each
 		 * module at load time, whereas the vmalloc region is shadowed
 		 * by KASAN zero pages. So keep modules out of the vmalloc
-		 * region if KASAN is enabled.
+		 * region if KASAN is enabled, and put the kernel well within
+		 * 4 GB of the module region.
 		 */
-		return offset;
+		return offset % SZ_2G;
 
 	if (IS_ENABLED(CONFIG_RANDOMIZE_MODULE_REGION_FULL)) {
 		/*
-		 * Randomize the module region independently from the core
-		 * kernel. This prevents modules from leaking any information
+		 * Randomize the module region over a 4 GB window covering the
+		 * kernel. This reduces the risk of modules leaking information
 		 * about the address of the kernel itself, but results in
 		 * branches between modules and the core kernel that are
 		 * resolved via PLTs. (Branches between modules will be
 		 * resolved normally.)
 		 */
-		module_range = VMALLOC_END - VMALLOC_START - MODULES_VSIZE;
-		module_alloc_base = VMALLOC_START;
+		module_range = SZ_4G - (u64)(_end - _stext);
+		module_alloc_base = max((u64)_end + offset - SZ_4G,
+					(u64)MODULES_VADDR);
 	} else {
 		/*
 		 * Randomize the module region by setting module_alloc_base to
arch/arm64/kernel/module.c +4 -3
index c8c6c2828b79..70c3e5518e95 100644
@@ -55,9 +55,10 @@ void *module_alloc(unsigned long size)
 		 * less likely that the module region gets exhausted, so we
 		 * can simply omit this fallback in that case.
 		 */
-		p = __vmalloc_node_range(size, MODULE_ALIGN, VMALLOC_START,
-				VMALLOC_END, GFP_KERNEL, PAGE_KERNEL_EXEC, 0,
-				NUMA_NO_NODE, __builtin_return_address(0));
+		p = __vmalloc_node_range(size, MODULE_ALIGN, module_alloc_base,
+				module_alloc_base + SZ_4G, GFP_KERNEL,
+				PAGE_KERNEL_EXEC, 0, NUMA_NO_NODE,
+				__builtin_return_address(0));
 
 	if (p && (kasan_module_alloc(p, size) < 0)) {
 		vfree(p);
include/linux/sizes.h +4 -0
index ce3e8150c174..fbde0bc7e882 100644
@@ -8,6 +8,8 @@
 #ifndef __LINUX_SIZES_H__
 #define __LINUX_SIZES_H__
 
+#include <linux/const.h>
+
 #define SZ_1				0x00000001
 #define SZ_2				0x00000002
 #define SZ_4				0x00000004
@@ -44,4 +46,6 @@
 #define SZ_1G				0x40000000
 #define SZ_2G				0x80000000
 
+#define SZ_4G				_AC(0x100000000, ULL)
+
 #endif /* __LINUX_SIZES_H__ */
Signature · 1 line
2.11.0
#3 lore raw

PATCH v4 3/5 arm64/kernel: don't ban ADRP to work around Cortex-A53 erratum #843419

In reply to Ard Biesheuvel

Working around Cortex-A53 erratum #843419 involves special handling of ADRP instructions that end up in the last two instruction slots of a 4k page, or whose output register gets overwritten without having been read. (Note that the latter instruction sequence is never emitted by a properly functioning compiler, which is why it is disregarded by the handling of the same erratum in the bfd.ld linker which we rely on for the core kernel)

Normally, this gets taken care of by the linker, which can spot such sequences at final link time, and insert a veneer if the ADRP ends up at a vulnerable offset. However, linux kernel modules are partially linked ELF objects, and so there is no 'final link time' other than the runtime loading of the module, at which time all the static relocations are resolved.

For this reason, we have implemented the #843419 workaround for modules by avoiding ADRP instructions altogether, by using the large C model, and by passing -mpc-relative-literal-loads to recent versions of GCC that may emit adrp/ldr pairs to perform literal loads. However, this workaround forces us to keep literal data mixed with the instructions in the executable .text segment, and literal data may inadvertently turn into an exploitable speculative gadget depending on the relative offsets of arbitrary symbols.

So let's reimplement this workaround in a way that allows us to switch back to the small C model, and to drop the -mpc-relative-literal-loads GCC switch, by patching affected ADRP instructions at runtime:
- ADRP instructions that do not appear at 4k relative offset 0xff8 or 0xffc are ignored
- ADRP instructions that are within 1 MB of their target symbol are converted into ADR instructions
- remaining ADRP instructions are redirected via a veneer that performs the load using an unaffected movn/movk sequence.

---
 arch/arm64/Kconfig                  | 11 +--
 arch/arm64/Makefile                 |  5 --
 arch/arm64/include/asm/module.h     |  2 +
 arch/arm64/kernel/module-plts.c     | 86 +++++++++++++++++++-
 arch/arm64/kernel/module.c          | 32 +++++++-
 arch/arm64/kernel/reloc_test_core.c |  4 +-
 arch/arm64/kernel/reloc_test_syms.S | 12 ++-
 7 files changed, 128 insertions(+), 24 deletions(-)
arch/arm64/Kconfig +4 -7
index ae7d3d4c0bbe..1b11285cba36 100644
@@ -455,12 +455,12 @@ config ARM64_ERRATUM_845719
 config ARM64_ERRATUM_843419
 	bool "Cortex-A53: 843419: A load or store might access an incorrect address"
 	default y
-	select ARM64_MODULE_CMODEL_LARGE if MODULES
+	select ARM64_MODULE_PLTS if MODULES
 	help
 	  This option links the kernel with '--fix-cortex-a53-843419' and
-	  builds modules using the large memory model in order to avoid the use
-	  of the ADRP instruction, which can cause a subsequent memory access
-	  to use an incorrect address on Cortex-A53 parts up to r0p4.
+	  enables PLT support to replace certain ADRP instructions, which can
+	  cause subsequent memory accesses to use an incorrect address on
+	  Cortex-A53 parts up to r0p4.
 
 	  If unsure, say Y.
 
@@ -1104,9 +1104,6 @@ config ARM64_SVE
 
 	  To enable use of this extension on CPUs that implement it, say Y.
 
-config ARM64_MODULE_CMODEL_LARGE
-	bool
-
 config ARM64_MODULE_PLTS
 	bool
 	select HAVE_MOD_ARCH_SPECIFIC
arch/arm64/Makefile +0 -5
index b481b4a7c011..811ad08c531d 100644
@@ -51,7 +51,6 @@ endif
 
 KBUILD_CFLAGS	+= -mgeneral-regs-only $(lseinstr) $(brokengasinst)
 KBUILD_CFLAGS	+= -fno-asynchronous-unwind-tables
-KBUILD_CFLAGS	+= $(call cc-option, -mpc-relative-literal-loads)
 KBUILD_AFLAGS	+= $(lseinstr) $(brokengasinst)
 
 KBUILD_CFLAGS	+= $(call cc-option,-mabi=lp64)
@@ -77,10 +76,6 @@ endif
 
 CHECKFLAGS	+= -D__aarch64__ -m64
 
-ifeq ($(CONFIG_ARM64_MODULE_CMODEL_LARGE), y)
-KBUILD_CFLAGS_MODULE	+= -mcmodel=large
-endif
-
 ifeq ($(CONFIG_ARM64_MODULE_PLTS),y)
 KBUILD_LDFLAGS_MODULE	+= -T $(srctree)/arch/arm64/kernel/module.lds
 endif
arch/arm64/include/asm/module.h +2 -0
index 4f766178fa6f..b6dbbe3123a9 100644
@@ -39,6 +39,8 @@ struct mod_arch_specific {
 u64 module_emit_plt_entry(struct module *mod, void *loc, const Elf64_Rela *rela,
 			  Elf64_Sym *sym);
 
+u64 module_emit_adrp_veneer(struct module *mod, void *loc, u64 val);
+
 #ifdef CONFIG_RANDOMIZE_BASE
 extern u64 module_alloc_base;
 #else
arch/arm64/kernel/module-plts.c +83 -3

121 lines not on this page · raw message

arch/arm64/kernel/module.c +28 -4
index 70c3e5518e95..89217704944e 100644
@@ -198,6 +198,31 @@ static int reloc_insn_imm(enum aarch64_reloc_op op, __le32 *place, u64 val,
 	return 0;
 }
 
+static int reloc_insn_adrp(struct module *mod, __le32 *place, u64 val)
+{
+	if (!IS_ENABLED(CONFIG_ARM64_ERRATUM_843419) ||
+	    ((u64)place & 0xfff) < 0xff8)
+		return reloc_insn_imm(RELOC_OP_PAGE, place, val, 12, 21,
+				      AARCH64_INSN_IMM_ADR);
+
+	/* patch ADRP to ADR if it is in range */
+	if (!reloc_insn_imm(RELOC_OP_PREL, place, val & ~0xfff, 0, 21,
+			    AARCH64_INSN_IMM_ADR)) {
+		((u8 *)place)[3] &= 0x7f; /* clear opcode bit 31 */
+	} else {
+		u32 insn;
+
+		/* out of range for ADR -> emit a veneer */
+		val = module_emit_adrp_veneer(mod, place, val & ~0xfff);
+		if (!val)
+			return -ENOEXEC;
+		insn = aarch64_insn_gen_branch_imm((u64)place, val,
+						   AARCH64_INSN_BRANCH_NOLINK);
+		*place = cpu_to_le32(insn);
+	}
+	return 0;
+}
+
 int apply_relocate_add(Elf64_Shdr *sechdrs,
 		       const char *strtab,
 		       unsigned int symindex,
@@ -337,14 +362,13 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
 			ovf = reloc_insn_imm(RELOC_OP_PREL, loc, val, 0, 21,
 					     AARCH64_INSN_IMM_ADR);
 			break;
-#ifndef CONFIG_ARM64_ERRATUM_843419
 		case R_AARCH64_ADR_PREL_PG_HI21_NC:
 			overflow_check = false;
 		case R_AARCH64_ADR_PREL_PG_HI21:
-			ovf = reloc_insn_imm(RELOC_OP_PAGE, loc, val, 12, 21,
-					     AARCH64_INSN_IMM_ADR);
+			ovf = reloc_insn_adrp(me, loc, val);
+			if (ovf && ovf != -ERANGE)
+				return ovf;
 			break;
-#endif
 		case R_AARCH64_ADD_ABS_LO12_NC:
 		case R_AARCH64_LDST8_ABS_LO12_NC:
 			overflow_check = false;
arch/arm64/kernel/reloc_test_core.c +2 -2
index c124752a8bd3..a70489c584c7 100644
@@ -28,6 +28,7 @@ asmlinkage u64 absolute_data16(void);
 asmlinkage u64 signed_movw(void);
 asmlinkage u64 unsigned_movw(void);
 asmlinkage u64 relative_adrp(void);
+asmlinkage u64 relative_adrp_far(void);
 asmlinkage u64 relative_adr(void);
 asmlinkage u64 relative_data64(void);
 asmlinkage u64 relative_data32(void);
@@ -43,9 +44,8 @@ static struct {
 	{ "R_AARCH64_ABS16",		absolute_data16, UL(SYM16_ABS_VAL) },
 	{ "R_AARCH64_MOVW_SABS_Gn",	signed_movw, UL(SYM64_ABS_VAL) },
 	{ "R_AARCH64_MOVW_UABS_Gn",	unsigned_movw, UL(SYM64_ABS_VAL) },
-#ifndef CONFIG_ARM64_ERRATUM_843419
 	{ "R_AARCH64_ADR_PREL_PG_HI21",	relative_adrp, (u64)&sym64_rel },
-#endif
+	{ "R_AARCH64_ADR_PREL_PG_HI21",	relative_adrp_far, (u64)&printk },
 	{ "R_AARCH64_ADR_PREL_LO21",	relative_adr, (u64)&sym64_rel },
 	{ "R_AARCH64_PREL64",		relative_data64, (u64)&sym64_rel },
 	{ "R_AARCH64_PREL32",		relative_data32, (u64)&sym64_rel },
arch/arm64/kernel/reloc_test_syms.S +9 -3
index e1edcefeb02d..f333b4b7880d 100644
@@ -43,15 +43,21 @@ ENTRY(unsigned_movw)
 	ret
 ENDPROC(unsigned_movw)
 
-#ifndef CONFIG_ARM64_ERRATUM_843419
-
+	.align	12
+	.space	0xff8
 ENTRY(relative_adrp)
 	adrp	x0, sym64_rel
 	add	x0, x0, #:lo12:sym64_rel
 	ret
 ENDPROC(relative_adrp)
 
-#endif
+	.align	12
+	.space	0xffc
+ENTRY(relative_adrp_far)
+	adrp	x0, printk
+	add	x0, x0, #:lo12:printk
+	ret
+ENDPROC(relative_adrp_far)
 
 ENTRY(relative_adr)
 	adr	x0, sym64_rel
Signature · 1 line
2.11.0
#4 lore raw

PATCH v4 4/5 arm64/errata: add REVIDR handling to framework

In reply to Ard Biesheuvel

In some cases, core variants that are affected by a certain erratum also exist in versions that have the erratum fixed, and this fact is recorded in a dedicated bit in system register REVIDR_EL1.

Since the architecture does not require that a certain bit retains its meaning across different variants of the same model, each such REVIDR bit is tightly coupled to a certain revision/variant value, and so we need a list of revidr_mask/midr pairs to carry this information.

So add the struct member and the associated macros and handling to allow REVIDR fixes to be taken into account.

---
 arch/arm64/include/asm/cpufeature.h |  4 ++++
 arch/arm64/kernel/cpu_errata.c      | 21 +++++++++++++++++---
 2 files changed, 22 insertions(+), 3 deletions(-)
arch/arm64/include/asm/cpufeature.h +4 -0
index 060e3a4008ab..fbf0aab94d67 100644
@@ -105,6 +105,10 @@ struct arm64_cpu_capabilities {
 		struct {	/* To be used for erratum handling only */
 			u32 midr_model;
 			u32 midr_range_min, midr_range_max;
+			const struct arm64_midr_revidr {
+				u32 midr_rv;		/* revision/variant */
+				u32 revidr_mask;
+			} * const fixed_revs;
 		};
 
 		struct {	/* Feature register checking */
arch/arm64/kernel/cpu_errata.c +18 -3
index 52f15cd896e1..b161abdd6e27 100644
@@ -24,10 +24,22 @@
 static bool __maybe_unused
 is_affected_midr_range(const struct arm64_cpu_capabilities *entry, int scope)
 {
+	const struct arm64_midr_revidr *fix;
+	u32 midr = read_cpuid_id(), revidr;
+
 	WARN_ON(scope != SCOPE_LOCAL_CPU || preemptible());
-	return MIDR_IS_CPU_MODEL_RANGE(read_cpuid_id(), entry->midr_model,
-				       entry->midr_range_min,
-				       entry->midr_range_max);
+	if (!MIDR_IS_CPU_MODEL_RANGE(midr, entry->midr_model,
+				     entry->midr_range_min,
+				     entry->midr_range_max))
+		return false;
+
+	midr &= MIDR_REVISION_MASK | MIDR_VARIANT_MASK;
+	revidr = read_cpuid(REVIDR_EL1);
+	for (fix = entry->fixed_revs; fix && fix->revidr_mask; fix++)
+		if (midr == fix->midr_rv && (revidr & fix->revidr_mask))
+			return false;
+
+	return true;
 }
 
 static bool __maybe_unused
@@ -242,6 +254,9 @@ static int qcom_enable_link_stack_sanitization(void *data)
 	.midr_range_min = 0, \
 	.midr_range_max = (MIDR_VARIANT_MASK | MIDR_REVISION_MASK)
 
+#define MIDR_FIXED(rev, revidr_mask) \
+	.fixed_revs = (struct arm64_midr_revidr[]){{ (rev), (revidr_mask) }, {}}
+
 const struct arm64_cpu_capabilities arm64_errata[] = {
 #if	defined(CONFIG_ARM64_ERRATUM_826319) || \
 	defined(CONFIG_ARM64_ERRATUM_827319) || \
Signature · 1 line
2.11.0
#5 lore raw

PATCH v4 5/5 arm64/kernel: enable A53 erratum #8434319 handling at runtime

In reply to Ard Biesheuvel

Omit patching of ADRP instruction at module load time if the current CPUs are not susceptible to the erratum.

---
 arch/arm64/include/asm/cpucaps.h |  3 ++-
 arch/arm64/kernel/cpu_errata.c   | 10 ++++++++++
 arch/arm64/kernel/module-plts.c  |  3 ++-
 arch/arm64/kernel/module.c       |  1 +
 4 files changed, 15 insertions(+), 2 deletions(-)
arch/arm64/include/asm/cpucaps.h +2 -1
index bb263820de13..39134c46bb13 100644
@@ -45,7 +45,8 @@
 #define ARM64_HARDEN_BRANCH_PREDICTOR		24
 #define ARM64_HARDEN_BP_POST_GUEST_EXIT		25
 #define ARM64_HAS_RAS_EXTN			26
+#define ARM64_WORKAROUND_843419			27
 
-#define ARM64_NCAPS				27
+#define ARM64_NCAPS				28
 
 #endif /* __ASM_CPUCAPS_H */
arch/arm64/kernel/cpu_errata.c +10 -0
index b161abdd6e27..e18023e5deb9 100644
@@ -298,6 +298,16 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
 			   MIDR_CPU_VAR_REV(1, 2)),
 	},
 #endif
+#ifdef CONFIG_ARM64_ERRATUM_843419
+	{
+	/* Cortex-A53 r0p[01234] */
+		.desc = "ARM erratum 843419",
+		.capability = ARM64_WORKAROUND_843419,
+		.def_scope = SCOPE_LOCAL_CPU,
+		MIDR_RANGE(MIDR_CORTEX_A53, 0x00, 0x04),
+		MIDR_FIXED(0x4, BIT(8)),
+	},
+#endif
 #ifdef CONFIG_ARM64_ERRATUM_845719
 	{
 	/* Cortex-A53 r0p[01234] */
arch/arm64/kernel/module-plts.c +2 -1
index 534bf1d47119..1a583ccace00 100644
@@ -158,7 +158,8 @@ static unsigned int count_plts(Elf64_Sym *syms, Elf64_Rela *rela, int num,
 			break;
 		case R_AARCH64_ADR_PREL_PG_HI21_NC:
 		case R_AARCH64_ADR_PREL_PG_HI21:
-			if (!IS_ENABLED(CONFIG_ARM64_ERRATUM_843419))
+			if (!IS_ENABLED(CONFIG_ARM64_ERRATUM_843419) ||
+			    !cpus_have_const_cap(ARM64_WORKAROUND_843419))
 				break;
 
 			/*
arch/arm64/kernel/module.c +1 -0
index 89217704944e..47b40aaa1a5d 100644
@@ -201,6 +201,7 @@ static int reloc_insn_imm(enum aarch64_reloc_op op, __le32 *place, u64 val,
 static int reloc_insn_adrp(struct module *mod, __le32 *place, u64 val)
 {
 	if (!IS_ENABLED(CONFIG_ARM64_ERRATUM_843419) ||
+	    !cpus_have_const_cap(ARM64_WORKAROUND_843419) ||
 	    ((u64)place & 0xfff) < 0xff8)
 		return reloc_insn_imm(RELOC_OP_PAGE, place, val, 12, 21,
 				      AARCH64_INSN_IMM_ADR);
Signature · 1 line
2.11.0
#6 lore raw

PATCH v4 5/5 arm64/kernel: enable A53 erratum #8434319 handling at runtime

Quoted 2 lines · Ard Biesheuvel

Omit patching of ADRP instruction at module load time if the current CPUs are not susceptible to the erratum.

[...]

Quoted 19 lines
arch/arm64/kernel/module-plts.c +2 -1
index 534bf1d47119..1a583ccace00 100644
@@ -158,7 +158,8 @@ static unsigned int count_plts(Elf64_Sym *syms, Elf64_Rela *rela, int num,
 			break;
 		case R_AARCH64_ADR_PREL_PG_HI21_NC:
 		case R_AARCH64_ADR_PREL_PG_HI21:
-			if (!IS_ENABLED(CONFIG_ARM64_ERRATUM_843419))
+			if (!IS_ENABLED(CONFIG_ARM64_ERRATUM_843419) ||
+			    !cpus_have_const_cap(ARM64_WORKAROUND_843419))
 				break;
 
 			/*
arch/arm64/kernel/module.c +0 -0
index 89217704944e..47b40aaa1a5d 100644

arch/arm64/kernel/module.c static int reloc_insn_imm(enum aarch64_reloc_op op, __le32 *place, u64 val, · line 204

Quoted 4 lines
arch/arm64/kernel/module.c +1 -0
@@ -201,6 +201,7 @@ static int reloc_insn_imm(enum aarch64_reloc_op op, __le32 *place, u64 val,
 static int reloc_insn_adrp(struct module *mod, __le32 *place, u64 val)
 {
 	if (!IS_ENABLED(CONFIG_ARM64_ERRATUM_843419) ||
+	    !cpus_have_const_cap(ARM64_WORKAROUND_843419) ||

Mind if I drop the IS_ENABLED check here and in the hunk above? The const_cap check along should be sufficient, no?

Will

#7 lore raw

PATCH v4 5/5 arm64/kernel: enable A53 erratum #8434319 handling at runtime

Quoted 19 lines · Will Deacon
Quoted 2 lines · Ard Biesheuvel

Omit patching of ADRP instruction at module load time if the current CPUs are not susceptible to the erratum.

[...]

Quoted 13 lines
arch/arm64/kernel/module-plts.c +2 -1
index 534bf1d47119..1a583ccace00 100644
@@ -158,7 +158,8 @@ static unsigned int count_plts(Elf64_Sym *syms, Elf64_Rela *rela, int num,
                      break;
              case R_AARCH64_ADR_PREL_PG_HI21_NC:
              case R_AARCH64_ADR_PREL_PG_HI21:
-                     if (!IS_ENABLED(CONFIG_ARM64_ERRATUM_843419))
+                     if (!IS_ENABLED(CONFIG_ARM64_ERRATUM_843419) ||
+                         !cpus_have_const_cap(ARM64_WORKAROUND_843419))
                              break;
 
Quoted 14 lines

arch/arm64/kernel/module.c static int reloc_insn_imm(enum aarch64_reloc_op op, __le32 *place, u64 val, · line 204

Quoted 10 lines
arch/arm64/kernel/module-plts.c +0 -0
                      /*
arch/arm64/kernel/module.c +1 -0
index 89217704944e..47b40aaa1a5d 100644
@@ -201,6 +201,7 @@ static int reloc_insn_imm(enum aarch64_reloc_op op, __le32 *place, u64 val,
 static int reloc_insn_adrp(struct module *mod, __le32 *place, u64 val)
 {
      if (!IS_ENABLED(CONFIG_ARM64_ERRATUM_843419) ||
+         !cpus_have_const_cap(ARM64_WORKAROUND_843419) ||

Mind if I drop the IS_ENABLED check here and in the hunk above? The const_cap check along should be sufficient, no?

Without the IS_ENABLED() check, the code will always be present in the object file.

I have no strong preference either way, though.

#8 lore raw

PATCH v4 5/5 arm64/kernel: enable A53 erratum #8434319 handling at runtime

Quoted 35 lines · Ard Biesheuvel
Quoted 19 lines · Will Deacon
Quoted 2 lines · Ard Biesheuvel

Omit patching of ADRP instruction at module load time if the current CPUs are not susceptible to the erratum.

[...]

Quoted 13 lines
arch/arm64/kernel/module-plts.c +2 -1
index 534bf1d47119..1a583ccace00 100644
@@ -158,7 +158,8 @@ static unsigned int count_plts(Elf64_Sym *syms, Elf64_Rela *rela, int num,
                      break;
              case R_AARCH64_ADR_PREL_PG_HI21_NC:
              case R_AARCH64_ADR_PREL_PG_HI21:
-                     if (!IS_ENABLED(CONFIG_ARM64_ERRATUM_843419))
+                     if (!IS_ENABLED(CONFIG_ARM64_ERRATUM_843419) ||
+                         !cpus_have_const_cap(ARM64_WORKAROUND_843419))
                              break;
 
Quoted 14 lines

arch/arm64/kernel/module.c static int reloc_insn_imm(enum aarch64_reloc_op op, __le32 *place, u64 val, · line 204

Quoted 10 lines
arch/arm64/kernel/module-plts.c +0 -0
                      /*
arch/arm64/kernel/module.c +1 -0
index 89217704944e..47b40aaa1a5d 100644
@@ -201,6 +201,7 @@ static int reloc_insn_imm(enum aarch64_reloc_op op, __le32 *place, u64 val,
 static int reloc_insn_adrp(struct module *mod, __le32 *place, u64 val)
 {
      if (!IS_ENABLED(CONFIG_ARM64_ERRATUM_843419) ||
+         !cpus_have_const_cap(ARM64_WORKAROUND_843419) ||

Mind if I drop the IS_ENABLED check here and in the hunk above? The const_cap check along should be sufficient, no?

Quoted 4 lines

Without the IS_ENABLED() check, the code will always be present in the object file.

I have no strong preference either way, though.

Ehm, strike that. You will probably hit a linker error if you drop it, because some dependent functions are #ifdef'ed out if the erratum workaround is disabled.

#9 lore raw

PATCH v4 5/5 arm64/kernel: enable A53 erratum #8434319 handling at runtime

Quoted 36 lines · Ard Biesheuvel
Quoted 35 lines · Ard Biesheuvel
Quoted 33 lines · Will Deacon
Quoted 2 lines · Ard Biesheuvel

Omit patching of ADRP instruction at module load time if the current CPUs are not susceptible to the erratum.

[...]

Quoted 19 lines
arch/arm64/kernel/module-plts.c +2 -1
index 534bf1d47119..1a583ccace00 100644
@@ -158,7 +158,8 @@ static unsigned int count_plts(Elf64_Sym *syms, Elf64_Rela *rela, int num,
                      break;
              case R_AARCH64_ADR_PREL_PG_HI21_NC:
              case R_AARCH64_ADR_PREL_PG_HI21:
-                     if (!IS_ENABLED(CONFIG_ARM64_ERRATUM_843419))
+                     if (!IS_ENABLED(CONFIG_ARM64_ERRATUM_843419) ||
+                         !cpus_have_const_cap(ARM64_WORKAROUND_843419))
                              break;
 
                      /*
arch/arm64/kernel/module.c +0 -0
index 89217704944e..47b40aaa1a5d 100644

arch/arm64/kernel/module.c static int reloc_insn_imm(enum aarch64_reloc_op op, __le32 *place, u64 val, · line 204

Quoted 4 lines
arch/arm64/kernel/module.c +1 -0
@@ -201,6 +201,7 @@ static int reloc_insn_imm(enum aarch64_reloc_op op, __le32 *place, u64 val,
 static int reloc_insn_adrp(struct module *mod, __le32 *place, u64 val)
 {
      if (!IS_ENABLED(CONFIG_ARM64_ERRATUM_843419) ||
+         !cpus_have_const_cap(ARM64_WORKAROUND_843419) ||

Mind if I drop the IS_ENABLED check here and in the hunk above? The const_cap check along should be sufficient, no?

Quoted 8 lines
Quoted 4 lines

Without the IS_ENABLED() check, the code will always be present in the object file.

I have no strong preference either way, though.

Ehm, strike that. You will probably hit a linker error if you drop it, because some dependent functions are #ifdef'ed out if the erratum workaround is disabled.

Ah, ok then. Thanks for the quick response.

Will

reviewer
#10 lore raw

PATCH v4 5/5 arm64/kernel: enable A53 erratum #8434319 handling at runtime

In reply to Ard Biesheuvel
Quoted 35 lines · Ard Biesheuvel
Quoted 19 lines · Will Deacon
Quoted 2 lines · Ard Biesheuvel

Omit patching of ADRP instruction at module load time if the current CPUs are not susceptible to the erratum.

[...]

Quoted 13 lines
arch/arm64/kernel/module-plts.c +2 -1
index 534bf1d47119..1a583ccace00 100644
@@ -158,7 +158,8 @@ static unsigned int count_plts(Elf64_Sym *syms, Elf64_Rela *rela, int num,
                      break;
              case R_AARCH64_ADR_PREL_PG_HI21_NC:
              case R_AARCH64_ADR_PREL_PG_HI21:
-                     if (!IS_ENABLED(CONFIG_ARM64_ERRATUM_843419))
+                     if (!IS_ENABLED(CONFIG_ARM64_ERRATUM_843419) ||
+                         !cpus_have_const_cap(ARM64_WORKAROUND_843419))
                              break;
 
Quoted 14 lines

arch/arm64/kernel/module.c static int reloc_insn_imm(enum aarch64_reloc_op op, __le32 *place, u64 val, · line 204

Quoted 10 lines
arch/arm64/kernel/module-plts.c +0 -0
                      /*
arch/arm64/kernel/module.c +1 -0
index 89217704944e..47b40aaa1a5d 100644
@@ -201,6 +201,7 @@ static int reloc_insn_imm(enum aarch64_reloc_op op, __le32 *place, u64 val,
 static int reloc_insn_adrp(struct module *mod, __le32 *place, u64 val)
 {
      if (!IS_ENABLED(CONFIG_ARM64_ERRATUM_843419) ||
+         !cpus_have_const_cap(ARM64_WORKAROUND_843419) ||

Mind if I drop the IS_ENABLED check here and in the hunk above? The const_cap check along should be sufficient, no?

Quoted 4 lines

Without the IS_ENABLED() check, the code will always be present in the object file.

I have no strong preference either way, though.

As with other case, perhaps fold this into a helper in <asm/cpufeature.h> ?

static inline bool system_needs_arm64_workaround_843419()

{
	return (IS_ENABLED(CONFIG_ARM64_ERRATUM_843419) &&
		cpus_have_const_cap(ARM64_WORKAROUND_843419))
}

... then use the inverse in the cases above.

Thanks,
Mark.

#11 lore raw

PATCH v4 5/5 arm64/kernel: enable A53 erratum #8434319 handling at runtime

Quoted 44 lines · Mark Rutland
Quoted 35 lines · Ard Biesheuvel
Quoted 19 lines · Will Deacon
Quoted 2 lines · Ard Biesheuvel

Omit patching of ADRP instruction at module load time if the current CPUs are not susceptible to the erratum.

[...]

Quoted 13 lines
arch/arm64/kernel/module-plts.c +2 -1
index 534bf1d47119..1a583ccace00 100644
@@ -158,7 +158,8 @@ static unsigned int count_plts(Elf64_Sym *syms, Elf64_Rela *rela, int num,
                      break;
              case R_AARCH64_ADR_PREL_PG_HI21_NC:
              case R_AARCH64_ADR_PREL_PG_HI21:
-                     if (!IS_ENABLED(CONFIG_ARM64_ERRATUM_843419))
+                     if (!IS_ENABLED(CONFIG_ARM64_ERRATUM_843419) ||
+                         !cpus_have_const_cap(ARM64_WORKAROUND_843419))
                              break;
 
Quoted 14 lines

arch/arm64/kernel/module.c static int reloc_insn_imm(enum aarch64_reloc_op op, __le32 *place, u64 val, · line 204

Quoted 10 lines
arch/arm64/kernel/module-plts.c +0 -0
                      /*
arch/arm64/kernel/module.c +1 -0
index 89217704944e..47b40aaa1a5d 100644
@@ -201,6 +201,7 @@ static int reloc_insn_imm(enum aarch64_reloc_op op, __le32 *place, u64 val,
 static int reloc_insn_adrp(struct module *mod, __le32 *place, u64 val)
 {
      if (!IS_ENABLED(CONFIG_ARM64_ERRATUM_843419) ||
+         !cpus_have_const_cap(ARM64_WORKAROUND_843419) ||

Mind if I drop the IS_ENABLED check here and in the hunk above? The const_cap check along should be sufficient, no?

Quoted 4 lines

Without the IS_ENABLED() check, the code will always be present in the object file.

I have no strong preference either way, though.

As with other case, perhaps fold this into a helper in <asm/cpufeature.h> ?

Quoted 8 lines

static inline bool system_needs_arm64_workaround_843419()

{
        return (IS_ENABLED(CONFIG_ARM64_ERRATUM_843419) &&
                cpus_have_const_cap(ARM64_WORKAROUND_843419))
}

... then use the inverse in the cases above.

I'm fine with adding a helper, but 'system_needs_arm64_workaround_843419' is a bit misleading, given that it returns false if the system needs it but support is compiled out.

#12 lore raw

PATCH v4 5/5 arm64/kernel: enable A53 erratum #8434319 handling at runtime

Quoted 51 lines · Ard Biesheuvel
Quoted 50 lines · Mark Rutland
Quoted 35 lines · Ard Biesheuvel
Quoted 19 lines · Will Deacon
Quoted 2 lines · Ard Biesheuvel

Omit patching of ADRP instruction at module load time if the current CPUs are not susceptible to the erratum.

[...]

Quoted 13 lines
arch/arm64/kernel/module-plts.c +2 -1
index 534bf1d47119..1a583ccace00 100644
@@ -158,7 +158,8 @@ static unsigned int count_plts(Elf64_Sym *syms, Elf64_Rela *rela, int num,
                      break;
              case R_AARCH64_ADR_PREL_PG_HI21_NC:
              case R_AARCH64_ADR_PREL_PG_HI21:
-                     if (!IS_ENABLED(CONFIG_ARM64_ERRATUM_843419))
+                     if (!IS_ENABLED(CONFIG_ARM64_ERRATUM_843419) ||
+                         !cpus_have_const_cap(ARM64_WORKAROUND_843419))
                              break;
 
Quoted 14 lines

arch/arm64/kernel/module.c static int reloc_insn_imm(enum aarch64_reloc_op op, __le32 *place, u64 val, · line 204

Quoted 10 lines
arch/arm64/kernel/module-plts.c +0 -0
                      /*
arch/arm64/kernel/module.c +1 -0
index 89217704944e..47b40aaa1a5d 100644
@@ -201,6 +201,7 @@ static int reloc_insn_imm(enum aarch64_reloc_op op, __le32 *place, u64 val,
 static int reloc_insn_adrp(struct module *mod, __le32 *place, u64 val)
 {
      if (!IS_ENABLED(CONFIG_ARM64_ERRATUM_843419) ||
+         !cpus_have_const_cap(ARM64_WORKAROUND_843419) ||

Mind if I drop the IS_ENABLED check here and in the hunk above? The const_cap check along should be sufficient, no?

Quoted 4 lines

Without the IS_ENABLED() check, the code will always be present in the object file.

I have no strong preference either way, though.

As with other case, perhaps fold this into a helper in <asm/cpufeature.h> ?

static inline bool system_needs_arm64_workaround_843419()

{
        return (IS_ENABLED(CONFIG_ARM64_ERRATUM_843419) &&
                cpus_have_const_cap(ARM64_WORKAROUND_843419))
}
Quoted 6 lines
Quoted 2 lines

... then use the inverse in the cases above.

I'm fine with adding a helper, but 'system_needs_arm64_workaround_843419' is a bit misleading, given that it returns false if the system needs it but support is compiled out.

FWIW, I'm fine with the code as it is.

Will