[libunwind][NFC] Remove the CET keyword in shadow stack-related stuffs #126663
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
libunwind currently supports shadow stack based on the Intel CET and AArch64 GCS technology, but throughout related codes, the Intel-specific keyword, "CET", is used to refer to the generic concept of control-flow integrity/shadow stack. This patch replaces such wordings with architecture-neutral term "shadow stack" (abbr. "ss") to allow future implementation to avoid using the Intel-specific "CET" term.
|
@llvm/pr-subscribers-libunwind Author: Ming-Yi Lai (mylai-mtk) Changeslibunwind currently supports shadow stack based on the Intel CET and AArch64 GCS technology, but throughout related codes, the Intel-specific keyword, "CET", is used to refer to the generic concept of control-flow integrity/shadow stack. This patch replaces such wordings with architecture-neutral term "shadow stack" (abbr. "ss") to allow future implementation to avoid using the Intel-specific "CET" term. PS: I'm implementing RISC-V Zicfiss support for libunwind, but found the everlasting CET out of place, so I think it's better to send out this renaming patch first Full diff: https://github.com/llvm/llvm-project/pull/126663.diff 5 Files Affected:
diff --git a/libunwind/src/CMakeLists.txt b/libunwind/src/CMakeLists.txt
index ecbd019bb29ea8f..3bbbc70fde79b74 100644
--- a/libunwind/src/CMakeLists.txt
+++ b/libunwind/src/CMakeLists.txt
@@ -36,7 +36,7 @@ set(LIBUNWIND_HEADERS
AddressSpace.hpp
assembly.h
CompactUnwinder.hpp
- cet_unwind.h
+ shadow_stack_unwind.h
config.h
dwarf2.h
DwarfInstructions.hpp
diff --git a/libunwind/src/Registers.hpp b/libunwind/src/Registers.hpp
index 861e6b5f6f2c583..df79f0439ae85c1 100644
--- a/libunwind/src/Registers.hpp
+++ b/libunwind/src/Registers.hpp
@@ -15,7 +15,7 @@
#include <stdint.h>
#include <string.h>
-#include "cet_unwind.h"
+#include "shadow_stack_unwind.h"
#include "config.h"
#include "libunwind.h"
@@ -48,7 +48,7 @@ class _LIBUNWIND_HIDDEN Registers_x86;
extern "C" void __libunwind_Registers_x86_jumpto(Registers_x86 *);
#if defined(_LIBUNWIND_USE_CET)
-extern "C" void *__libunwind_cet_get_jump_target() {
+extern "C" void *__libunwind_ss_get_jump_target() {
return reinterpret_cast<void *>(&__libunwind_Registers_x86_jumpto);
}
#endif
@@ -268,7 +268,7 @@ class _LIBUNWIND_HIDDEN Registers_x86_64;
extern "C" void __libunwind_Registers_x86_64_jumpto(Registers_x86_64 *);
#if defined(_LIBUNWIND_USE_CET)
-extern "C" void *__libunwind_cet_get_jump_target() {
+extern "C" void *__libunwind_ss_get_jump_target() {
return reinterpret_cast<void *>(&__libunwind_Registers_x86_64_jumpto);
}
#endif
@@ -1817,7 +1817,7 @@ class _LIBUNWIND_HIDDEN Registers_arm64;
extern "C" void __libunwind_Registers_arm64_jumpto(Registers_arm64 *);
#if defined(_LIBUNWIND_USE_GCS)
-extern "C" void *__libunwind_cet_get_jump_target() {
+extern "C" void *__libunwind_ss_get_jump_target() {
return reinterpret_cast<void *>(&__libunwind_Registers_arm64_jumpto);
}
#endif
diff --git a/libunwind/src/UnwindCursor.hpp b/libunwind/src/UnwindCursor.hpp
index 0923052b1b588cf..5cb04b1f76820f2 100644
--- a/libunwind/src/UnwindCursor.hpp
+++ b/libunwind/src/UnwindCursor.hpp
@@ -11,7 +11,7 @@
#ifndef __UNWINDCURSOR_HPP__
#define __UNWINDCURSOR_HPP__
-#include "cet_unwind.h"
+#include "shadow_stack_unwind.h"
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
@@ -3122,7 +3122,7 @@ bool UnwindCursor<A, R>::isReadableAddr(const pint_t addr) const {
#endif
#if defined(_LIBUNWIND_USE_CET) || defined(_LIBUNWIND_USE_GCS)
-extern "C" void *__libunwind_cet_get_registers(unw_cursor_t *cursor) {
+extern "C" void *__libunwind_ss_get_registers(unw_cursor_t *cursor) {
AbstractUnwindCursor *co = (AbstractUnwindCursor *)cursor;
return co->get_registers();
}
diff --git a/libunwind/src/UnwindLevel1.c b/libunwind/src/UnwindLevel1.c
index 7e785f4d31e716a..2d471d5d65690ce 100644
--- a/libunwind/src/UnwindLevel1.c
+++ b/libunwind/src/UnwindLevel1.c
@@ -25,7 +25,7 @@
#include <stdio.h>
#include <string.h>
-#include "cet_unwind.h"
+#include "shadow_stack_unwind.h"
#include "config.h"
#include "libunwind.h"
#include "libunwind_ext.h"
@@ -36,14 +36,17 @@
#ifndef _LIBUNWIND_SUPPORT_SEH_UNWIND
-// When CET is enabled, each "call" instruction will push return address to
-// CET shadow stack, each "ret" instruction will pop current CET shadow stack
-// top and compare it with target address which program will return.
-// In exception handing, some stack frames will be skipped before jumping to
-// landing pad and we must adjust CET shadow stack accordingly.
-// _LIBUNWIND_POP_CET_SSP is used to adjust CET shadow stack pointer and we
-// directly jump to __libunwind_Registers_x86/x86_64_jumpto instead of using
-// a regular function call to avoid pushing to CET shadow stack again.
+// When shadow stack is enabled, a separate stack containing only return
+// addresses would be maintained. On function return, the return address would
+// be compared to the popped address from shadow stack to ensure the return
+// target is not tempered with. When unwinding, we're skipping the normal return
+// procedure for multiple frames and thus need to pop the return addresses of
+// the skipped frames from shadow stack to avoid triggering an exception (using
+// `_LIBUNWIND_POP_SS_SSP()`). Also, some architectures, like the x86-family
+// CET, push the return adddresses onto shadow stack with common call
+// instructions, so for these architectures, normal function calls should be
+// avoided when invoking the `jumpto()` function. To do this, we use inline
+// assemblies to "goto" the `jumpto()` for these architectures.
#if !defined(_LIBUNWIND_USE_CET) && !defined(_LIBUNWIND_USE_GCS)
#define __unw_phase2_resume(cursor, fn) \
do { \
@@ -51,38 +54,38 @@
__unw_resume((cursor)); \
} while (0)
#elif defined(_LIBUNWIND_TARGET_I386)
-#define __cet_ss_step_size 4
+#define __shadow_stack_step_size (4)
#define __unw_phase2_resume(cursor, fn) \
do { \
- _LIBUNWIND_POP_CET_SSP((fn)); \
- void *cetRegContext = __libunwind_cet_get_registers((cursor)); \
- void *cetJumpAddress = __libunwind_cet_get_jump_target(); \
+ _LIBUNWIND_POP_SS_SSP((fn)); \
+ void *ssRegContext = __libunwind_ss_get_registers((cursor)); \
+ void *ssJumpAddress = __libunwind_ss_get_jump_target(); \
__asm__ volatile("push %%edi\n\t" \
"sub $4, %%esp\n\t" \
- "jmp *%%edx\n\t" :: "D"(cetRegContext), \
- "d"(cetJumpAddress)); \
+ "jmp *%%edx\n\t" :: "D"(ssRegContext), \
+ "d"(ssJumpAddress)); \
} while (0)
#elif defined(_LIBUNWIND_TARGET_X86_64)
-#define __cet_ss_step_size 8
+#define __shadow_stack_step_size (8)
#define __unw_phase2_resume(cursor, fn) \
do { \
- _LIBUNWIND_POP_CET_SSP((fn)); \
- void *cetRegContext = __libunwind_cet_get_registers((cursor)); \
- void *cetJumpAddress = __libunwind_cet_get_jump_target(); \
- __asm__ volatile("jmpq *%%rdx\n\t" :: "D"(cetRegContext), \
- "d"(cetJumpAddress)); \
+ _LIBUNWIND_POP_SS_SSP((fn)); \
+ void *ssRegContext = __libunwind_ss_get_registers((cursor)); \
+ void *ssJumpAddress = __libunwind_ss_get_jump_target(); \
+ __asm__ volatile("jmpq *%%rdx\n\t" :: "D"(ssRegContext), \
+ "d"(ssJumpAddress)); \
} while (0)
#elif defined(_LIBUNWIND_TARGET_AARCH64)
-#define __cet_ss_step_size 8
+#define __shadow_stack_step_size (8)
#define __unw_phase2_resume(cursor, fn) \
do { \
- _LIBUNWIND_POP_CET_SSP((fn)); \
- void *cetRegContext = __libunwind_cet_get_registers((cursor)); \
- void *cetJumpAddress = __libunwind_cet_get_jump_target(); \
+ _LIBUNWIND_POP_SS_SSP((fn)); \
+ void *ssRegContext = __libunwind_ss_get_registers((cursor)); \
+ void *ssJumpAddress = __libunwind_ss_get_jump_target(); \
__asm__ volatile("mov x0, %0\n\t" \
"br %1\n\t" \
: \
- : "r"(cetRegContext), "r"(cetJumpAddress) \
+ : "r"(ssRegContext), "r"(ssJumpAddress) \
: "x0"); \
} while (0)
#endif
@@ -255,16 +258,16 @@ unwind_phase2(unw_context_t *uc, unw_cursor_t *cursor, _Unwind_Exception *except
}
#endif
-// In CET enabled environment, we check return address stored in normal stack
-// against return address stored in CET shadow stack, if the 2 addresses don't
+// In shadow stack enabled environment, we check return address stored in normal
+// stack against return address stored in shadow stack, if the 2 addresses don't
// match, it means return address in normal stack has been corrupted, we return
// _URC_FATAL_PHASE2_ERROR.
#if defined(_LIBUNWIND_USE_CET) || defined(_LIBUNWIND_USE_GCS)
if (shadowStackTop != 0) {
unw_word_t retInNormalStack;
__unw_get_reg(cursor, UNW_REG_IP, &retInNormalStack);
- unsigned long retInShadowStack = *(
- unsigned long *)(shadowStackTop + __cet_ss_step_size * framesWalked);
+ unsigned long retInShadowStack = *(unsigned long *)
+ (shadowStackTop + __shadow_stack_step_size * framesWalked);
if (retInNormalStack != retInShadowStack)
return _URC_FATAL_PHASE2_ERROR;
}
diff --git a/libunwind/src/cet_unwind.h b/libunwind/src/shadow_stack_unwind.h
similarity index 88%
rename from libunwind/src/cet_unwind.h
rename to libunwind/src/shadow_stack_unwind.h
index 47d7616a7322c32..8588ea01accd922 100644
--- a/libunwind/src/cet_unwind.h
+++ b/libunwind/src/shadow_stack_unwind.h
@@ -7,8 +7,8 @@
//
//===----------------------------------------------------------------------===//
-#ifndef LIBUNWIND_CET_UNWIND_H
-#define LIBUNWIND_CET_UNWIND_H
+#ifndef LIBUNWIND_SHADOW_STACK_UNWIND_H
+#define LIBUNWIND_SHADOW_STACK_UNWIND_H
#include "libunwind.h"
@@ -21,7 +21,7 @@
#include <cet.h>
#include <immintrin.h>
-#define _LIBUNWIND_POP_CET_SSP(x) \
+#define _LIBUNWIND_POP_SS_SSP(x) \
do { \
unsigned long ssp = _get_ssp(); \
if (ssp != 0) { \
@@ -46,7 +46,7 @@
#define _LIBUNWIND_USE_GCS 1
#endif
-#define _LIBUNWIND_POP_CET_SSP(x) \
+#define _LIBUNWIND_POP_SS_SSP(x) \
do { \
if (__chkfeat(_CHKFEAT_GCS)) { \
unsigned tmp = (x); \
@@ -57,7 +57,7 @@
#endif
-extern void *__libunwind_cet_get_registers(unw_cursor_t *);
-extern void *__libunwind_cet_get_jump_target(void);
+extern void *__libunwind_ss_get_registers(unw_cursor_t *);
+extern void *__libunwind_ss_get_jump_target(void);
#endif
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
Normally we should not remove symbols for backward compatibility, but in this niche case I am not sure anyone uses these CET function names, so I think the rename is fine. Instead of |
|
If there are concerns about backward compatibility, I can limit the renaming to non-symbols only, and add RISC-V Zicfiss-related symbols using its own appropriate names. This stops the spread of CET-based names, but it would look a bit inconsistent across Intel CET, AArch64 GCS and RISC-V Zicfiss even though the code to support these extensions basically do the same thing. Note that in this case, the codes for AArch64 GCS would look the weirdest, since they already adopted the inappropriate name of CET for some symbols. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
libunwind currently supports shadow stack based on the Intel CET and AArch64 GCS technology, but throughout related codes, the Intel-specific keyword, "CET", is used to refer to the generic concept of control-flow integrity/shadow stack. This patch replaces such wordings with architecture-neutral term "shadow stack" (abbr. "ss") to allow future implementation to avoid using the Intel-specific "CET" term.
PS: I'm implementing RISC-V Zicfiss support for libunwind, but found the everlasting CET out of place, so I think it's better to send out this renaming patch first