arm64: Dynamic shadow call stack fixes
cover arm64: Dynamic shadow call stack fixes
From: Ard Biesheuvel <[email protected]>
When building modules using the large code model, the emitted DWARF metadata uses large fields for the code references too, and this is currently not being handled.
The CIE header frame contains metadata that describes the size of these fields, and so this should be parsed and taken into account when processing the FDE frames (patch #2)
But first, the handling of this CIE frame header needs to be fixed, as it currently [inadvertently] used hardcoded defaults for some values that happen to always match what Clang emits in practice (patch #1)
Ard Biesheuvel (3):
arm64/scs: Fix handling of DWARF augmentation data in CIE/FDE frames arm64/scs: Deal with 64-bit relative offsets in FDE frames arm64/scs: Drop unused prototype __pi_scs_patch_vmlinux()
arch/arm64/include/asm/scs.h | 8 +- arch/arm64/kernel/module.c | 10 ++- arch/arm64/kernel/pi/patch-scs.c | 93 ++++++++++++++------ 3 files changed, 80 insertions(+), 31 deletions(-)
Signature · 1 line
2.47.0.277.g8800431eea-goog
PATCH 1/3 arm64/scs: Fix handling of DWARF augmentation data in CIE/FDE frames
From: Ard Biesheuvel <[email protected]>
The dynamic SCS patching code pretends to parse the DWARF augmentation data in the CIE (header) frame, and handle accordingly when processing the individual FDE frames based on this CIE frame. However, the boolean variable is defined inside the loop, and so the parsed value is ignored.
The same applies to the code alignment field, which is also read from the header but then discarded.
This was never spotted before because Clang is the only compiler that supports dynamic SCS patching (which is essentially an Android feature), and the unwind tables it produces are highly uniform, and match the de facto defaults.
So instead of testing for the 'z' flag in the augmentation data field, require a fixed augmentation data string of 'zR', and simplify the rest of the code accordingly.
Also introduce some error codes to specify why the patching failed, and log it to the kernel console on failure when this happens when loading a module. (Doing so for vmlinux is infeasible, as the patching is done extremely early in the boot.)
--- arch/arm64/include/asm/scs.h | 7 +++ arch/arm64/kernel/module.c | 10 +++- arch/arm64/kernel/pi/patch-scs.c | 63 +++++++++++--------- 3 files changed, 50 insertions(+), 30 deletions(-)
arch/arm64/include/asm/scs.h +7 -0
@@ -46,6 +46,13 @@ static inline void dynamic_scs_init(void)static inline void dynamic_scs_init (void ) {}#endif +enum { +EDYNSCS_INVALID_CIE_HEADER =1 , +EDYNSCS_INVALID_CIE_SDATA_SIZE =2 , +EDYNSCS_INVALID_FDE_AUGM_DATA_SIZE =3 , +EDYNSCS_INVALID_CFA_OPCODE =4 , +}; +int __pi_scs_patch (const u8 eh_frame[],int size); asmlinkage void __pi_scs_patch_vmlinux(void);
arch/arm64/kernel/module.c +8 -2
@@ -462,14 +462,20 @@ int module_finalize(const Elf_Ehdr *hdr, struct module *me) {const Elf_Shdr *s; +int ret; + s =find_section (hdr, sechdrs,".altinstructions" );if (s)apply_alternatives_module ((void *)s->sh_addr, s->sh_size);if (scs_is_dynamic ()) { s =find_section (hdr, sechdrs,".init.eh_frame" ); -if (s) -__pi_scs_patch ((void *)s->sh_addr, s->sh_size); +if (s) { + ret =__pi_scs_patch ((void *)s->sh_addr, s->sh_size); +if (ret) +pr_err ("module %s: error occurred during dynamic SCS patching (%d)\n" , + me->name, ret); + } }return module_init_ftrace_plt (hdr, sechdrs, me);
arch/arm64/kernel/pi/patch-scs.c +35 -28
114 lines not on this page · raw message
Signature · 1 line
2.47.0.277.g8800431eea-goog
Re: [PATCH 1/3] arm64/scs: Fix handling of DWARF augmentation data in CIE/FDE frames
Quoted 5 lines · Ard Biesheuvel
Also introduce some error codes to specify why the patching failed, and log it to the kernel console on failure when this happens when loading a module. (Doing so for vmlinux is infeasible, as the patching is done extremely early in the boot.)
It's a bit unfortunate that we might fail quietly for vmlinux. Could we store scs_patch() return value somewhere and print out a warning in dynamic_scs_init() if something went wrong, for example?
Sami
Re: [PATCH 1/3] arm64/scs: Fix handling of DWARF augmentation data in CIE/FDE frames
Quoted 12 lines · Sami Tolvanen
Quoted 5 lines · Ard Biesheuvel
Also introduce some error codes to specify why the patching failed, and log it to the kernel console on failure when this happens when loading a module. (Doing so for vmlinux is infeasible, as the patching is done extremely early in the boot.)
It's a bit unfortunate that we might fail quietly for vmlinux. Could we store
scs_patch()return value somewhere and print out a warning indynamic_scs_init()if something went wrong, for example?
Yeah good point.
PATCH 2/3 arm64/scs: Deal with 64-bit relative offsets in FDE frames
In reply to Ard BiesheuvelFrom: Ard Biesheuvel <[email protected]>
In some cases, the compiler may decide to emit DWARF FDE frames with 64-bit signed fields for the code offset and range fields. This may happen when using the large code model, for instance, which permits an executable to be spread out over more than 4 GiB of address space.
Whether this is the case can be inferred from the augmentation data in the CIE frame, so decode this data before processing the FDE frames.
--- arch/arm64/kernel/pi/patch-scs.c | 34 ++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-)
arch/arm64/kernel/pi/patch-scs.c +32 -2
@@ -50,6 +50,10 @@ bool dynamic_scs_is_enabled;#define DW_CFA_GNU_negative_offset_extended 0x2f#define DW_CFA_hi_user 0x3f +#define DW_EH_PE_sdata4 0x0b +#define DW_EH_PE_sdata8 0x0c +#define DW_EH_PE_pcrel 0x10 +enum {PACIASP =0xd503233f ,AUTIASP =0xd50323bf ,
@@ -125,6 +129,7 @@ struct eh_frame {u8 data_alignment_factor;u8 return_address_register;u8 augmentation_data_size; +u8 fde_pointer_format; };struct {// FDE
@@ -132,11 +137,18 @@ struct eh_frame {s32 range;u8 opcodes[]; }; + +struct {// FDE +s64 initial_loc64; +s64 range64; +u8 opcodes64[]; + }; }; };static int scs_handle_fde_frame (const struct eh_frame *frame,int code_alignment_factor, +bool use_sdata8,bool dry_run) {int size = frame->size - offsetof(struct eh_frame , opcodes) +4 ;
@@ -144,6 +156,12 @@ static int scs_handle_fde_frame(const struct eh_frame *frame,const u8 *opcode = frame->opcodes;int l; +if (use_sdata8) { + loc = (u64 )&frame->initial_loc64 + frame->initial_loc64; + opcode = frame->opcodes64; + size -=8 ; + } +// assume single byte uleb128_t for augmentation data size if (*opcode &BIT (7 ))return EDYNSCS_INVALID_FDE_AUGM_DATA_SIZE ;
@@ -210,6 +228,7 @@ static int scs_handle_fde_frame(const struct eh_frame *frame,int scs_patch (const u8 eh_frame[],int size) {int code_alignment_factor =1 ; +bool fde_use_sdata8 = false;const u8 *p = eh_frame;while (size >4 ) {
@@ -245,13 +264,24 @@ int scs_patch(const u8 eh_frame[], int size)return EDYNSCS_INVALID_CIE_HEADER ; code_alignment_factor = frame->code_alignment_factor; + +switch (frame->fde_pointer_format) { +case DW_EH_PE_pcrel | DW_EH_PE_sdata4: + fde_use_sdata8 = false; +break ; +case DW_EH_PE_pcrel | DW_EH_PE_sdata8: + fde_use_sdata8 = true; +break ; +default : +return EDYNSCS_INVALID_CIE_SDATA_SIZE ; + } }else { ret =scs_handle_fde_frame (frame, code_alignment_factor, - true); + fde_use_sdata8, true);if (ret)return ret;scs_handle_fde_frame (frame, code_alignment_factor, - false); + fde_use_sdata8, false); } p +=sizeof (frame->size) + frame->size;
Signature · 1 line
2.47.0.277.g8800431eea-goog
PATCH 3/3 arm64/scs: Drop unused prototype __pi_scs_patch_vmlinux()
In reply to Ard BiesheuvelFrom: Ard Biesheuvel <[email protected]>
The function scs_patch_vmlinux() was removed in the LPA2 boot code refactoring so remove the declaration as well.
--- arch/arm64/include/asm/scs.h | 1 - 1 file changed, 1 deletion(-)
arch/arm64/include/asm/scs.h +0 -1
@@ -54,7 +54,6 @@ enum { };int __pi_scs_patch (const u8 eh_frame[],int size); -asmlinkage void __pi_scs_patch_vmlinux(void);#endif /* __ASSEMBLY __ */
Signature · 1 line
2.47.0.277.g8800431eea-goog
Hi Ard,
Quoted 24 lines · Ard Biesheuvel
From: Ard Biesheuvel <[email protected]>
When building modules using the large code model, the emitted DWARF metadata uses large fields for the code references too, and this is currently not being handled.
The CIE header frame contains metadata that describes the size of these fields, and so this should be parsed and taken into account when processing the FDE frames (patch #2)
But first, the handling of this CIE frame header needs to be fixed, as it currently [inadvertently] used hardcoded defaults for some values that happen to always match what Clang emits in practice (patch #1)
Cc Sami Tolvanen <[email protected]>Cc Kees Cook <[email protected]>Cc Nathan Chancellor <[email protected]>Ard Biesheuvel (3):
arm64/scs: Fix handling of DWARF augmentation data in CIE/FDE frames arm64/scs: Deal with 64-bit relative offsets in FDE frames arm64/scs: Drop unused prototype__pi_scs_patch_vmlinux()
Quoted 4 lines
arch/arm64/include/asm/scs.h | 8 +- arch/arm64/kernel/module.c | 10 ++- arch/arm64/kernel/pi/patch-scs.c | 93 ++++++++++++++------ 3 files changed, 80 insertions(+), 31 deletions(-)
I was able to reproduce the issue by building modules with -mcmodel=large, and I confirmed that this series fixes it. For the series:
Sami
Quoted 9 lines · Ard Biesheuvel
When building modules using the large code model, the emitted DWARF metadata uses large fields for the code references too, and this is currently not being handled.
The CIE header frame contains metadata that describes the size of these fields, and so this should be parsed and taken into account when processing the FDE frames (patch #2)
[...]
Applied to arm64 (for-next/scs), thanks!
[1/3] arm64/scs: Fix handling of DWARF augmentation data in CIE/FDE frames
https://git.kernel.org/arm64/c/ccf54058f532 [2/3] arm64/scs: Deal with 64-bit relative offsets in FDE frames https://git.kernel.org/arm64/c/60de7a647fc5 [3/3] arm64/scs: Drop unused prototype __pi_scs_patch_vmlinux() https://git.kernel.org/arm64/c/47965a49a2c8
Signature · 1 line
Catalin