Skip to content
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.

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

Add initial support for ARM MTE #311

Merged
merged 31 commits into from
Mar 26, 2024
Merged

Add initial support for ARM MTE #311

merged 31 commits into from
Mar 26, 2024

Conversation

ayrtonm
Copy link
Contributor

@ayrtonm ayrtonm commented Dec 12, 2023

This PR adds IA2_USE_MPK and IA2_USE_MTE macros to libia2 and replaces calls to pkey_mprotect with an arch-independent function for tagging loaded writable ELF segments with a memory key. On ARM this uses addg to increment a pointer through the range of pages passed to mprotect and st2g to tag the memory.

This should already pass tests because the #ifdefs used in CI ignore ARM for now, but I'd like to at least build libia2 for ARM in CI before merging this.

edit: nginx build failure may currently be expected(?)

@ayrtonm ayrtonm requested a review from sim-immunant December 12, 2023 04:35
@ayrtonm
Copy link
Contributor Author

ayrtonm commented Dec 12, 2023

Looks like CI works as expected. Also I think the addgs in insert_tag may be a security issue but I need to read a bit more about what kind of CFI we expect to require and how it'll be implemented.

@ayrtonm
Copy link
Contributor Author

ayrtonm commented Dec 12, 2023

I'm still trying to figure out if the addgs are a security issue but either way I think this is lower priority than setting up CI for arm. As a first step we could just build libia2 for aarch64 on donna. I'm not sure if there's a clean way to build for two archs with one cmake config step so we might just do one config per arch.

@ayrtonm ayrtonm force-pushed the am/mte_runtime_init branch 13 times, most recently from 3c98cd7 to dd016ac Compare December 12, 2023 19:24
@ayrtonm
Copy link
Contributor Author

ayrtonm commented Dec 12, 2023

I got libia2 building in CI with a cross-compiler for aarch64 on x86. I see the preprocessor warnings I added for IA2_USE_MTE so it is building for MTE, but we might want to test a bit more thoroughly even before the ARM call gates are ready. I'm still trying to think if there's a good intermediate test between this and running a compartmentalized program in user-mode qemu though.

@@ -3,6 +3,12 @@ project(libia2)

add_library(libia2 ia2.c threads.c main.c exit.c)
target_compile_options(libia2 PRIVATE "-fPIC")
if(LIBIA2_MTE)
target_compile_definitions(libia2 PUBLIC IA2_USE_MTE=1)
target_compile_options(libia2 PUBLIC "-march=armv8.5-a+memtag" "-ffixed-x18")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should take a closer look at the behavior of -ffixed-x18. In my armtest repo, even when compiling with that flag, X18 still seems to get saved and restored between function calls. System libraries also frequently alter x18, so we'll have to compile the whole shebang from libc up with this flag as well.

libia2/main.c Outdated Show resolved Hide resolved
@ayrtonm ayrtonm mentioned this pull request Jan 5, 2024
28 tasks
@ayrtonm
Copy link
Contributor Author

ayrtonm commented Feb 15, 2024

While working on getting CMake working with the correct compilers for each target I realized I missed some #ifdefs in ia2_compartment_init.inc. Also it's probably better to add individual #warnings at each location where we're currently missing ARM64 code.

@ayrtonm ayrtonm force-pushed the am/mte_runtime_init branch from dd016ac to a62ebdc Compare February 21, 2024 16:38
@ayrtonm
Copy link
Contributor Author

ayrtonm commented Feb 21, 2024

The issue with CMake auto-adding -I/usr/include to tests was caused by LLVM include dirs and fixed by replacing include_directories in rewriter/CMakeLists.txt with the usual target_include_directories. The whole repo doesn't really support cross-compiling correctly because CMake makes it a pain to use more than one toolchain and the "just set CMAKE_C_COMPILER" hack I used for pad-tls won't work for the rewriter, so instead we need to use ExternalProject_Add for host tools.

@ayrtonm ayrtonm force-pushed the am/mte_runtime_init branch from eeeaacc to 433ad5f Compare March 6, 2024 16:45
@ayrtonm ayrtonm force-pushed the am/mte_runtime_init branch from 433ad5f to 48295ac Compare March 6, 2024 19:56
@ayrtonm ayrtonm changed the base branch from main to am/update_docs March 11, 2024 20:15
@ayrtonm ayrtonm force-pushed the am/mte_runtime_init branch from 48295ac to 23bfc97 Compare March 13, 2024 21:11
ayrtonm added 28 commits March 26, 2024 08:28
Note that the rewriter emits arch-specific assembly and this just sets the arch
for the scrub registers header.
CMake has weird rules about what dependencies cause custom commands to be
re-run. See the comment in ia2.cmake for details.
@ayrtonm ayrtonm merged commit 1648dfc into main Mar 26, 2024
25 of 34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants