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

c18n: Autumn 2023 release #1653

Merged
merged 8 commits into from
Nov 1, 2023
Merged

c18n: Autumn 2023 release #1653

merged 8 commits into from
Nov 1, 2023

Conversation

dpgao
Copy link
Contributor

@dpgao dpgao commented Feb 14, 2023

This PR contains changes pertaining to compartmentalisation that should be included in the autumn 2023 release.

Key changes:

  • Support the transition ABI for passing memory arguments.
  • Trampolines now clear unused argument registers based on function signature information embedded in the ELF.
  • Compartment transition tracing and overhead simulation.
  • Allow defining compartments that consist of multiple libraries.
  • Support for the benchmark ABI.

Note: Support for the benchmark ABI has not been merged yet.

Copy link
Member

@brooksdavis brooksdavis left a comment

Choose a reason for hiding this comment

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

I'm not sure how best to integrate the libunwind changes. Perhaps @jrtc27, @arichardson, or @bsdjhb have opinions.

lib/libthr/thread/thr_sig.c Outdated Show resolved Hide resolved
lib/libgcc_s/Makefile Outdated Show resolved Hide resolved
lib/libgcc_s/Makefile Outdated Show resolved Hide resolved
@dpgao dpgao changed the title c18n: Support for C++ exceptions c18n: summer 2023 release Jun 18, 2023
@dpgao dpgao force-pushed the c18n branch 2 times, most recently from d1dcb82 to 18d13f8 Compare June 27, 2023 12:15
@dpgao dpgao mentioned this pull request Jun 27, 2023
@dpgao dpgao force-pushed the c18n branch 2 times, most recently from 40c238f to a24c2bf Compare June 27, 2023 19:30
@dpgao dpgao requested review from brooksdavis and jrtc27 June 27, 2023 19:39
@dpgao dpgao force-pushed the c18n branch 14 times, most recently from e81efa3 to 25e6fa9 Compare July 4, 2023 11:27
Copy link
Member

@jrtc27 jrtc27 left a comment

Choose a reason for hiding this comment

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

Mostly fine, but there are a few later commits that fix things up in earlier commits that need squashing into the earlier commits or, in some cases, may need to be reordered (i.e. ideally the "don't use c30 to determine restricted" commit would come before the one that adds the rtld_die hack so that never gets introduced in the history)

libexec/rtld-elf/aarch64/rtld_start.S Outdated Show resolved Hide resolved
@@ -322,7 +326,11 @@ exit_thread(void)
* Kernel will do wakeup at the address, so joiner thread
* will be resumed if it is sleeping at the address.
*/
#if defined(__CHERI_PURE_CAPABILITY__) && defined(RTLD_SANDBOX)
_rtld_thr_exit(&curthread->tid);
Copy link
Member

Choose a reason for hiding this comment

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

This PR does things the old way in the initial commit and then later reworks things; please squash that rather than introducing code you later remove

table_index_to_compart_id(unsigned index)
{
struct tramp_stk_table dummy;
return (sizeof(dummy.stacks->bottom) * index / sizeof(*dummy.stacks));
Copy link
Member

Choose a reason for hiding this comment

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

Newline before

Copy link
Member

Choose a reason for hiding this comment

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

Not resolved in the commit this was a comment on. Resolving it in a later commit leaves incorrectly-styled code in the history.

libexec/rtld-elf/aarch64/rtld_c18n_machdep.c Outdated Show resolved Hide resolved
libexec/rtld-elf/rtld.c Outdated Show resolved Hide resolved
libexec/rtld-elf/rtld.c Outdated Show resolved Hide resolved
libexec/rtld-elf/rtld.c Outdated Show resolved Hide resolved
Comment on lines 361 to 527
size_t size = atomic_load_explicit(&pg->size, memory_order_relaxed);
do {
Copy link
Member

Choose a reason for hiding this comment

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

Still present in the first big commit

lib/libc/aarch64/gen/_setjmp.S Show resolved Hide resolved
libexec/rtld-elf64cb-c18n/Makefile Outdated Show resolved Hide resolved
@dpgao dpgao force-pushed the c18n branch 11 times, most recently from d63f0f1 to b2d5fa6 Compare October 25, 2023 16:10
Copy link
Member

@jrtc27 jrtc27 left a comment

Choose a reason for hiding this comment

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

I cannot review the benchmark ABI commit. It's hundreds of lines of churn completely reworking implementations beyond just adding some #ifdefs. That is not how to do things, either fold the reworking into earlier commits so there's less "do A, wait no do B, wait no do C instead" or, if it's instead reworking code that this PR hasn't touched before that point then a separate commit is fine, but not as part of the benchmark ABI commit. That has to be "add benchmark ABI without messing with existing purecap implementation beyond adding some ifs/#ifdefs", not shuffling things around, rewriting assembly sequences, adding entirely new sets of functions used in both cases, etc.

table_index_to_compart_id(unsigned index)
{
struct tramp_stk_table dummy;
return (sizeof(dummy.stacks->bottom) * index / sizeof(*dummy.stacks));
Copy link
Member

Choose a reason for hiding this comment

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

Not resolved in the commit this was a comment on. Resolving it in a later commit leaves incorrectly-styled code in the history.

libexec/rtld-elf/aarch64/rtld_c18n_machdep.c Outdated Show resolved Hide resolved
libexec/rtld-elf/rtld_c18n.c Show resolved Hide resolved
libexec/rtld-elf/rtld_c18n.c Outdated Show resolved Hide resolved
libexec/rtld-elf/rtld_c18n.c Outdated Show resolved Hide resolved
libexec/rtld-elf/aarch64/reloc.c Show resolved Hide resolved
libexec/rtld-elf/aarch64/rtld_c18n_asm.S Outdated Show resolved Hide resolved
libexec/rtld-elf/aarch64/rtld_c18n_machdep.c Outdated Show resolved Hide resolved
libexec/rtld-elf/aarch64/rtld_start.S Outdated Show resolved Hide resolved
bin/cheribsdtest/Makefile Outdated Show resolved Hide resolved
@dpgao dpgao force-pushed the c18n branch 3 times, most recently from 9ee9fbc to c207bc4 Compare October 31, 2023 15:48
Copy link
Member

@jrtc27 jrtc27 left a comment

Choose a reason for hiding this comment

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

Let's get this in at long last once CI is happy

Stack unwinding does not work under c18n so we explicitly disable it.
AT_PHDR is not executable and it is not necessary to clear its Executive
permission bit. See freebsd_copyout_auxargs in sys/kern/imgact_elf.c.
(1) Support new ABI for passing memory arguments

This commit supports the new purecap ABI for passing memory arguments
(enabled by -morello-bounded-memargs=caller-only). During a compartment
transition, the trampolines copy memory arguments (if any) to the
callee's stack.

Trampolines now also save and clear callee-saved registers in the
trusted stack. The implementation of setjmp and longjmp are updated to
support the resulting enlarged trusted frames.

The trampoline generation infrastructure has been improve to allow
better extensibility.

(2) Clear unused registers during compartment transition

Implements unused argument/return value register clearing for dynamic
functions resolved by name (i.e., not function pointers). This
implementation relies on a custom ELF section recording the signature of
each dynamic function.

(3) Allow multiple libraries in one compartment

Introduce a policy mechanism that allows (a) multiple libraries to be in
the same compartment so that no trampolines are inserted for function
calls between them, and (b) certain 'trusted' symbols to be resolved
directly without being wrapped by a trampoline.

Feature (b) is used to mark a number of libc str* and mem* functions as
trusted, significantly reducing the number of domain transitions for
typical applications. setjmp/longjmp are also mark as trusted, which
results in significantly simpler implementations to make them work under
c18n.
Put atomic variables on different cache lines and put the first two
capability-sized words of a trampoline on the same cache line.
@dpgao dpgao merged commit 291af57 into dev Nov 1, 2023
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet