Skip to content

[DRAFT]gh-128605: Add branch protections for aarch64 in asm_trampoline.S #130864

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[DRAFT]gh-128605: Add branch protections for aarch64 in asm_trampoline.S #130864

wants to merge 2 commits into from
+60 −0

Conversation

stratakis
Copy link
Contributor

@stratakis stratakis commented Mar 5, 2025 •
edited by bedevere-app bot
Loading

@stratakis
Apply Intel Control-flow Technology for x86-64 on asm_trampoline.S
dcc714a 
Intel's Control-flow Technology requires manual application
for the assembler files.

See also: https://sourceware.org/annobin/annobin.html/Test-cf-protection.html
@bedevere-app bedevere-app bot mentioned this pull request Mar 5, 2025
Open
@stratakis
Copy link
Contributor Author

The current equivalent eh_frame for the aarch64 assembly code, generated by C code doesn't seem to properly match, although at the same time it works fine. Here is a comparison between x86_64 and aarch64.

x86_64 eh_frame:

00000000 0000000000000014 00000000 CIE
  Version:               1
  Augmentation:          "zR"
  Code alignment factor: 1
  Data alignment factor: -8
  Return address column: 16
  Augmentation data:     1b
  DW_CFA_def_cfa: r7 (rsp) ofs 8
  DW_CFA_offset: r16 (rip) at cfa-8
  DW_CFA_nop
  DW_CFA_nop

00000018 0000000000000014 0000001c FDE cie=00000000 pc=0000000000000000..000000000000000b
  DW_CFA_advance_loc: 4 to 0000000000000004
  DW_CFA_def_cfa_offset: 16
  DW_CFA_advance_loc: 6 to 000000000000000a
  DW_CFA_def_cfa_offset: 8
  DW_CFA_nop

Equivalent to:

cpython/Python/perf_jit_trampoline.c

Lines 474 to 478 in d0ecbdd

#ifdef __x86_64__
DWRF_U8(DWRF_CFA_advance_loc | 4);
DWRF_U8(DWRF_CFA_def_cfa_offset); DWRF_UV(16);
DWRF_U8(DWRF_CFA_advance_loc | 6);
DWRF_U8(DWRF_CFA_def_cfa_offset); DWRF_UV(8);

Whereas the eh_frame for aarch64:

00000000 0000000000000010 00000000 CIE
  Version:               1
  Augmentation:          "zR"
  Code alignment factor: 4
  Data alignment factor: -8
  Return address column: 30
  Augmentation data:     1b
  DW_CFA_def_cfa: r31 (sp) ofs 0

00000014 0000000000000020 00000018 FDE cie=00000000 pc=0000000000000000..0000000000000014
  DW_CFA_advance_loc: 4 to 0000000000000004
  DW_CFA_def_cfa_offset: 16
  DW_CFA_offset: r29 (x29) at cfa-16
  DW_CFA_offset: r30 (x30) at cfa-8
  DW_CFA_advance_loc: 12 to 0000000000000010
  DW_CFA_restore: r30 (x30)
  DW_CFA_restore: r29 (x29)
  DW_CFA_def_cfa_offset: 0
  DW_CFA_nop
  DW_CFA_nop
  DW_CFA_nop
  DW_CFA_nop
  DW_CFA_nop
  DW_CFA_nop
  DW_CFA_nop

Equivalent to:

cpython/Python/perf_jit_trampoline.c

Lines 480 to 489 in d0ecbdd

#elif defined(__aarch64__) && defined(__AARCH64EL__) && !defined(__ILP32__)
DWRF_U8(DWRF_CFA_advance_loc | 1);
DWRF_U8(DWRF_CFA_def_cfa_offset); DWRF_UV(16);
DWRF_U8(DWRF_CFA_offset | 29); DWRF_UV(2);
DWRF_U8(DWRF_CFA_offset | 30); DWRF_UV(1);
DWRF_U8(DWRF_CFA_advance_loc | 3);
DWRF_U8(DWRF_CFA_offset | -(64 - 29));
DWRF_U8(DWRF_CFA_offset | -(64 - 30));
DWRF_U8(DWRF_CFA_def_cfa_offset);
DWRF_UV(0);

@stratakis stratakis force-pushed the aarch64_branch_protection branch from a6fe267 to fe6fb29 Compare March 12, 2025 14:25
@stratakis
Copy link
Contributor Author

Ah wasn't taking into account the code factor alignment for aarch64. Fixed that.

@stratakis stratakis force-pushed the aarch64_branch_protection branch from fe6fb29 to 8772759 Compare March 17, 2025 13:04
@stratakis
Copy link
Contributor Author

eh_frame for the new assembly which should be implemented in C:

0000000 0000000000000010 0000000 CIE
Version: 1
Augmentation: "zR"
Code alignment factor: 4
Data alignment factor: -8
Return address column: 30
Augmentation data: 1b
DW_CFA_def_cfa: r31 (sp) ofs 0

00000014 0000000000000020 00000018 FDE cie=00000000 pc=0000000000000000..000000000000001c
DW_CFA_advance_loc: 4 to 0000000000000004
DW_CFA_AARCH64_negate_ra_state
DW_CFA_advance_loc: 4 to 0000000000000008
DW_CFA_def_cfa_offset: 16
DW_CFA_offset: r29 (x29) at cfa-16
DW_CFA_offset: r30 (x30) at cfa-8
DW_CFA_advance_loc: 12 to 0000000000000014
DW_CFA_restore: r30 (x30)
DW_CFA_restore: r29 (x29)
DW_CFA_def_cfa_offset: 0
DW_CFA_advance_loc: 4 to 0000000000000018
DW_CFA_AARCH64_negate_ra_state
DW_CFA_nop
DW_CFA_nop
DW_CFA_nop

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brandtbucher brandtbucher Awaiting requested review from brandtbucher brandtbucher is a code owner

@savannahostrowski savannahostrowski Awaiting requested review from savannahostrowski savannahostrowski is a code owner

Assignees
No one assigned
Labels
awaiting review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@stratakis