[compiler-rt][CET] Enable CET for compiler-rt builtin library

[xiongji90]

Summary

Control-flow Enforcement Technology (CET), published by Intel, introduces indirect branch tracking(IBT) feature aiming to ensure the target address of an indirect jump/call is not tampered.
When IBT is enabled, each function or target of any indirect jump/call will start with an 'endbr32/64' instruction otherwise the program will crash during execution.
To build an application with CET enabled. we need to ensure:

1. build the source code with "-fcf-protection=full"
2. all the libraries linked with .o files must be CET enabled too

This patch aims to enable CET for compiler-rt builtins library, we add an option "COMPILER_RT_ENABLE_CET" whose default value is OFF to enable CET for compiler-rt in building time and when this option is "ON", "-fcf-protection=full" is added to BUILTINS_CFLAG and the "endbr32/64" will be placed in the beginning of each assembly function. And we also enable running all builtin tests with CET enabled when COMPILER_RT_ENABLE_CET is on. All builtin tests pass on CET enabled system.

[xiongji90]

Hi, @MaskRay
could you help review this patch?
Thanks very much.

[efriedma]

compiler-rt/lib/builtins/assembly.h

19

Should this be guarded by COMPILER_RT_ENABLE_CET somehow? Or does cet.h do that?

I'm a little concerned about the defined(linux) check, since the CMake lets you enable CET on non-Linux targets.

[xiongji90]

compiler-rt/lib/builtins/assembly.h

19

Hi, @efriedma
You are correct, we need "CET" macro to guard here, I will update the patch.
Currently, CET is implemented on Linux + intel platform(x86_64/x86) and "cet.h" is used on Linux platform to provide some CET related macro defs.
This is why I check "linux" here. I know MS is also working on enabling CET on Windows platform but I have seen any Windows OS version published claiming to support CET. Enabling CET on a platform requires hardware, OS, toolchain, runtime libraries to fully support CET.

Thanks very much.

[xiongji90]

include <cet.h> and define CET endbr macros only when "CET" is defined. When using gcc or clang, "CET" is defined when -fcf-protection option is used.

[xiongji90]

compiler-rt/lib/builtins/assembly.h

17

CET is a x86/x86_64 specific macro, so remove the unnecessary x86/x86_64 macro check.

[xiongji90]

Hi, @compnerd , @MaskRay and @efriedma

Could you help review this patch? Thanks very much!

[MaskRay]

compiler-rt/CMakeLists.txt

60

This can be dropped.

if(NOT COMPILER_RT_HAS_FCF_PROTECTION_FLAG) takes care of it.

[xiongji90]

compiler-rt/CMakeLists.txt

248

Hi, @MaskRay
I moved the "-fcf-protection" option check here and dropped the "MSVC" check.
I also moved the "check_c_compiler_flag" for "-fcf-protection=full" into config-ix.cmake

[MaskRay]

LGTM, but please give other reviewers some time for response.

compiler-rt/lib/builtins/i386/udivdi3.S

26 ↗

(On Diff #374751)

the indentation is off

compiler-rt/test/builtins/CMakeLists.txt

52

On non-x86 COMPILER_RT_ENABLE_CET should have failed when the control flow reaches here.

[MaskRay]

Ugh, wait. Can functions like __floatundisf be indirect call targets? Compilers generate direct calls to them and we don't expect users to indirectly call them.

I also don't find AArch64 bti notation.

[manojgupta]

I tried to use it locally and ran into following issues:

ld.lld: warning: /usr/lib64/clang/14.0.0/lib/linux/clang_rt.crtbegin-x86_64.o: -z force-ibt: file does not have GNU_PROPERTY_X86_FEATURE_1_IBT property
ld.lld: warning: /usr/lib64/clang/14.0.0/lib/linux/clang_rt.crtend-x86_64.o: -z force-ibt: file does not have GNU_PROPERTY_X86_FEATURE_1_IBT property

Apparently crt* files are not built with fcf-protection=full flag.

[xiongji90]

In D109811#3045740, @manojgupta wrote:

I tried to use it locally and ran into following issues:

ld.lld: warning: /usr/lib64/clang/14.0.0/lib/linux/clang_rt.crtbegin-x86_64.o: -z force-ibt: file does not have GNU_PROPERTY_X86_FEATURE_1_IBT property
ld.lld: warning: /usr/lib64/clang/14.0.0/lib/linux/clang_rt.crtend-x86_64.o: -z force-ibt: file does not have GNU_PROPERTY_X86_FEATURE_1_IBT property

Apparently crt* files are not built with fcf-protection=full flag.

Hi, @manojgupta
The latest patch have also enable CET for crt objects, all CRT tests pass on CET platform when CET is enabled for compiler-rt.

Thanks very much.

compiler-rt/test/builtins/CMakeLists.txt

52

Hi, @MaskRay
If target arch is not i386 or x86_64, we will report fatal error now.

[xiongji90]

In D109811#3021479, @MaskRay wrote:

Ugh, wait. Can functions like __floatundisf be indirect call targets? Compilers generate direct calls to them and we don't expect users to indirectly call them.

I also don't find AArch64 bti notation.

Hi, @MaskRay
I agree that we don't expect users to call those builtin functions indirectly but compiler doesn't have any mechanism to forbid users to do that currently. As long as those builtin functions are exported, user can call them directly or indirectly and this may lead to CET IBT violation.
I suggest to keep the "endbr" here, keeping the "endbr" in builtin functions just adds several endbr instructions into final binary and it can make CET enabling more unified, all functions will start with "endbr" when IBT is enabled, it will also avoid crash when some users are playing tricks with those builtin function via indirect calls.

Thanks very much.

[MaskRay]

In D109811#3050475, @xiongji90 wrote:

In D109811#3021479, @MaskRay wrote:

Ugh, wait. Can functions like __floatundisf be indirect call targets? Compilers generate direct calls to them and we don't expect users to indirectly call them.

I also don't find AArch64 bti notation.

Hi, @MaskRay
I agree that we don't expect users to call those builtin functions indirectly but compiler doesn't have any mechanism to forbid users to do that currently. As long as those builtin functions are exported, user can call them directly or indirectly and this may lead to CET IBT violation.

That will be pilot error. Unsupporting that use case is ok.

If there is legitimate use case (I doubt), we can add endbr as needed. The defensive approach is unnecessary.

I suggest to keep the "endbr" here, keeping the "endbr" in builtin functions just adds several endbr instructions into final binary and it can make CET enabling more unified, all functions will start with "endbr" when IBT is enabled, it will also avoid crash when some users are playing tricks with those builtin function via indirect calls.

I believe libgcc has much fewer endbr annotations.

Thanks very much.

[xiongji90]

In D109811#3052510, @MaskRay wrote:

In D109811#3050475, @xiongji90 wrote:

In D109811#3021479, @MaskRay wrote:

Ugh, wait. Can functions like __floatundisf be indirect call targets? Compilers generate direct calls to them and we don't expect users to indirectly call them.

I also don't find AArch64 bti notation.

Hi, @MaskRay
I agree that we don't expect users to call those builtin functions indirectly but compiler doesn't have any mechanism to forbid users to do that currently. As long as those builtin functions are exported, user can call them directly or indirectly and this may lead to CET IBT violation.

That will be pilot error. Unsupporting that use case is ok.

If there is legitimate use case (I doubt), we can add endbr as needed. The defensive approach is unnecessary.

I suggest to keep the "endbr" here, keeping the "endbr" in builtin functions just adds several endbr instructions into final binary and it can make CET enabling more unified, all functions will start with "endbr" when IBT is enabled, it will also avoid crash when some users are playing tricks with those builtin function via indirect calls.

I believe libgcc has much fewer endbr annotations.

Thanks very much.

Hi, @MaskRay
I just "objdump" libgcc and find all builtin functions have endbr annotation. It seems that libgcc adopted the same strategy as we did in this patch. CET enabling for runtime libraries is transparent to users, if we remove those endbr annotations, I am afraid we may have to tell users indirectly calling those builtin functions in their source code in a CET enabled environment is forbidden which is a little confusing.
Thanks very much.

[xiongji90]

Hi, @MaskRay
I removed endbr in builtin .S file according to your suggestion, could you help review again?
Thanks very much.

[MaskRay]

If no customization is needed (like libc++/libc++abi D107560), why not just require explicit CXXFLAGS? I don't see a need for a new CMake option.

[xiongji90]

In D109811#3065945, @MaskRay wrote:

If no customization is needed (like libc++/libc++abi D107560), why not just require explicit CXXFLAGS? I don't see a need for a new CMake option.

Hi, @MaskRay
Compiler-rt does not have a way to pass custom flags like libc++ (LIBCXX_COMPILE_FLAGS), if we add CET flag to cxxflag, it will impact all components when we build compiler-rt with other components.
And adding an cmake option also provides us a way to run compiler-rt tests with CET enabled.
Thanks very much.

[xiongji90]

In D109811#3065945, @MaskRay wrote:

If no customization is needed (like libc++/libc++abi D107560), why not just require explicit CXXFLAGS? I don't see a need for a new CMake option.

Hi, @manojgupta
Do you think explicit CXXFLAGS is enough? If not, I am afraid we have to add CMake option.

Thanks very much.

[manojgupta]

Unfortunately, explicit CXXFLAGS is not enough. If we add CXXFLAGS, it will make all of llvm including clang to be built with CET which is not what we want.

[compnerd]

I think that we might really want to subsequently rework the compiler-rt handling to use target_compile_options. This munging of flags manually is really rather onerous, but that is beyond the scope of this change.

compiler-rt/CMakeLists.txt

248–250

This is misrepresenting the state and is not really correct. Please change this. MSVC supports CET just as well with /CETCOMPAT. Please avoid using FATAL_ERROR, this allows any other configuration issues to be identified during the same run.

compiler-rt/lib/builtins/CMakeLists.txt

681

Please also ensure that the compiler supports the flag when adding it.

compiler-rt/lib/builtins/assembly.h

18

Please use the same thing we use elsewhere in compiler-rt:

#if !defined(__has_include)
#define __has_include(include) 0
#endif

Then you can unconditionally use __has_include.

compiler-rt/lib/crt/CMakeLists.txt

103

Similar - please ensure that the compiler supports the flag.

compiler-rt/test/builtins/CMakeLists.txt

53

Please use i?86|x86_64. Some targets use i686 and some use i386.

57

I think that it might be better to write this as:

if(COMPILER_RT_ENABLE_CET)
  if(NOT arch MATCHES "i?86|x86_64|AMD64")
    message(SEND_ERROR "${arch} does not support CET")
  endif()
  if(COMPILER_RT_HAS_FCF_PROTECTION_FLAG)
    list(APPEND BUILTINS_TEST_TARGET_CFLAGS -fcf-protection=full)
    string(REPLACE ";" " " BUILTINS_TEST_TARGET_CFLAGS"${BUILTINS_TEST_TARGET_CFLAGS}")
  endif()
endif()

[xiongji90]

compiler-rt/lib/builtins/CMakeLists.txt

681

Done.

compiler-rt/lib/builtins/assembly.h

18

Done.

compiler-rt/lib/crt/CMakeLists.txt

103

Done.

compiler-rt/test/builtins/CMakeLists.txt

53

Done.

57

Done.

[xiongji90]

compiler-rt/CMakeLists.txt

248–250

Hi, @compnerd
I added "MSVC" check here to avoid impacting MSVC compiler and changed "FATAL_ERROR" to "SEND_ERROR", is it OK to you?

Thanks very much.

[xiongji90]

Hi, @compnerd , @MaskRay and @manojgupta
I have updated the patches according to previous comments, could you help review it?

Thanks very much.

[manojgupta]

@compnerd can you please check again.

[xiongji90]

Hi, @compnerd
Could you check again if the latest update meets your requirement?
Thanks very much.

[MaskRay]

Ping @compnerd

[MaskRay]

Ping @compnerd

[xiongji90]

Hi, @compnerd
Kind ping~

[xiongji90]

Hi, @compnerd
Kind ping~

[compnerd]

compiler-rt/CMakeLists.txt

59

I don't think that this needs to be mark_as_advanced.

[MaskRay]

compiler-rt/lib/builtins/assembly.h

19

If __CET__ is defined (very new compiler), __has_include should be available, so you can simplify here a bit.

[xiongji90]

Hi, @MaskRay and @compnerd
I have updated the patch based on your comments and may need your approval again.
Thanks very much for the review.

compiler-rt/CMakeLists.txt

59

Done.
Thanks very much.

compiler-rt/lib/builtins/assembly.h

19

Done.
Thanks very much.

compiler-rt/CMakeLists.txt

 option(COMPILER_RT_BUILD_MEMPROF "Build memory profiling runtime" ON)
 mark_as_advanced(COMPILER_RT_BUILD_MEMPROF)
 option(COMPILER_RT_BUILD_XRAY_NO_PREINIT "Build xray with no preinit patching" OFF)
 mark_as_advanced(COMPILER_RT_BUILD_XRAY_NO_PREINIT)
 option(COMPILER_RT_BUILD_ORC "Build ORC runtime" ON)
 mark_as_advanced(COMPILER_RT_BUILD_ORC)
 option(COMPILER_RT_BUILD_GWP_ASAN "Build GWP-ASan, and link it into SCUDO" ON)
 mark_as_advanced(COMPILER_RT_BUILD_GWP_ASAN)
+option(COMPILER_RT_ENABLE_CET "Build Compiler RT with CET enabled" OFF)
 
 if(FUCHSIA)
   set(COMPILER_RT_HWASAN_WITH_INTERCEPTORS_DEFAULT OFF)
 else()
   set(COMPILER_RT_HWASAN_WITH_INTERCEPTORS_DEFAULT ON)
 endif()
 set(COMPILER_RT_HWASAN_WITH_INTERCEPTORS ${COMPILER_RT_HWASAN_WITH_INTERCEPTORS_DEFAULT} CACHE BOOL "Enable libc interceptors in HWASan (testing mode)")
 
 set(COMPILER_RT_BAREMETAL_BUILD OFF CACHE BOOL
   "Build for a bare-metal target.")
 
option(COMPILER_RT_USE_BUILTINS_LIBRARY
   "Use compiler-rt builtins instead of libgcc" ${DEFAULT_COMPILER_RT_USE_BUILTINS_LIBRARY})
 
 include(config-ix)
 
 #================================
 # Setup Compiler Flags
 #================================
 
+# fcf-protection is a gcc/clang option for CET support on Linux platforms.
+# We need to handle MSVC CET option on Windows platforms.
+if (NOT MSVC)
+  if (COMPILER_RT_ENABLE_CET AND NOT COMPILER_RT_HAS_FCF_PROTECTION_FLAG)
+    message(FATAL_ERROR "Compiler used to build compiler-rt doesn't support CET!")
+  endif()
+endif()
+
 if(MSVC)
   # Override any existing /W flags with /W4. This is what LLVM does.  Failing to
   # remove other /W[0-4] flags will result in a warning about overriding a
   # previous flag.
   if (COMPILER_RT_HAS_W4_FLAG)
     string(REGEX REPLACE " /W[0-4]" "" CMAKE_C_FLAGS "${CMAKE_C_FLAGS}")
     string(REGEX REPLACE " /W[0-4]" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
     append_string_if(COMPILER_RT_HAS_W4_FLAG /W4 CMAKE_C_FLAGS CMAKE_CXX_FLAGS)

compiler-rt/cmake/config-ix.cmake

if (MINGW)
     list(APPEND CMAKE_REQUIRED_LIBRARIES ${MINGW_LIBRARIES})
   endif()
 endif ()
 
 # CodeGen options.
 check_c_compiler_flag(-ffreestanding         COMPILER_RT_HAS_FFREESTANDING_FLAG)
 check_c_compiler_flag(-fomit-frame-pointer   COMPILER_RT_HAS_OMIT_FRAME_POINTER_FLAG)
 check_c_compiler_flag(-std=c11               COMPILER_RT_HAS_STD_C11_FLAG)
+check_c_compiler_flag(-fcf-protection=full   COMPILER_RT_HAS_FCF_PROTECTION_FLAG)
 check_cxx_compiler_flag(-fPIC                COMPILER_RT_HAS_FPIC_FLAG)
 check_cxx_compiler_flag(-fPIE                COMPILER_RT_HAS_FPIE_FLAG)
 check_cxx_compiler_flag(-fno-builtin         COMPILER_RT_HAS_FNO_BUILTIN_FLAG)
 check_cxx_compiler_flag(-fno-exceptions      COMPILER_RT_HAS_FNO_EXCEPTIONS_FLAG)
 check_cxx_compiler_flag(-fomit-frame-pointer COMPILER_RT_HAS_FOMIT_FRAME_POINTER_FLAG)
 check_cxx_compiler_flag(-funwind-tables      COMPILER_RT_HAS_FUNWIND_TABLES_FLAG)
 check_cxx_compiler_flag(-fno-stack-protector COMPILER_RT_HAS_FNO_STACK_PROTECTOR_FLAG)
 check_cxx_compiler_flag(-fno-sanitize=safe-stack COMPILER_RT_HAS_FNO_SANITIZE_SAFE_STACK_FLAG)

compiler-rt/lib/builtins/CMakeLists.txt

 
 if (APPLE)
   add_subdirectory(Darwin-excludes)
   add_subdirectory(macho_embedded)
   darwin_add_builtin_libraries(${BUILTIN_SUPPORTED_OS})
 else ()
   set(BUILTIN_CFLAGS "")
 
+  if (COMPILER_RT_HAS_FCF_PROTECTION_FLAG)
+    append_list_if(COMPILER_RT_ENABLE_CET -fcf-protection=full BUILTIN_CFLAGS)
+  endif()
+
   append_list_if(COMPILER_RT_HAS_FLOAT16 -DCOMPILER_RT_HAS_FLOAT16 BUILTIN_CFLAGS)
 
   append_list_if(COMPILER_RT_HAS_STD_C11_FLAG -std=c11 BUILTIN_CFLAGS)
 
   # These flags would normally be added to CMAKE_C_FLAGS by the llvm
   # cmake step. Add them manually if this is a standalone build.
   if(COMPILER_RT_STANDALONE_BUILD)
     if(COMPILER_RT_BUILTINS_ENABLE_PIC)

compiler-rt/lib/builtins/assembly.h

 //===-- assembly.h - compiler-rt assembler support macros -----------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file defines macros for use in compiler-rt assembler source.
 // This file is not part of the interface of this library.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef COMPILERRT_ASSEMBLY_H
 #define COMPILERRT_ASSEMBLY_H
 
+#if defined(__linux__) && defined(__CET__)
+#if __has_include(<cet.h>)
+#include <cet.h>
+#endif
+#endif
+
 #if defined(__APPLE__) && defined(__aarch64__)
 #define SEPARATOR %%
 #else
 #define SEPARATOR ;
 #endif
 
 #if defined(__APPLE__)
 #define HIDDEN(name) .private_extern name

compiler-rt/lib/crt/CMakeLists.txt

 check_cxx_section_exists(".init_array" COMPILER_RT_HAS_INITFINI_ARRAY
   SOURCE "volatile int x;\n__attribute__((constructor)) void f() {x = 0;}\nint main() { return 0; }\n")
 
 append_list_if(COMPILER_RT_HAS_STD_C11_FLAG -std=c11 CRT_CFLAGS)
 append_list_if(COMPILER_RT_HAS_INITFINI_ARRAY -DCRT_HAS_INITFINI_ARRAY CRT_CFLAGS)
 append_list_if(COMPILER_RT_CRT_USE_EH_FRAME_REGISTRY -DEH_USE_FRAME_REGISTRY CRT_CFLAGS)
 append_list_if(COMPILER_RT_HAS_FPIC_FLAG -fPIC CRT_CFLAGS)
 append_list_if(COMPILER_RT_HAS_WNO_PEDANTIC -Wno-pedantic CRT_CFLAGS)
+if (COMPILER_RT_HAS_FCF_PROTECTION_FLAG)
+  append_list_if(COMPILER_RT_ENABLE_CET -fcf-protection=full CRT_CFLAGS)
+endif()
 
 foreach(arch ${CRT_SUPPORTED_ARCH})
   add_compiler_rt_runtime(clang_rt.crtbegin
     OBJECT
     ARCHS ${arch}
     SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/crtbegin.c
     CFLAGS ${CRT_CFLAGS}
     PARENT_TARGET crt)
   add_compiler_rt_runtime(clang_rt.crtend
     OBJECT
     ARCHS ${arch}
     SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/crtend.c
     CFLAGS ${CRT_CFLAGS}
     PARENT_TARGET crt)
 endforeach()

compiler-rt/test/builtins/CMakeLists.txt

if (${arch} STREQUAL "riscv32")
     string(REPLACE ";" " " BUILTINS_TEST_TARGET_CFLAGS "${BUILTINS_TEST_TARGET_CFLAGS}")
   endif()
 
   if (${arch} MATCHES "arm|aarch64|arm64" AND COMPILER_RT_HAS_FLOAT16)
     list(APPEND BUILTINS_TEST_TARGET_CFLAGS -DCOMPILER_RT_HAS_FLOAT16)
     string(REPLACE ";" " " BUILTINS_TEST_TARGET_CFLAGS "${BUILTINS_TEST_TARGET_CFLAGS}")
   endif()
 
+  if(COMPILER_RT_ENABLE_CET)
+    if(NOT arch MATCHES "i?86|x86_64|AMD64")
+      message(SEND_ERROR "${arch} does not support CET")
+    endif()
+    if(COMPILER_RT_HAS_FCF_PROTECTION_FLAG)
+      list(APPEND BUILTINS_TEST_TARGET_CFLAGS -fcf-protection=full)
+      string(REPLACE ";" " " BUILTINS_TEST_TARGET_CFLAGS "${BUILTINS_TEST_TARGET_CFLAGS}")
+    endif()
+  endif()
+
   # Compute builtins available in library and add them as lit features.
   if(APPLE)
     # TODO: Support other Apple platforms.
     set(BUILTIN_LIB_TARGET_NAME "clang_rt.builtins_${arch}_osx")
   else()
     set(BUILTIN_LIB_TARGET_NAME "clang_rt.builtins-${arch}")
   endif()
   if (NOT TARGET "${BUILTIN_LIB_TARGET_NAME}")

compiler-rt/test/crt/CMakeLists.txt

 if (COMPILER_RT_BUILD_CRT AND COMPILER_RT_HAS_CRT)
   foreach(arch ${CRT_TEST_ARCH})
     set(CRT_TEST_TARGET_ARCH ${arch})
     string(TOLOWER "-${arch}-${OS_NAME}" CRT_TEST_CONFIG_SUFFIX)
     get_test_cc_for_arch(${arch} CRT_TEST_TARGET_CC CRT_TEST_TARGET_CFLAGS)
     string(TOUPPER ${arch} ARCH_UPPER_CASE)
     set(CONFIG_NAME ${ARCH_UPPER_CASE}${OS_NAME}Config)
 
+    if (COMPILER_RT_ENABLE_CET)
+      if (${arch} MATCHES "i386|x86_64")
+        list(APPEND CRT_TEST_TARGET_CFLAGS -fcf-protection=full)
+        string(REPLACE ";" " " CRT_TEST_TARGET_CFLAGS "${CRT_TEST_TARGET_CFLAGS}")
+      else()
+        message(FATAL_ERROR "The target arch ${arch} doesn't support CET")
+      endif()
+    endif()
     configure_lit_site_cfg(
       ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.py.in
       ${CMAKE_CURRENT_BINARY_DIR}/${CONFIG_NAME}/lit.site.cfg.py)
     list(APPEND CRT_TESTSUITES ${CMAKE_CURRENT_BINARY_DIR}/${CONFIG_NAME})
   endforeach()
 endif()
 
 add_lit_testsuite(check-crt "Running the CRT tests"
   ${CRT_TESTSUITES}
   DEPENDS ${CRT_TEST_DEPS})
 set_target_properties(check-crt PROPERTIES FOLDER "Compiler-RT Misc")