c18n: New {get,set}context APIs for libunwind #2123

dpgao · 2024-06-20T10:41:32Z

See also #2122 for a draft of this PR.

_rtld_unw_getcontext_epilogue is removed because when c18n is not enabled, the trusted stack field of the context is unused anyway, so there's no need to fill it.

The assembly stub in _rtld_unw_setcontext is removed because _rtld_unw_setcontext is now expected to be called before all registers are restored, so it does not need to restore any register anymore.

dstolfa · 2024-06-20T14:26:10Z

This makes changes in setjmp/longjmp as well and the commit message should probably reflect that.

bsdjhb

Certainly some text in the commit log explaining why setjmp/longjmp are changing would be good.

bsdjhb · 2024-07-01T18:49:22Z

lib/libc/aarch64/gen/_setjmp.S

-#if defined(__CHERI_PURE_CAPABILITY__) && defined(RTLD_SANDBOX)
-	mov	c2, c8
-#else
+#if !defined(__CHERI_PURE_CAPABILITY__) || !defined(RTLD_SANDBOX)


I find it easier to read as !(purecap && sandbox) rather than !purecap || !sandbox.

Also, is RTLD_SANDBOX ever true for !purecap? That is, can we simplify some of the #ifdef's to #ifdef RTLD_SANDBOX or #ifndef RTLD_SANDBOX?

Also, is RTLD_SANDBOX ever true for !purecap? That is, can we simplify some of the #ifdef's to #ifdef RTLD_SANDBOX or #ifndef RTLD_SANDBOX?

The purecap check is indeed redundant. I'll do a find-and-replace once the libsys is merged. (libsys renames RTLD_SANDBOX to CHERI_LIB_C18N.)

@bsdjhb I've removed the checks for __CHERI_PURE_CAPABILITY__ in #2200.

Security fixes: CTSRD-CHERI#2135 rar: Fix OOB in rar e8 filter (CVE-2024-26256) CTSRD-CHERI#2145 zip: Fix out of boundary access Important bugfixes: CTSRD-CHERI#2131 7zip: Limit amount of properties CTSRD-CHERI#2110 bsdtar: Fix error handling around strtol() usages CTSRD-CHERI#2116 passphrase: Never allow empty passwords CTSRD-CHERI#2124 rar: Fix "File CRC Error" when extracting specific rar4 archives CTSRD-CHERI#2123 xar: Avoid infinite link loop CTSRD-CHERI#2108 zip: Update AppleDouble support for directories CTSRD-CHERI#2071 zstd: Implement core detection Obained from: libarchive Libarchive commit: 313aa1fa10b657de791e3202c168a6c833bc3543

Libarchive 3.7.4 + three fixes from master Security fixes: CTSRD-CHERI#2135 rar: Fix OOB in rar e8 filter (CVE-2024-26256) CTSRD-CHERI#2145 zip: Fix out of boundary access CTSRD-CHERI#2148 rar: Fix OOB in rar delta filter CTSRD-CHERI#2149 rar: Fix OOB in rar audio filter Important bugfixes: CTSRD-CHERI#2131 7zip: Limit amount of properties CTSRD-CHERI#2110 bsdtar: Fix error handling around strtol() usages CTSRD-CHERI#2116 passphrase: Never allow empty passwords CTSRD-CHERI#2124 rar: Fix "File CRC Error" when extracting specific rar4 archives CTSRD-CHERI#2123 xar: Avoid infinite link loop CTSRD-CHERI#2150 xar: Fix another infinite loop and expat error handling CTSRD-CHERI#2108 zip: Update AppleDouble support for directories CTSRD-CHERI#2071 zstd: Implement core detectiongit PR: 278588 (exp-run) MFC after: 1 day

dstolfa

Builds & runs fine, similar to CTSRD-CHERI/llvm-project#740 just a few nits.

libexec/rtld-elf/rtld_c18n.h

jrtc27 · 2024-08-27T20:52:19Z

libexec/rtld-elf/rtld_c18n.c


-static void *
-unwind_cursor(void)
+bool c18n_is_enabled(void);


These should probably be in sys/sys/link_elf.h with dl_iterate_phdr and friends.

Yes. Although I guess it would require renaming struct trusted_frame etc. to have a dl_ prefix?

Hmmm, since a struct trusted_frame * only ever leaves RTLD as a sealed capability, a void * should be sufficient.

libexec/rtld-elf/rtld_c18n.c

lib/libc/aarch64/gen/_setjmp.S

jrtc27 · 2024-08-27T21:36:21Z

libexec/rtld-elf/rtld_c18n.h

@@ -217,8 +259,8 @@ func_sig_legal(struct func_sig sig)
 /*
 * This macro can only be used in a function directly invoked by a trampoline.
 */
-#define	c18n_return_address()						\
-	(C18N_ENABLED ? get_trusted_stk()->pc : __builtin_return_address(0))
+#define	c18n_return_address()	(C18N_ENABLED ?				\


There should probably be a non-c18n fallback implementation of this so you don't need to ifdef every use.

Will do in another PR

jrtc27 · 2024-08-27T21:47:04Z

libexec/rtld-elf/rtld_c18n.c

@@ -936,13 +913,17 @@ unwind_stack(struct jmp_args ret, void *rcsp, struct trusted_frame *target)
 	struct trusted_frame *cur, *tf;
 	sigset_t nset, oset;

+	if (!C18N_ENABLED)
+		return (ret);
+
 	/*
 	 * Make the function re-entrant by blocking all signals.
 	 */
 	SIGFILLSET(nset);
 	sigprocmask(SIG_SETMASK, &nset, &oset);


Could expose functions from rtld_lock.c that use sigfastblock when available, saves having to do syscalls here for single-threaded applications. In theory one could also register a per-thread mask just for c18n to use, given rtld unregisters its locking one when a thread is created. But not for this PR.

dpgao · 2024-08-28T17:08:17Z

@jrtc27 I've changed how setjmp/longjmp invokes the RTLD's unwinding APIs such that it is very simple to remove the ifdefs in a future PR (essentially we just need a noop ASM stub like in libunwind). I also added dl_ prefixes to the APIs and exported them in link_elf.h.

dpgao · 2024-08-29T20:33:46Z

The dl_* functions now use the normal ABI and CTSRD-CHERI/llvm-project#740 has been updated accordingly.

jrtc27 · 2024-08-29T20:38:30Z

Can we keep the old interfaces implemented as compatibility shims so upgrades work when c18n is enabled?

dpgao · 2024-08-30T14:17:24Z

Can we keep the old interfaces implemented as compatibility shims so upgrades work when c18n is enabled?

It seems that preserving the old interfaces implies retaining much of the old implementation as well (including the assembly stubs), but that's fine.

sys/sys/link_elf.h

jrtc27 · 2024-10-18T17:38:37Z

libexec/rtld-elf/rtld_c18n.c

 }

+/*
+ * XXX: These functions are kept here for compatibility with old libunwind.


libc and libunwind (ditto for the other instance below)

And maybe group all the compat interfaces together at the end?

It is possible to move all the function definitions to the end, but the declaration of _rtld_unw_setcontext_impl has to stay in front because of the function-pointer workaround employed in the previous release. _rtld_unw_setcontext_impl has to be wrapped into _rtld_unw_setcontext_ptr in c18n_init2.

libexec/rtld-elf/rtld_c18n.c

lib/libgcc_s/Makefile

lib/libc/gen/dlfcn.c

sys/sys/link_elf.h

jrtc27 · 2024-10-28T18:45:18Z

Be very careful about git subrepo commits in a PR, the parent commit is recorded so rebasing breaks them for next time someone uses git subrepo.

bsdjhb · 2024-10-28T18:51:22Z

I'm in the middle of another FreeBSD merge, but I can do an update of libunwind after that so long as the new libunwind will still work fine with the current dev

jrtc27 · 2024-10-28T18:54:26Z

Yeah, it would be easiest if we decoupled rtld and libunwind changes. Because this PR keeps the old rtld interfaces (and they’ll stay for one release) we don’t need to import libunwind in this PR, we can just merge the new interfaces ready for a future import.

jrtc27 · 2024-10-28T18:55:02Z

(But note that updating libunwind in dev now would break)

dpgao · 2024-10-28T19:46:16Z

Be very careful about git subrepo commits in a PR, the parent commit is recorded so rebasing breaks them for next time someone uses git subrepo.

I see. I've removed the subrepo pull commit.

Using relational operators to compare pointers to different trusted frames is undefined behaviour.

Previously, libunwind hard-codes knowledge about the layout of the trusted frame and has read access to the trusted stack. This is fragile and insecure. Now, the task of extracting the relevant registers from the trusted stack is delegated to RTLD, and libunwind no longer has access to the trusted stack. The unified unwinding APIs are declared as dl_c18n_* functions in link.h. setjmp/longjmp have been updated to use them. The previous API has been retained for compatibility with old libunwind.

dpgao mentioned this pull request Jun 20, 2024

[RFC] c18n: New {get,set}context APIs for libunwind #2122

Closed

bsdjhb reviewed Jul 1, 2024

View reviewed changes

dpgao force-pushed the c18n-unw-real branch from 45c9be3 to ff2600d Compare July 18, 2024 14:41

dpgao force-pushed the c18n-unw-real branch from ff2600d to 3b76a6e Compare August 22, 2024 19:27

dpgao requested review from jrtc27 and dstolfa August 22, 2024 19:46

dstolfa reviewed Aug 24, 2024

View reviewed changes

libexec/rtld-elf/rtld_c18n.h Outdated Show resolved Hide resolved

libexec/rtld-elf/rtld_c18n.h Show resolved Hide resolved

jrtc27 reviewed Aug 27, 2024

View reviewed changes

dpgao force-pushed the c18n-unw-real branch from 3b76a6e to 530305d Compare August 28, 2024 16:58

dpgao requested review from jrtc27 and dstolfa August 28, 2024 17:10

dpgao force-pushed the c18n-unw-real branch from 530305d to 7a0f720 Compare August 29, 2024 20:28

dpgao force-pushed the c18n-unw-real branch from 7a0f720 to f0c2c5d Compare August 30, 2024 10:57

jrtc27 reviewed Oct 18, 2024

View reviewed changes

sys/sys/link_elf.h Outdated Show resolved Hide resolved

jrtc27 reviewed Oct 18, 2024

View reviewed changes

libexec/rtld-elf/rtld_c18n.c Outdated Show resolved Hide resolved

dpgao force-pushed the c18n-unw-real branch 2 times, most recently from b36ee3e to 7d7f130 Compare October 18, 2024 21:04

jrtc27 reviewed Oct 18, 2024

View reviewed changes

lib/libgcc_s/Makefile Show resolved Hide resolved

lib/libc/gen/dlfcn.c Show resolved Hide resolved

dpgao force-pushed the c18n-unw-real branch from 7d7f130 to a803555 Compare October 19, 2024 14:44

dpgao mentioned this pull request Oct 19, 2024

libunwind: Use new unwinding APIs under c18n CTSRD-CHERI/llvm-project#740

Merged

jrtc27 reviewed Oct 21, 2024

View reviewed changes

sys/sys/link_elf.h Outdated Show resolved Hide resolved

dpgao force-pushed the c18n-unw-real branch from a803555 to 9f7f84a Compare October 22, 2024 10:46

dpgao requested a review from jrtc27 October 23, 2024 16:49

dpgao force-pushed the c18n-unw-real branch 3 times, most recently from 41d8ed3 to 7574483 Compare October 28, 2024 17:22

dpgao force-pushed the c18n-unw-real branch from 7574483 to 05401a2 Compare October 28, 2024 19:45

dpgao added 2 commits October 30, 2024 16:46

c18n: Compare pointers by converting them to integers first

5a00523

Using relational operators to compare pointers to different trusted frames is undefined behaviour.

dpgao force-pushed the c18n-unw-real branch from 05401a2 to 08300c1 Compare October 30, 2024 17:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

c18n: New {get,set}context APIs for libunwind #2123

c18n: New {get,set}context APIs for libunwind #2123

dpgao commented Jun 20, 2024

dstolfa commented Jun 20, 2024

bsdjhb left a comment

bsdjhb Jul 1, 2024

bsdjhb Jul 1, 2024 •

edited by jrtc27

Loading

dpgao Jul 19, 2024

dpgao Aug 22, 2024

dstolfa left a comment

jrtc27 Aug 27, 2024

dpgao Aug 28, 2024

dpgao Aug 28, 2024 •

edited

Loading

jrtc27 Aug 27, 2024

dpgao Aug 28, 2024

jrtc27 Aug 27, 2024

dpgao commented Aug 28, 2024

dpgao commented Aug 29, 2024

jrtc27 commented Aug 29, 2024

dpgao commented Aug 30, 2024

jrtc27 Oct 18, 2024

jrtc27 Oct 18, 2024

dpgao Oct 19, 2024

jrtc27 commented Oct 28, 2024 •

edited

Loading

bsdjhb commented Oct 28, 2024

jrtc27 commented Oct 28, 2024

jrtc27 commented Oct 28, 2024

dpgao commented Oct 28, 2024

c18n: New {get,set}context APIs for libunwind #2123

Are you sure you want to change the base?

c18n: New {get,set}context APIs for libunwind #2123

Conversation

dpgao commented Jun 20, 2024

dstolfa commented Jun 20, 2024

bsdjhb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bsdjhb Jul 1, 2024 • edited by jrtc27 Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dstolfa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dpgao Aug 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dpgao commented Aug 28, 2024

dpgao commented Aug 29, 2024

jrtc27 commented Aug 29, 2024

dpgao commented Aug 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jrtc27 commented Oct 28, 2024 • edited Loading

bsdjhb commented Oct 28, 2024

jrtc27 commented Oct 28, 2024

jrtc27 commented Oct 28, 2024

dpgao commented Oct 28, 2024

bsdjhb Jul 1, 2024 •

edited by jrtc27

Loading

dpgao Aug 28, 2024 •

edited

Loading

jrtc27 commented Oct 28, 2024 •

edited

Loading