arm64/kernel: get rid of GCC large model code
v3 cover arm64/kernel: get rid of GCC large model code
Hi all,
I am resending this as an RFC, because I'd like to understand whether anyone else shares my concern, or whether I am being overly paranoid.
v2 blurb:
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
Ard Biesheuvel (3):
arm64/kernel: kaslr: reduce module randomization range to 4 GB arm64/kernel: don't ban ADRP to work around Cortex-A53 erratum #843419 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/module.h | 2 + arch/arm64/kernel/cpu_errata.c | 26 +++++++ arch/arm64/kernel/kaslr.c | 20 +++-- arch/arm64/kernel/module-plts.c | 77 +++++++++++++++++++- arch/arm64/kernel/module.c | 40 ++++++++-- arch/arm64/kernel/reloc_test_core.c | 4 +- arch/arm64/kernel/reloc_test_syms.S | 12 ++- include/linux/sizes.h | 2 + 11 files changed, 171 insertions(+), 38 deletions(-)
Signature · 1 line
2.11.0
RFC PATCH v3 1/3 arm64/kernel: kaslr: reduce module randomization range to 4 GB
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 | 2 ++ 4 files changed, 21 insertions(+), 15 deletions(-)
arch/arm64/Kconfig +3 -4
@@ -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
@@ -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
@@ -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 +2 -0
@@ -44,4 +44,6 @@#define SZ_1G 0x40000000#define SZ_2G 0x80000000 +#define SZ_4G 0x100000000ULL +#endif /* __LINUX_SIZES_H__ */
Signature · 1 line
2.11.0
Mark Rutland
reviewerRFC PATCH v3 1/3 arm64/kernel: kaslr: reduce module randomization range to 4 GB
Quoted 9 lines · 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.
Quoted 4 lines
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.
I guess that means they pollute the I-caches, too?
How big a difference does this series make to .text size?
I don't really have a strong opinion here. IIRC the idea for randomizing modules across the whole vmalloc space was to make it harder for module bugs to leak "real" kernel addresses, but I don't know how much that's likely to help in practice, and the performance / cache footprint wins are enticing.
[...]
arch/arm64/kernel/kaslr.c u64 __init kaslr_early_init(u64 dt_phys) · line 157
Quoted 10 lines
arch/arm64/kernel/kaslr.c +3
-2@@ -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 ;
I wonder if we can do more here, taking the kernel size into account.
[...]
include/linux/sizes.h line 47
Quoted 9 lines
include/linux/sizes.h +1
-0@@ -44,4 +44,6 @@#define SZ_1G 0x40000000#define SZ_2G 0x80000000 +#define SZ_4G 0x100000000ULL
Some asm includes <linux/sizes.h>, so it'd be nice for this to use ULL().
Masahiro Yamada had patches moving that to <linux/const.h>.
Thanks,
Mark.
RFC PATCH v3 1/3 arm64/kernel: kaslr: reduce module randomization range to 4 GB
Quoted 10 lines · Mark Rutland
Quoted 9 lines · 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.
Quoted 7 lines
Quoted 4 lines
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.
I guess that means they pollute the I-caches, too?
Yes.
Quoted 2 lines
How big a difference does this series make to .text size?
I will add the numbers for a couple of sizable modules when I respin, although i don't expect that aspect to be the most convincing.
Quoted 6 lines
I don't really have a strong opinion here. IIRC the idea for randomizing modules across the whole vmalloc space was to make it harder for module bugs to leak "real" kernel addresses, but I don't know how much that's likely to help in practice, and the performance / cache footprint wins are enticing.
Yes. I think the important thing is that they are randomized independently, and the additional entropy in the high order bits is unlikely to make a huge difference imo.
When we added KASLR, the reason we enabled full module randomization by default was to get coverage for the new PLT code, not because it is deemed 'better' in some respect.
Quoted 15 lines
[...]
arch/arm64/kernel/kaslr.c u64 __init kaslr_early_init(u64 dt_phys) · line 157
Quoted 10 lines
arch/arm64/kernel/kaslr.c +3
-2@@ -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 ;I wonder if we can do more here, taking the kernel size into account.
Quoted 2 lines
[...]
Not sure whether it matters tbh. To be honest, I think most KASAN users turn KASLR off unless they are debugging some aspect of KASLR itself (that's certainly how I use it)
Quoted 8 lines
Quoted 8 lines
include/linux/sizes.h +0
-0@@ -44,4 +44,6 @@#define SZ_1G 0x40000000#define SZ_2G 0x80000000
Quoted 7 lines
include/linux/sizes.h line 47
Quoted 1 line
include/linux/sizes.h +1
-0+#define SZ_4G 0x100000000ULLSome asm includes <
linux/sizes.h>, so it'd be nice for this to useULL().Masahiro Yamada had patches moving that to <
linux/const.h>.
OK
RFC PATCH v3 1/3 arm64/kernel: kaslr: reduce module randomization range to 4 GB
Quoted 19 lines · Ard Biesheuvel
Quoted 10 lines · Mark Rutland
Quoted 9 lines · 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.Quoted 7 lines
Quoted 4 lines
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.
I guess that means they pollute the I-caches, too?
Quoted 8 lines
Yes.
Quoted 2 lines
How big a difference does this series make to .text size?
I will add the numbers for a couple of sizable modules when I respin, although i don't expect that aspect to be the most convincing.
For the record, this is what I get before and after for a widely used fairly large module:
text data bss dec hex filename
317907 7004 16 324927 4f53f net/mac80211/mac80211.ko
text data bss dec hex filename
315603 7004 19 322626 4ec42 net/mac80211/mac80211.ko
This is large model vs small model with the stack protector in strong mode.
So while the size does decrease slightly, I think the reduced cache footprint is a more important metric here, and I am not sure how to measure that.
Quoted 54 lines
Quoted 6 lines
I don't really have a strong opinion here. IIRC the idea for randomizing modules across the whole vmalloc space was to make it harder for module bugs to leak "real" kernel addresses, but I don't know how much that's likely to help in practice, and the performance / cache footprint wins are enticing.
Yes. I think the important thing is that they are randomized independently, and the additional entropy in the high order bits is unlikely to make a huge difference imo.
When we added KASLR, the reason we enabled full module randomization by default was to get coverage for the new PLT code, not because it is deemed 'better' in some respect.
Quoted 15 lines
[...]
arch/arm64/kernel/kaslr.c u64 __init kaslr_early_init(u64 dt_phys) · line 157
Quoted 10 lines
arch/arm64/kernel/kaslr.c +3
-2@@ -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 ;I wonder if we can do more here, taking the kernel size into account.
Quoted 2 lines
[...]
Not sure whether it matters tbh. To be honest, I think most KASAN users turn KASLR off unless they are debugging some aspect of KASLR itself (that's certainly how I use it)
Quoted 8 lines
Quoted 8 lines
include/linux/sizes.h +0
-0@@ -44,4 +44,6 @@#define SZ_1G 0x40000000#define SZ_2G 0x80000000Quoted 7 lines
include/linux/sizes.h line 47
Quoted 1 line
include/linux/sizes.h +1
-0+#define SZ_4G 0x100000000ULLSome asm includes <
linux/sizes.h>, so it'd be nice for this to useULL().Masahiro Yamada had patches moving that to <
linux/const.h>.OK
RFC PATCH v3 2/3 arm64/kernel: don't ban ADRP to work around Cortex-A53 erratum #843419
In reply to Ard BiesheuvelWorking 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)
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 | 76 +++++++++++++++++++- 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, 120 insertions(+), 22 deletions(-)
arch/arm64/Kconfig +4 -7
@@ -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
@@ -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
@@ -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 +73 -3
111 lines not on this page · raw message
arch/arm64/kernel/module.c +30 -2
@@ -198,6 +198,32 @@ static int reloc_insn_imm(enum aarch64_reloc_op op, __le32 *place, u64 val,return 0 ; } +static bool reloc_adrp_erratum_843419 (struct module *mod,__le32 *place, +u64 val) +{ +if (!IS_ENABLED (CONFIG_ARM64_ERRATUM_843419 )) +return false; + +/* only ADRP instructions at the end of a 4k page are affected */ +if (((u64 )place &0xfff ) <0xff8 ) +return false; + +/* 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 ); + insn =aarch64_insn_gen_branch_imm ((u64 )place, val, +AARCH64_INSN_BRANCH_NOLINK ); + *place =cpu_to_le32 (insn); + } +return true; +} + int apply_relocate_add(Elf64_Shdr *sechdrs, const char *strtab, unsigned int symindex,
@@ -337,14 +363,16 @@ 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 =0 ; +if (reloc_adrp_erratum_843419 (me, loc, val)) +break ; + ovf =reloc_insn_imm (RELOC_OP_PAGE , loc, val,12 ,21 ,AARCH64_INSN_IMM_ADR );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
@@ -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
@@ -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
Mark Rutland
reviewerRFC PATCH v3 2/3 arm64/kernel: don't ban ADRP to work around Cortex-A53 erratum #843419
Quoted 4 lines · Ard Biesheuvel
arch/arm64/kernel/module.c +4
-0+static bool reloc_adrp_erratum_843419 (struct module *mod,__le32 *place, +u64 val) +{ +if (!IS_ENABLED (CONFIG_ARM64_ERRATUM_843419 ))
arch/arm64/kernel/module.c static int reloc_insn_imm(enum aarch64_reloc_op op, __le32 *place, u64 val, · line 214
Quoted 10 lines
arch/arm64/kernel/module.c +10
-0+return false; + +/* only ADRP instructions at the end of a 4k page are affected */ +if (((u64 )place &0xfff ) <0xff8 ) +return false; + +/* 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 */
That's a bit hideous, and broken for BE, I think.
Can we not construct the equivalent ADR using the usual insn code, and insert it?
Thanks,
Mark.
RFC PATCH v3 2/3 arm64/kernel: don't ban ADRP to work around Cortex-A53 erratum #843419
Quoted 18 lines · Mark Rutland
Quoted 4 lines · Ard Biesheuvel
arch/arm64/kernel/module.c +4
-0+static bool reloc_adrp_erratum_843419 (struct module *mod,__le32 *place, +u64 val) +{ +if (!IS_ENABLED (CONFIG_ARM64_ERRATUM_843419 ))arch/arm64/kernel/module.c static int reloc_insn_imm(enum aarch64_reloc_op op, __le32 *place, u64 val, · line 214
Quoted 10 lines
arch/arm64/kernel/module.c +10
-0+return false; + +/* only ADRP instructions at the end of a 4k page are affected */ +if (((u64 )place &0xfff ) <0xff8 ) +return false; + +/* 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 */ That's a bit hideous, and broken for BE, I think.
Instructions are always LE
Quoted 3 lines
Can we not construct the equivalent ADR using the usual insn code, and insert it?
Sure.
Mark Rutland
reviewerRFC PATCH v3 2/3 arm64/kernel: don't ban ADRP to work around Cortex-A53 erratum #843419
Quoted 19 lines · Ard Biesheuvel
Quoted 17 lines · Mark Rutland
Quoted 4 lines · Ard Biesheuvel
arch/arm64/kernel/module.c +4
-0+static bool reloc_adrp_erratum_843419 (struct module *mod,__le32 *place, +u64 val) +{ +if (!IS_ENABLED (CONFIG_ARM64_ERRATUM_843419 ))arch/arm64/kernel/module.c static int reloc_insn_imm(enum aarch64_reloc_op op, __le32 *place, u64 val, · line 214
Quoted 10 lines
arch/arm64/kernel/module.c +10
-0+return false; + +/* only ADRP instructions at the end of a 4k page are affected */ +if (((u64 )place &0xfff ) <0xff8 ) +return false; + +/* 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 */ That's a bit hideous, and broken for BE, I think.
Quoted 1 line
Instructions are always LE
Ugh; I forgot that single didn't have endianness. :(
Quoted 4 lines
Quoted 2 lines
Can we not construct the equivalent ADR using the usual insn code, and insert it?
Sure.
Cheers!
Mark.
RFC PATCH v3 2/3 arm64/kernel: don't ban ADRP to work around Cortex-A53 erratum #843419
Quoted 22 lines · Mark Rutland
Quoted 19 lines · Ard Biesheuvel
Quoted 17 lines · Mark Rutland
Quoted 4 lines · Ard Biesheuvel
arch/arm64/kernel/module.c +4
-0+static bool reloc_adrp_erratum_843419 (struct module *mod,__le32 *place, +u64 val) +{ +if (!IS_ENABLED (CONFIG_ARM64_ERRATUM_843419 ))arch/arm64/kernel/module.c static int reloc_insn_imm(enum aarch64_reloc_op op, __le32 *place, u64 val, · line 214
Quoted 10 lines
arch/arm64/kernel/module.c +10
-0+return false; + +/* only ADRP instructions at the end of a 4k page are affected */ +if (((u64 )place &0xfff ) <0xff8 ) +return false; + +/* 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 */ That's a bit hideous, and broken for BE, I think.
Quoted 1 line
Instructions are always LE
Quoted 9 lines
Ugh; I forgot that single didn't have endianness. :(
Quoted 4 lines
Quoted 2 lines
Can we not construct the equivalent ADR using the usual insn code, and insert it?
Sure.
Cheers!
Umm, it appears we don't have any code to emit ADR or ADRP instructions, and I am reluctant to add a bunch of routines that are only ever used to flick a single opcode bit.
Mark Rutland
reviewerRFC PATCH v3 2/3 arm64/kernel: don't ban ADRP to work around Cortex-A53 erratum #843419
Quoted 13 lines · Ard Biesheuvel
Quoted 12 lines · Mark Rutland
Quoted 11 lines · Ard Biesheuvel
Quoted 9 lines · Mark Rutland
arch/arm64/kernel/module.c static int reloc_insn_imm(enum aarch64_reloc_op op, __le32 *place, u64 val, · line 214
Quoted 4 lines · Ard Biesheuvel
arch/arm64/kernel/module.c +4
-0+/* 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 */ Can we not construct the equivalent ADR using the usual insn code, and insert it?
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
Sure.
Cheers!
Umm, it appears we don't have any code to emit ADR or ADRP instructions, and I am reluctant to add a bunch of routines that are only ever used to flick a single opcode bit.
Fair enough.
Mark.
RFC PATCH v3 2/3 arm64/kernel: don't ban ADRP to work around Cortex-A53 erratum #843419
In reply to Ard BiesheuvelQuoted 5 lines · 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)
Does the workaround currently implemented in the linker also make this same assumption? If not, I'm a little wary that we're making an assumption about compiler behaviour with no way to detect whether its been violated or not.
Quoted 9 lines
arch/arm64/kernel/module-plts.c +1
-0@@ -41,6 +41,46 @@ u64 module_emit_plt_entry(struct module *mod, void *loc, const Elf64_Rela *rela,return (u64)&plt[i]; } +#ifdef CONFIG_ARM64_ERRATUM_843419
arch/arm64/kernel/module-plts.c u64 module_emit_plt_entry(struct module *mod, void *loc, const Elf64_Rela *rela, · line 54
Quoted 10 lines
arch/arm64/kernel/module-plts.c +10
-0+u64 module_emit_adrp_veneer (struct module *mod,void *loc,u64 val) +{ +struct mod_plt_sec *pltsec = !in_init (mod, loc) ? &mod->arch.core : + &mod->arch.init; +struct plt_entry *plt = (struct plt_entry *)pltsec->plt->sh_addr; +int i = pltsec->plt_num_entries++; +u32 mov0, mov1, mov2, br; +int rd; + +BUG_ON (pltsec->plt_num_entries > pltsec->plt_max_entries);
I'd prefer just to fail loading the module, but I see we already have a BUG_ON for the existing PLT code. Oh well.
Quoted 60 lines
arch/arm64/kernel/module-plts.c +36
-1+ +/* get the destination register of the ADRP instruction */ + rd =aarch64_insn_decode_register (AARCH64_INSN_REGTYPE_RD , +le32_to_cpup ((__le32 *)loc)); + +/* generate the veneer instructions */ + mov0 =aarch64_insn_gen_movewide (rd, (u16 )~val,0 , +AARCH64_INSN_VARIANT_64BIT , +AARCH64_INSN_MOVEWIDE_INVERSE ); + mov1 =aarch64_insn_gen_movewide (rd, (u16)(val >>16 ),16 , +AARCH64_INSN_VARIANT_64BIT , +AARCH64_INSN_MOVEWIDE_KEEP ); + mov2 =aarch64_insn_gen_movewide (rd, (u16)(val >>32 ),32 , +AARCH64_INSN_VARIANT_64BIT , +AARCH64_INSN_MOVEWIDE_KEEP ); + br =aarch64_insn_gen_branch_imm ((u64)&plt[i].br, (u64 )loc +4 , +AARCH64_INSN_BRANCH_NOLINK ); + + plt[i] = (struct plt_entry ){ +cpu_to_le32 (mov0), +cpu_to_le32 (mov1), +cpu_to_le32 (mov2), +cpu_to_le32 (br) + }; + + return (u64)&plt[i]; +} +#endif +#define cmp_3way (a,b) ((a) < (b) ? -1 : (a) > (b))static int cmp_rela (const void *a,const void *b)@@ -68,16 +108,21 @@ static bool duplicate_rel(const Elf64_Rela *rela, int num) }static unsigned int count_plts (Elf64_Sym *syms,Elf64_Rela *rela,int num, -Elf64_Word dstidx) +Elf64_Word dstidx,Elf_Shdr *dstsec) {unsigned int ret =0 ;Elf64_Sym *s;int i;for (i =0 ; i < num; i++) { +u64 min_align; +switch (ELF64_R_TYPE (rela[i].r_info)) {case R_AARCH64_JUMP26 :case R_AARCH64_CALL26 : +if (!IS_ENABLED (CONFIG_RANDOMIZE_BASE )) +break ; +/* * We only have to consider branch targets that resolve * to symbols that are defined in a different section. @@ -109,6 +154,31 @@ static unsigned int count_plts(Elf64_Sym *syms, Elf64_Rela *rela, int num,if (rela[i].r_addend !=0 || !duplicate_rel (rela, i)) ret++;break ; +case R_AARCH64_ADR_PREL_PG_HI21_NC :
arch/arm64/kernel/module-plts.c static unsigned int count_plts(Elf64_Sym *syms, Elf64_Rela *rela, int num, · line 167
Quoted 10 lines
arch/arm64/kernel/module-plts.c +10
-0+case R_AARCH64_ADR_PREL_PG_HI21 : +if (!IS_ENABLED (CONFIG_ARM64_ERRATUM_843419 )) +break ; + +/* +* Determine the minimal safe alignment for this ADRP +* instruction: the section alignment at which it is +* guaranteed not to appear at a vulnerable offset. +*/ + min_align =2 <<ffz (rela[i].r_offset |0x7 );
I'm struggling to decipher this, can you give me a hint please? Why 0x7? Is the "2" because of the two vulnerable offsets?
Will
RFC PATCH v3 2/3 arm64/kernel: don't ban ADRP to work around Cortex-A53 erratum #843419
Quoted 7 lines · Will Deacon
Quoted 5 lines · 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)
Quoted 5 lines
Does the workaround currently implemented in the linker also make this same assumption? If not, I'm a little wary that we're making an assumption about compiler behaviour with no way to detect whether its been violated or not.
I can check, but I don't see how a compiler would ever choose 'adrp' when it emits what amounts to a nop instruction.
Quoted 8 lines
Quoted 8 lines
arch/arm64/kernel/module-plts.c +0
-0@@ -41,6 +41,46 @@ u64 module_emit_plt_entry(struct module *mod, void *loc, const Elf64_Rela *rela,return (u64)&plt[i]; }
Quoted 15 lines
arch/arm64/kernel/module-plts.c u64 module_emit_plt_entry(struct module *mod, void *loc, const Elf64_Rela *rela, · line 54
Quoted 11 lines
arch/arm64/kernel/module-plts.c +11
-0+#ifdef CONFIG_ARM64_ERRATUM_843419 +u64 module_emit_adrp_veneer (struct module *mod,void *loc,u64 val) +{ +struct mod_plt_sec *pltsec = !in_init (mod, loc) ? &mod->arch.core : + &mod->arch.init; +struct plt_entry *plt = (struct plt_entry *)pltsec->plt->sh_addr; +int i = pltsec->plt_num_entries++; +u32 mov0, mov1, mov2, br; +int rd; + +BUG_ON (pltsec->plt_num_entries > pltsec->plt_max_entries);I'd prefer just to fail loading the module, but I see we already have a
BUG_ONfor the existing PLT code. Oh well.
I can fix that in a followup patch
Quoted 65 lines
Quoted 65 lines
arch/arm64/kernel/module-plts.c +41
-1+ +/* get the destination register of the ADRP instruction */ + rd =aarch64_insn_decode_register (AARCH64_INSN_REGTYPE_RD , +le32_to_cpup ((__le32 *)loc)); + +/* generate the veneer instructions */ + mov0 =aarch64_insn_gen_movewide (rd, (u16 )~val,0 , +AARCH64_INSN_VARIANT_64BIT , +AARCH64_INSN_MOVEWIDE_INVERSE ); + mov1 =aarch64_insn_gen_movewide (rd, (u16)(val >>16 ),16 , +AARCH64_INSN_VARIANT_64BIT , +AARCH64_INSN_MOVEWIDE_KEEP ); + mov2 =aarch64_insn_gen_movewide (rd, (u16)(val >>32 ),32 , +AARCH64_INSN_VARIANT_64BIT , +AARCH64_INSN_MOVEWIDE_KEEP ); + br =aarch64_insn_gen_branch_imm ((u64)&plt[i].br, (u64 )loc +4 , +AARCH64_INSN_BRANCH_NOLINK ); + + plt[i] = (struct plt_entry ){ +cpu_to_le32 (mov0), +cpu_to_le32 (mov1), +cpu_to_le32 (mov2), +cpu_to_le32 (br) + }; + + return (u64)&plt[i]; +} +#endif +#define cmp_3way (a,b) ((a) < (b) ? -1 : (a) > (b))static int cmp_rela (const void *a,const void *b)@@ -68,16 +108,21 @@ static bool duplicate_rel(const Elf64_Rela *rela, int num) }static unsigned int count_plts (Elf64_Sym *syms,Elf64_Rela *rela,int num, -Elf64_Word dstidx) +Elf64_Word dstidx,Elf_Shdr *dstsec) {unsigned int ret =0 ;Elf64_Sym *s;int i;for (i =0 ; i < num; i++) { +u64 min_align; +switch (ELF64_R_TYPE (rela[i].r_info)) {case R_AARCH64_JUMP26 :case R_AARCH64_CALL26 : +if (!IS_ENABLED (CONFIG_RANDOMIZE_BASE )) +break ; +/* * We only have to consider branch targets that resolve * to symbols that are defined in a different section. @@ -109,6 +154,31 @@ static unsigned int count_plts(Elf64_Sym *syms, Elf64_Rela *rela, int num,if (rela[i].r_addend !=0 || !duplicate_rel (rela, i)) ret++;break ; +case R_AARCH64_ADR_PREL_PG_HI21_NC : +case R_AARCH64_ADR_PREL_PG_HI21 : +if (!IS_ENABLED (CONFIG_ARM64_ERRATUM_843419 )) +break ; + +/*
Quoted 9 lines
arch/arm64/kernel/module-plts.c static unsigned int count_plts(Elf64_Sym *syms, Elf64_Rela *rela, int num, · line 167
Quoted 5 lines
arch/arm64/kernel/module-plts.c +5
-0+* Determine the minimal safe alignment for this ADRP +* instruction: the section alignment at which it is +* guaranteed not to appear at a vulnerable offset. +*/ + min_align =2 <<ffz (rela[i].r_offset |0x7 );I'm struggling to decipher this, can you give me a hint please? Why
0x7? Is the "2" because of the two vulnerable offsets?
What it basically does is preserve 0 bits in the address of the instruction.
Given that ADRP instructions at addresses ending in fff8 or fffc may be vulnerable, if any of the bits [11:3] are zero, we can increase the alignment of this section to ensure it remains zero, guaranteeing that the resulting address won't end in fff8 or fffc
Bits 0 .. 2 don't count, hence the 0x7. If bit 3 is zero, we can align to '1 << (3 + 1)' == '2 << 3', and bit 3 will remain zero.
RFC PATCH v3 2/3 arm64/kernel: don't ban ADRP to work around Cortex-A53 erratum #843419
Quoted 8 lines · Ard Biesheuvel
Quoted 7 lines · Will Deacon
Quoted 5 lines · 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)
Quoted 8 lines
Quoted 5 lines
Does the workaround currently implemented in the linker also make this same assumption? If not, I'm a little wary that we're making an assumption about compiler behaviour with no way to detect whether its been violated or not.
I can check, but I don't see how a compiler would ever choose 'adrp' when it emits what amounts to a nop instruction.
Agreed, but it wouldn't be the first time we've seen compilers do weird things.
Quoted 24 lines
Quoted 8 lines
Quoted 8 lines
arch/arm64/kernel/module-plts.c +0
-0@@ -41,6 +41,46 @@ u64 module_emit_plt_entry(struct module *mod, void *loc, const Elf64_Rela *rela,return (u64)&plt[i]; }Quoted 15 lines
arch/arm64/kernel/module-plts.c u64 module_emit_plt_entry(struct module *mod, void *loc, const Elf64_Rela *rela, · line 54
Quoted 11 lines
arch/arm64/kernel/module-plts.c +11
-0+#ifdef CONFIG_ARM64_ERRATUM_843419 +u64 module_emit_adrp_veneer (struct module *mod,void *loc,u64 val) +{ +struct mod_plt_sec *pltsec = !in_init (mod, loc) ? &mod->arch.core : + &mod->arch.init; +struct plt_entry *plt = (struct plt_entry *)pltsec->plt->sh_addr; +int i = pltsec->plt_num_entries++; +u32 mov0, mov1, mov2, br; +int rd; + +BUG_ON (pltsec->plt_num_entries > pltsec->plt_max_entries);I'd prefer just to fail loading the module, but I see we already have a
BUG_ONfor the existing PLT code. Oh well.
Quoted 1 line
I can fix that in a followup patch
Thanks.
Quoted 77 lines
Quoted 65 lines
Quoted 65 lines
arch/arm64/kernel/module-plts.c +41
-1+ +/* get the destination register of the ADRP instruction */ + rd =aarch64_insn_decode_register (AARCH64_INSN_REGTYPE_RD , +le32_to_cpup ((__le32 *)loc)); + +/* generate the veneer instructions */ + mov0 =aarch64_insn_gen_movewide (rd, (u16 )~val,0 , +AARCH64_INSN_VARIANT_64BIT , +AARCH64_INSN_MOVEWIDE_INVERSE ); + mov1 =aarch64_insn_gen_movewide (rd, (u16)(val >>16 ),16 , +AARCH64_INSN_VARIANT_64BIT , +AARCH64_INSN_MOVEWIDE_KEEP ); + mov2 =aarch64_insn_gen_movewide (rd, (u16)(val >>32 ),32 , +AARCH64_INSN_VARIANT_64BIT , +AARCH64_INSN_MOVEWIDE_KEEP ); + br =aarch64_insn_gen_branch_imm ((u64)&plt[i].br, (u64 )loc +4 , +AARCH64_INSN_BRANCH_NOLINK ); + + plt[i] = (struct plt_entry ){ +cpu_to_le32 (mov0), +cpu_to_le32 (mov1), +cpu_to_le32 (mov2), +cpu_to_le32 (br) + }; + + return (u64)&plt[i]; +} +#endif +#define cmp_3way (a,b) ((a) < (b) ? -1 : (a) > (b))static int cmp_rela (const void *a,const void *b)@@ -68,16 +108,21 @@ static bool duplicate_rel(const Elf64_Rela *rela, int num) }static unsigned int count_plts (Elf64_Sym *syms,Elf64_Rela *rela,int num, -Elf64_Word dstidx) +Elf64_Word dstidx,Elf_Shdr *dstsec) {unsigned int ret =0 ;Elf64_Sym *s;int i;for (i =0 ; i < num; i++) { +u64 min_align; +switch (ELF64_R_TYPE (rela[i].r_info)) {case R_AARCH64_JUMP26 :case R_AARCH64_CALL26 : +if (!IS_ENABLED (CONFIG_RANDOMIZE_BASE )) +break ; +/* * We only have to consider branch targets that resolve * to symbols that are defined in a different section. @@ -109,6 +154,31 @@ static unsigned int count_plts(Elf64_Sym *syms, Elf64_Rela *rela, int num,if (rela[i].r_addend !=0 || !duplicate_rel (rela, i)) ret++;break ; +case R_AARCH64_ADR_PREL_PG_HI21_NC : +case R_AARCH64_ADR_PREL_PG_HI21 : +if (!IS_ENABLED (CONFIG_ARM64_ERRATUM_843419 )) +break ; + +/* Quoted 9 lines
arch/arm64/kernel/module-plts.c static unsigned int count_plts(Elf64_Sym *syms, Elf64_Rela *rela, int num, · line 167
Quoted 5 lines
arch/arm64/kernel/module-plts.c +5
-0+* Determine the minimal safe alignment for this ADRP +* instruction: the section alignment at which it is +* guaranteed not to appear at a vulnerable offset. +*/ + min_align =2 <<ffz (rela[i].r_offset |0x7 );I'm struggling to decipher this, can you give me a hint please? Why
0x7? Is the "2" because of the two vulnerable offsets?What it basically does is preserve 0 bits in the address of the instruction.
Quoted 7 lines
Given that ADRP instructions at addresses ending in fff8 or fffc may be vulnerable, if any of the bits [11:3] are zero, we can increase the alignment of this section to ensure it remains zero, guaranteeing that the resulting address won't end in fff8 or fffc
Bits 0 .. 2 don't count, hence the
0x7. If bit 3 is zero, we can align to '1 << (3 + 1)' == '2 << 3', and bit 3 will remain zero.
Please put this in a comment ;)
Will
RFC PATCH v3 2/3 arm64/kernel: don't ban ADRP to work around Cortex-A53 erratum #843419
Quoted 15 lines · Will Deacon
Quoted 14 lines · Ard Biesheuvel
Quoted 12 lines · Will Deacon
Quoted 5 lines · 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)
Does the workaround currently implemented in the linker also make this same assumption? If not, I'm a little wary that we're making an assumption about compiler behaviour with no way to detect whether its been violated or not.
Quoted 6 lines
Quoted 2 lines
I can check, but I don't see how a compiler would ever choose 'adrp' when it emits what amounts to a nop instruction.
Agreed, but it wouldn't be the first time we've seen compilers do weird things.
I just checked ld.bfd: it only checks for sequence 1, which is the one that affects ADRP instructions at offsets 0xfff8/0xfffc modulo 4 KB. It actually checks more elaborately whether the sequence is in fact vulnerable rather than assuming any ADRP instruction at such an offset may be vulnerable, but in my testing of the current patches, I only get a handful of them anyway of which the majority are resolved by patching ADRP->ADR.
Sequence 2 is described as follows in the docs:
"""
Sequence 2:
1) An ADRP instruction, which writes to a register Rn.
2) Another instruction which writes to Rn.
o This cannot be a branch or an ADRP.
o This cannot read Rn.
3)
4)
Another instruction.
o This cannot be a branch.
o This cannot write Rn.
o This may optionally read Rn.
A load or store instruction from the "Load/store register (unsigned immediate)" encoding class, using Rn as the
base address register.
"""
and ld.bfd does not check for it at all.
Quoted 110 lines
Quoted 24 lines
Quoted 8 lines
Quoted 8 lines
arch/arm64/kernel/module-plts.c +0
-0@@ -41,6 +41,46 @@ u64 module_emit_plt_entry(struct module *mod, void *loc, const Elf64_Rela *rela,return (u64)&plt[i]; }Quoted 15 lines
arch/arm64/kernel/module-plts.c u64 module_emit_plt_entry(struct module *mod, void *loc, const Elf64_Rela *rela, · line 54
Quoted 11 lines
arch/arm64/kernel/module-plts.c +11
-0+#ifdef CONFIG_ARM64_ERRATUM_843419 +u64 module_emit_adrp_veneer (struct module *mod,void *loc,u64 val) +{ +struct mod_plt_sec *pltsec = !in_init (mod, loc) ? &mod->arch.core : + &mod->arch.init; +struct plt_entry *plt = (struct plt_entry *)pltsec->plt->sh_addr; +int i = pltsec->plt_num_entries++; +u32 mov0, mov1, mov2, br; +int rd; + +BUG_ON (pltsec->plt_num_entries > pltsec->plt_max_entries);I'd prefer just to fail loading the module, but I see we already have a
BUG_ONfor the existing PLT code. Oh well.Quoted 1 line
I can fix that in a followup patch
Thanks.
Quoted 82 lines
Quoted 65 lines
Quoted 65 lines
arch/arm64/kernel/module-plts.c +41
-1+ +/* get the destination register of the ADRP instruction */ + rd =aarch64_insn_decode_register (AARCH64_INSN_REGTYPE_RD , +le32_to_cpup ((__le32 *)loc)); + +/* generate the veneer instructions */ + mov0 =aarch64_insn_gen_movewide (rd, (u16 )~val,0 , +AARCH64_INSN_VARIANT_64BIT , +AARCH64_INSN_MOVEWIDE_INVERSE ); + mov1 =aarch64_insn_gen_movewide (rd, (u16)(val >>16 ),16 , +AARCH64_INSN_VARIANT_64BIT , +AARCH64_INSN_MOVEWIDE_KEEP ); + mov2 =aarch64_insn_gen_movewide (rd, (u16)(val >>32 ),32 , +AARCH64_INSN_VARIANT_64BIT , +AARCH64_INSN_MOVEWIDE_KEEP ); + br =aarch64_insn_gen_branch_imm ((u64)&plt[i].br, (u64 )loc +4 , +AARCH64_INSN_BRANCH_NOLINK ); + + plt[i] = (struct plt_entry ){ +cpu_to_le32 (mov0), +cpu_to_le32 (mov1), +cpu_to_le32 (mov2), +cpu_to_le32 (br) + }; + + return (u64)&plt[i]; +} +#endif +#define cmp_3way (a,b) ((a) < (b) ? -1 : (a) > (b))static int cmp_rela (const void *a,const void *b)@@ -68,16 +108,21 @@ static bool duplicate_rel(const Elf64_Rela *rela, int num) }static unsigned int count_plts (Elf64_Sym *syms,Elf64_Rela *rela,int num, -Elf64_Word dstidx) +Elf64_Word dstidx,Elf_Shdr *dstsec) {unsigned int ret =0 ;Elf64_Sym *s;int i;for (i =0 ; i < num; i++) { +u64 min_align; +switch (ELF64_R_TYPE (rela[i].r_info)) {case R_AARCH64_JUMP26 :case R_AARCH64_CALL26 : +if (!IS_ENABLED (CONFIG_RANDOMIZE_BASE )) +break ; +/* * We only have to consider branch targets that resolve * to symbols that are defined in a different section. @@ -109,6 +154,31 @@ static unsigned int count_plts(Elf64_Sym *syms, Elf64_Rela *rela, int num,if (rela[i].r_addend !=0 || !duplicate_rel (rela, i)) ret++;break ; +case R_AARCH64_ADR_PREL_PG_HI21_NC : +case R_AARCH64_ADR_PREL_PG_HI21 : +if (!IS_ENABLED (CONFIG_ARM64_ERRATUM_843419 )) +break ; + +/* Quoted 9 lines
arch/arm64/kernel/module-plts.c static unsigned int count_plts(Elf64_Sym *syms, Elf64_Rela *rela, int num, · line 167
Quoted 5 lines
arch/arm64/kernel/module-plts.c +5
-0+* Determine the minimal safe alignment for this ADRP +* instruction: the section alignment at which it is +* guaranteed not to appear at a vulnerable offset. +*/ + min_align =2 <<ffz (rela[i].r_offset |0x7 );I'm struggling to decipher this, can you give me a hint please? Why
0x7? Is the "2" because of the two vulnerable offsets?What it basically does is preserve 0 bits in the address of the instruction.
Given that ADRP instructions at addresses ending in fff8 or fffc may be vulnerable, if any of the bits [11:3] are zero, we can increase the alignment of this section to ensure it remains zero, guaranteeing that the resulting address won't end in fff8 or fffc
Quoted 5 lines
Quoted 2 lines
Bits 0 .. 2 don't count, hence the
0x7. If bit 3 is zero, we can align to '1 << (3 + 1)' == '2 << 3', and bit 3 will remain zero.Please put this in a comment ;)
OK
RFC PATCH v3 2/3 arm64/kernel: don't ban ADRP to work around Cortex-A53 erratum #843419
Quoted 50 lines · Ard Biesheuvel
Quoted 15 lines · Will Deacon
Quoted 14 lines · Ard Biesheuvel
Quoted 12 lines · Will Deacon
Quoted 5 lines · 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)
Does the workaround currently implemented in the linker also make this same assumption? If not, I'm a little wary that we're making an assumption about compiler behaviour with no way to detect whether its been violated or not.
Quoted 6 lines
Quoted 2 lines
I can check, but I don't see how a compiler would ever choose 'adrp' when it emits what amounts to a nop instruction.
Agreed, but it wouldn't be the first time we've seen compilers do weird things.
I just checked ld.bfd: it only checks for sequence 1, which is the one that affects ADRP instructions at offsets 0xfff8/0xfffc modulo 4 KB. It actually checks more elaborately whether the sequence is in fact vulnerable rather than assuming any ADRP instruction at such an offset may be vulnerable, but in my testing of the current patches, I only get a handful of them anyway of which the majority are resolved by patching
ADRP->ADR.Sequence 2 is described as follows in the docs:
"""
Sequence 2:
1) An ADRP instruction, which writes to a register Rn.
2) Another instruction which writes to Rn.
o This cannot be a branch or an ADRP.
o This cannot read Rn.
3)
4)
Another instruction.
o This cannot be a branch.
o This cannot write Rn.
o This may optionally read Rn.
A load or store instruction from the "Load/store register (unsigned immediate)" encoding class, using Rn as the
base address register.
"""
Quoted 1 line
and ld.bfd does not check for it at all.
In which case, the code you're proposing for modules is just as good as what we're doing for the kernel text itself. Might be worth mentioning that somewhere, but I think it's fine.
Thanks for looking,
Will
RFC PATCH v3 3/3 arm64/kernel: enable A53 erratum #8434319 handling at runtime
In reply to Ard BiesheuvelOmit patching of ADRP instruction at module load time if the current CPUs are not susceptible to the erratum.
---
Open question: how should we handle big.LITTLE configurations where affected Cortex-A53s may appear late.
arch/arm64/include/asm/cpucaps.h | 3 ++- arch/arm64/kernel/cpu_errata.c | 26 ++++++++++++++++++++ arch/arm64/kernel/module-plts.c | 3 ++- arch/arm64/kernel/module.c | 3 ++- 4 files changed, 32 insertions(+), 3 deletions(-)
arch/arm64/include/asm/cpucaps.h +2 -1
@@ -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 +26 -0
@@ -228,6 +228,23 @@ static int qcom_enable_link_stack_sanitization(void *data) }#endif /* CONFIG_HARDEN_BRANCH_PREDICTOR */ +static bool __maybe_unused +needs_erratum_843419_workaround (const struct arm64_cpu_capabilities *entry, +int scope) +{ +u32 cpuid =read_cpuid_id (); + +WARN_ON (scope !=SCOPE_LOCAL_CPU ||preemptible ()); + +if ((cpuid &MIDR_CPU_MODEL_MASK ) !=MIDR_CORTEX_A53 ) +return false; +else if ((cpuid & (MIDR_REVISION_MASK |MIDR_VARIANT_MASK )) ==0x4 ) +/* erratum was fixed in some versions of r0p4 */ +return !(read_cpuid (REVIDR_EL1 ) &BIT (8 )); +else +return true; +} +#define MIDR_RANGE (model, min, max) \ .def_scope = SCOPE_LOCAL_CPU, \ .matches = is_affected_midr_range, \
@@ -283,6 +300,15 @@ 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, + .matches = needs_erratum_843419_workaround, + }, +#endif #ifdef CONFIG_ARM64_ERRATUM_845719 { /* Cortex-A53 r0p[01234] */
arch/arm64/kernel/module-plts.c +2 -1
@@ -156,7 +156,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 +2 -1
@@ -201,7 +201,8 @@ static int reloc_insn_imm(enum aarch64_reloc_op op, __le32 *place, u64 val,static bool reloc_adrp_erratum_843419 (struct module *mod,__le32 *place,u64 val) { -if (!IS_ENABLED (CONFIG_ARM64_ERRATUM_843419 )) +if (!IS_ENABLED (CONFIG_ARM64_ERRATUM_843419 ) || + !cpus_have_const_cap (ARM64_WORKAROUND_843419 ))return false;/* only ADRP instructions at the end of a 4k page are affected */
Signature · 1 line
2.11.0
Mark Rutland
reviewerRFC PATCH v3 3/3 arm64/kernel: enable A53 erratum #8434319 handling at runtime
Quoted 7 lines · Ard Biesheuvel
Omit patching of ADRP instruction at module load time if the current CPUs are not susceptible to the erratum.
Signed-off-by Ard Biesheuvel <[email protected]>---
Open question: how should we handle big.LITTLE configurations where affected Cortex-A53s may appear late.
I think verify_local_cpu_errata_workarounds() will handle that for us, aborting the late onlining of a CPU with an erratum a boot CPU didn't have.
[...]
Quoted 6 lines
arch/arm64/kernel/cpu_errata.c +6
-0+static bool __maybe_unused +needs_erratum_843419_workaround (const struct arm64_cpu_capabilities *entry, +int scope) +{ +u32 cpuid =read_cpuid_id (); +
arch/arm64/kernel/cpu_errata.c static int qcom_enable_link_stack_sanitization(void *data) · line 246
Quoted 10 lines
arch/arm64/kernel/cpu_errata.c +10
-0+WARN_ON (scope !=SCOPE_LOCAL_CPU ||preemptible ()); + +if ((cpuid &MIDR_CPU_MODEL_MASK ) !=MIDR_CORTEX_A53 ) +return false; +else if ((cpuid & (MIDR_REVISION_MASK |MIDR_VARIANT_MASK )) ==0x4 ) +/* erratum was fixed in some versions of r0p4 */ +return !(read_cpuid (REVIDR_EL1 ) &BIT (8 )); +else +return true; +}
This would be easier to read as:
if ((cpuid & MIDR_CPU_MODEL_MASK) != MIDR_CORTEX_A53) return false; /* erratum was fixed in some versions of r0p4 */ if ((cpuid & (MIDR_REVISION_MASK | MIDR_VARIANT_MASK)) == 0x4) return !(read_cpuid(REVIDR_EL1) & BIT(8)); return true;
Thanks,
Mark.
RFC PATCH v3 3/3 arm64/kernel: enable A53 erratum #8434319 handling at runtime
In reply to Ard BiesheuvelQuoted 7 lines · Ard Biesheuvel
Omit patching of ADRP instruction at module load time if the current CPUs are not susceptible to the erratum.
Signed-off-by Ard Biesheuvel <[email protected]>---
Open question: how should we handle big.LITTLE configurations where affected Cortex-A53s may appear late.
We should fail to bring them online. I think the infrastructure already exists for this and we used it for other errata already.
Quoted 25 lines
arch/arm64/include/asm/cpucaps.h +2
-1@@ -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 +3
-0@@ -228,6 +228,23 @@ static int qcom_enable_link_stack_sanitization(void *data) }#endif /* CONFIG_HARDEN_BRANCH_PREDICTOR */ +static bool __maybe_unused +needs_erratum_843419_workaround (const struct arm64_cpu_capabilities *entry, +int scope)
arch/arm64/kernel/cpu_errata.c static int qcom_enable_link_stack_sanitization(void *data) · line 243
Quoted 10 lines
arch/arm64/kernel/cpu_errata.c +10
-0+{ +u32 cpuid =read_cpuid_id (); + +WARN_ON (scope !=SCOPE_LOCAL_CPU ||preemptible ()); + +if ((cpuid &MIDR_CPU_MODEL_MASK ) !=MIDR_CORTEX_A53 ) +return false; +else if ((cpuid & (MIDR_REVISION_MASK |MIDR_VARIANT_MASK )) ==0x4 ) +/* erratum was fixed in some versions of r0p4 */ +return !(read_cpuid (REVIDR_EL1 ) &BIT (8 ));
The rXpY information is in the MIDR, so this is checking for something else afaict.
Will
RFC PATCH v3 3/3 arm64/kernel: enable A53 erratum #8434319 handling at runtime
Quoted 4 lines · Will Deacon
Quoted 3 lines · Ard Biesheuvel
Omit patching of ADRP instruction at module load time if the current CPUs are not susceptible to the erratum.
Quoted 8 lines
Quoted 4 lines
Signed-off-by Ard Biesheuvel <[email protected]>---
Open question: how should we handle big.LITTLE configurations where affected Cortex-A53s may appear late.We should fail to bring them online. I think the infrastructure already exists for this and we used it for other errata already.
I think that is what it does currently. The question is whether that should be considered a regression or not.
Quoted 22 lines
Quoted 22 lines
arch/arm64/include/asm/cpucaps.h +2
-1@@ -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 +0
-0@@ -228,6 +228,23 @@ static int qcom_enable_link_stack_sanitization(void *data) }#endif /* CONFIG_HARDEN_BRANCH_PREDICTOR */
Quoted 17 lines
arch/arm64/kernel/cpu_errata.c static int qcom_enable_link_stack_sanitization(void *data) · line 243
Quoted 13 lines
arch/arm64/kernel/cpu_errata.c +13
-0+static bool __maybe_unused +needs_erratum_843419_workaround (const struct arm64_cpu_capabilities *entry, +int scope) +{ +u32 cpuid =read_cpuid_id (); + +WARN_ON (scope !=SCOPE_LOCAL_CPU ||preemptible ()); + +if ((cpuid &MIDR_CPU_MODEL_MASK ) !=MIDR_CORTEX_A53 ) +return false; +else if ((cpuid & (MIDR_REVISION_MASK |MIDR_VARIANT_MASK )) ==0x4 ) +/* erratum was fixed in some versions of r0p4 */ +return !(read_cpuid (REVIDR_EL1 ) &BIT (8 ));The rXpY information is in the MIDR, so this is checking for something else afaict.
No, it checks the REVIDR of r0p4 parts, of which bit 8 tells us if this specific erratum has been fixed.
RFC PATCH v3 3/3 arm64/kernel: enable A53 erratum #8434319 handling at runtime
Quoted 10 lines · Ard Biesheuvel
Quoted 9 lines · Will Deacon
Quoted 7 lines · Ard Biesheuvel
Omit patching of ADRP instruction at module load time if the current CPUs are not susceptible to the erratum.
Signed-off-by Ard Biesheuvel <[email protected]>---
Open question: how should we handle big.LITTLE configurations where affected Cortex-A53s may appear late.
Quoted 6 lines
Quoted 3 lines
We should fail to bring them online. I think the infrastructure already exists for this and we used it for other errata already.
I think that is what it does currently. The question is whether that should be considered a regression or not.
This can only happen with maxcpus=, right? I wouldn't be too worried about it in that case.
Quoted 41 lines
Quoted 23 lines
Quoted 23 lines
arch/arm64/include/asm/cpucaps.h +2
-1@@ -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 +0
-0@@ -228,6 +228,23 @@ static int qcom_enable_link_stack_sanitization(void *data) }#endif /* CONFIG_HARDEN_BRANCH_PREDICTOR */ Quoted 17 lines
arch/arm64/kernel/cpu_errata.c static int qcom_enable_link_stack_sanitization(void *data) · line 243
Quoted 13 lines
arch/arm64/kernel/cpu_errata.c +13
-0+static bool __maybe_unused +needs_erratum_843419_workaround (const struct arm64_cpu_capabilities *entry, +int scope) +{ +u32 cpuid =read_cpuid_id (); + +WARN_ON (scope !=SCOPE_LOCAL_CPU ||preemptible ()); + +if ((cpuid &MIDR_CPU_MODEL_MASK ) !=MIDR_CORTEX_A53 ) +return false; +else if ((cpuid & (MIDR_REVISION_MASK |MIDR_VARIANT_MASK )) ==0x4 ) +/* erratum was fixed in some versions of r0p4 */ +return !(read_cpuid (REVIDR_EL1 ) &BIT (8 ));The rXpY information is in the MIDR, so this is checking for something else afaict.
Quoted 2 lines
No, it checks the REVIDR of r0p4 parts, of which bit 8 tells us if this specific erratum has been fixed.
/me checks errata notice.
Ok, fair enough! Given that it looks like this mechanism is used for other errata too, it would make sense to support it in the arm64_cpu_capabilities structure alongside the midr stuff.
Will
RFC PATCH v3 3/3 arm64/kernel: enable A53 erratum #8434319 handling at runtime
Quoted 62 lines · Will Deacon
Quoted 10 lines · Ard Biesheuvel
Quoted 9 lines · Will Deacon
Quoted 7 lines · Ard Biesheuvel
Omit patching of ADRP instruction at module load time if the current CPUs are not susceptible to the erratum.
Signed-off-by Ard Biesheuvel <[email protected]>---
Open question: how should we handle big.LITTLE configurations where affected Cortex-A53s may appear late.Quoted 6 lines
Quoted 3 lines
We should fail to bring them online. I think the infrastructure already exists for this and we used it for other errata already.
I think that is what it does currently. The question is whether that should be considered a regression or not.
This can only happen with maxcpus=, right? I wouldn't be too worried about it in that case.
Quoted 41 lines
Quoted 40 lines
Quoted 26 lines
arch/arm64/include/asm/cpucaps.h +2
-1@@ -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 +3
-0@@ -228,6 +228,23 @@ static int qcom_enable_link_stack_sanitization(void *data) }#endif /* CONFIG_HARDEN_BRANCH_PREDICTOR */ +static bool __maybe_unused +needs_erratum_843419_workaround (const struct arm64_cpu_capabilities *entry, +int scope)arch/arm64/kernel/cpu_errata.c static int qcom_enable_link_stack_sanitization(void *data) · line 243
Quoted 10 lines
arch/arm64/kernel/cpu_errata.c +10
-0+{ +u32 cpuid =read_cpuid_id (); + +WARN_ON (scope !=SCOPE_LOCAL_CPU ||preemptible ()); + +if ((cpuid &MIDR_CPU_MODEL_MASK ) !=MIDR_CORTEX_A53 ) +return false; +else if ((cpuid & (MIDR_REVISION_MASK |MIDR_VARIANT_MASK )) ==0x4 ) +/* erratum was fixed in some versions of r0p4 */ +return !(read_cpuid (REVIDR_EL1 ) &BIT (8 ));The rXpY information is in the MIDR, so this is checking for something else afaict.
Quoted 9 lines
Quoted 2 lines
No, it checks the REVIDR of r0p4 parts, of which bit 8 tells us if this specific erratum has been fixed.
/me checks errata notice.
Ok, fair enough! Given that it looks like this mechanism is used for other errata too, it would make sense to support it in the
arm64_cpu_capabilitiesstructure alongside the midr stuff.
Are you saying I should abstract REVIDR access for this series? I don't mind, but in that case, could you be a bit clearer about what you would like to see?
RFC PATCH v3 3/3 arm64/kernel: enable A53 erratum #8434319 handling at runtime
Quoted 35 lines · Ard Biesheuvel
Quoted 34 lines · Will Deacon
Quoted 28 lines · Ard Biesheuvel
Quoted 9 lines · Will Deacon
Quoted 8 lines · Ard Biesheuvel
arch/arm64/kernel/cpu_errata.c +0
-0@@ -228,6 +228,23 @@ static int qcom_enable_link_stack_sanitization(void *data) }#endif /* CONFIG_HARDEN_BRANCH_PREDICTOR */ Quoted 17 lines
arch/arm64/kernel/cpu_errata.c static int qcom_enable_link_stack_sanitization(void *data) · line 243
Quoted 13 lines
arch/arm64/kernel/cpu_errata.c +13
-0+static bool __maybe_unused +needs_erratum_843419_workaround (const struct arm64_cpu_capabilities *entry, +int scope) +{ +u32 cpuid =read_cpuid_id (); + +WARN_ON (scope !=SCOPE_LOCAL_CPU ||preemptible ()); + +if ((cpuid &MIDR_CPU_MODEL_MASK ) !=MIDR_CORTEX_A53 ) +return false; +else if ((cpuid & (MIDR_REVISION_MASK |MIDR_VARIANT_MASK )) ==0x4 ) +/* erratum was fixed in some versions of r0p4 */ +return !(read_cpuid (REVIDR_EL1 ) &BIT (8 ));The rXpY information is in the MIDR, so this is checking for something else afaict.
Quoted 2 lines
No, it checks the REVIDR of r0p4 parts, of which bit 8 tells us if this specific erratum has been fixed.
/me checks errata notice.
Quoted 8 lines
Quoted 4 lines
Ok, fair enough! Given that it looks like this mechanism is used for other errata too, it would make sense to support it in the
arm64_cpu_capabilitiesstructure alongside the midr stuff.Are you saying I should abstract REVIDR access for this series? I don't mind, but in that case, could you be a bit clearer about what you would like to see?
Sorry, yes. I was thinking about having a revidr mask/value pair field in the arm64_cpu_capabilities structure which, if set to a non-zero value by the errata entry, is then used as a secondary check if the MIDR match indicates that the CPU is affected by the erratum. A quick look at some of our errata notices suggests that this can then be used for some other Cortex cores as well.
Feel free to do it as a follow-up patch if it's easier.
Will
v3 cover arm64/kernel: get rid of GCC large model code
In reply to Ard BiesheuvelQuoted 11 lines · Ard Biesheuvel
I am resending this as an RFC, because I'd like to understand whether anyone else shares my concern, or whether I am being overly paranoid.
v2 blurb:
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.
Quoted 7 lines
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
I think this is a sensible thing to do, cheers.
Will