Merge pull request #2090 from RossBrunton/ross/harden

Hardening flags

.github/workflows/cmake.yml

@@ -49,7 +49,7 @@ jobs:
     - name: Install apt packages
       run: |
         sudo apt-get update
-        sudo apt-get install -y ${{matrix.compiler.c}}
+        sudo apt-get install -y ${{matrix.compiler.c}} devscripts
 
     - name: Install libhwloc
       run: .github/scripts/install_hwloc.sh
@@ -82,6 +82,7 @@ jobs:
 
     - name: Configure CMake
       if: matrix.os == 'ubuntu-22.04'
+      # WEXTRA: https://github.com/oneapi-src/unified-runtime/issues/2109
       run: >
         cmake
         -B${{github.workspace}}/build
@@ -91,6 +92,7 @@ jobs:
         -DCMAKE_BUILD_TYPE=${{matrix.build_type}}
         -DUR_BUILD_TESTS=ON
         -DUR_FORMAT_CPP_STYLE=OFF
+        -DUR_DEVELOPER_MODE=ON
         -DUR_DPCXX=${{github.workspace}}/dpcpp_compiler/bin/clang++
         -DUR_CONFORMANCE_TEST_LOADER=OFF
         ${{matrix.libbacktrace}}
@@ -99,6 +101,8 @@ jobs:
 
     - name: Configure CMake
       if: matrix.os == 'ubuntu-20.04'
+      # WEXTRA: https://github.com/oneapi-src/unified-runtime/issues/2109
+      # Note: Disable Werror, since 20.04 raises different ones than 22.04
       run: >
         cmake
         -B${{github.workspace}}/build
@@ -108,13 +112,19 @@ jobs:
         -DCMAKE_BUILD_TYPE=${{matrix.build_type}}
         -DUR_BUILD_TESTS=ON
         -DUR_FORMAT_CPP_STYLE=OFF
+        -DUR_DEVELOPER_MODE=OFF
         ${{matrix.libbacktrace}}
         ${{matrix.pool_tracking}}
         ${{matrix.latency_tracking}}
 
     - name: Build
       run: cmake --build ${{github.workspace}}/build -j $(nproc)
 
+    - name: Verify hardening flags have been set
+      run: cmake --build ${{github.workspace}}/build --target verify-hardening
+      # https://github.com/oneapi-src/unified-runtime/issues/2120
+      if: ${{ matrix.compiler.cxx != 'clang++' && matrix.os != 'ubuntu-20.04' }}
+
     - name: Test
       working-directory: ${{github.workspace}}/build
       run: ctest -C ${{matrix.build_type}} --output-on-failure -L "umf|loader|validation|tracing|unit|urtrace"

CMakeLists.txt

@@ -34,7 +34,7 @@ option(UR_BUILD_EXAMPLES "Build example applications." ON)
 option(UR_BUILD_TESTS "Build unit tests." ON)
 option(UR_BUILD_TOOLS "build ur tools" ON)
 option(UR_FORMAT_CPP_STYLE "format code style of C++ sources" OFF)
-option(UR_DEVELOPER_MODE "enable developer checks, treats warnings as errors" OFF)
+option(UR_DEVELOPER_MODE "treats warnings as errors" OFF)
 option(UR_ENABLE_FAST_SPEC_MODE "enable fast specification generation mode" OFF)
 option(UR_USE_ASAN "enable AddressSanitizer" OFF)
 option(UR_USE_UBSAN "enable UndefinedBehaviorSanitizer" OFF)
@@ -162,6 +162,12 @@ if(UR_ENABLE_TRACING)
             LIBRARY_OUTPUT_DIRECTORY ${CMAKE_LIBRARY_OUTPUT_DIRECTORY}
         )
 
+        if (NOT MSVC)
+            # Hardening flags cause issues on Windows
+            add_ur_target_compile_options(xptifw)
+            add_ur_target_link_options(xptifw)
+        endif()
+
         if (UR_STATIC_LOADER)
             install(TARGETS xpti xptifw
                 EXPORT ${PROJECT_NAME}-targets
@@ -270,6 +276,13 @@ add_custom_target(verify-licenses
     COMMENT "Verify all files contain a license."
 )
 
+# Add hardening check
+add_custom_target(verify-hardening
+    COMMAND "${PROJECT_SOURCE_DIR}/scripts/check-hardening.sh"
+        ${CMAKE_BINARY_DIR}
+    COMMENT "Check hardening settings on built binaries and libraries"
+)
+
 # Add code formatter target
 add_custom_target(cppformat)
 # ... and all source files to the formatter

README.md

@@ -122,7 +122,7 @@ List of options provided by CMake:
 | UR_BUILD_TESTS | Build the tests | ON/OFF | ON |
 | UR_BUILD_TOOLS | Build tools | ON/OFF | ON |
 | UR_FORMAT_CPP_STYLE | Format code style | ON/OFF | OFF |
-| UR_DEVELOPER_MODE | Treat warnings as errors and enables additional checks | ON/OFF | OFF |
+| UR_DEVELOPER_MODE | Treat warnings as errors | ON/OFF | OFF |
 | UR_ENABLE_FAST_SPEC_MODE | Enable fast specification generation mode | ON/OFF | OFF |
 | UR_USE_ASAN | Enable AddressSanitizer | ON/OFF | OFF |
 | UR_USE_TSAN | Enable ThreadSanitizer | ON/OFF | OFF |

cmake/helpers.cmake

@@ -60,26 +60,40 @@ endmacro()
 
 function(add_ur_target_compile_options name)
     if(NOT MSVC)
+        target_compile_definitions(${name} PRIVATE -D_FORTIFY_SOURCE=2)
         target_compile_options(${name} PRIVATE
-            -fPIC
+            # Warning options
             -Wall
             -Wpedantic
             -Wempty-body
+            -Wformat
+            -Wformat-security
             -Wunused-parameter
+
+            # Hardening options
+            -fPIC
+            -fstack-protector-strong
+            -fvisibility=hidden # Required for -fsanitize=cfi
+            # -fsanitize=cfi requires -flto, which breaks a lot of things
+            # See: https://github.com/oneapi-src/unified-runtime/issues/2120
+            # -flto
+            # $<$<CXX_COMPILER_ID:Clang,AppleClang>:-fsanitize=cfi>
+            # -fcf-protection not supported in GCC < 8
+            $<$<OR:$<NOT:$<CXX_COMPILER_ID:GNU>>,$<VERSION_GREATER_EQUAL:$<CXX_COMPILER_VERSION>,8>>:-fcf-protection=full>
+            # -fstack-clash-protection is not supported in apple clang or GCC < 8
+            $<$<AND:$<CXX_COMPILER_ID:GNU>,$<VERSION_GREATER_EQUAL:$<CXX_COMPILER_VERSION>,8>>:-fstack-clash-protection>
+            $<$<CXX_COMPILER_ID:Clang>:-fstack-clash-protection>
+
+            # Colored output
             $<$<CXX_COMPILER_ID:GNU>:-fdiagnostics-color=always>
             $<$<CXX_COMPILER_ID:Clang,AppleClang>:-fcolor-diagnostics>
         )
+        if (UR_DEVELOPER_MODE)
+            target_compile_options(${name} PRIVATE -Werror)
+        endif()
         if (CMAKE_BUILD_TYPE STREQUAL "Release")
-            target_compile_definitions(${name} PRIVATE -D_FORTIFY_SOURCE=2)
             target_compile_options(${name} PRIVATE -fvisibility=hidden)
         endif()
-        if(UR_DEVELOPER_MODE)
-            target_compile_options(${name} PRIVATE
-                -Werror
-                -fno-omit-frame-pointer
-                -fstack-protector-strong
-            )
-        endif()
     elseif(MSVC)
         target_compile_options(${name} PRIVATE
             $<$<CXX_COMPILER_ID:MSVC>:/MP>  # clang-cl.exe does not support /MP
@@ -103,7 +117,15 @@ endfunction()
 function(add_ur_target_link_options name)
     if(NOT MSVC)
         if (NOT APPLE)
-            target_link_options(${name} PRIVATE "LINKER:-z,relro,-z,now")
+            target_link_options(${name} PRIVATE "LINKER:-z,relro,-z,now,-z,noexecstack")
+            if (UR_DEVELOPER_MODE)
+                target_link_options(${name} PRIVATE -Werror)
+            endif()
+            if (CMAKE_BUILD_TYPE STREQUAL "Release")
+                target_link_options(${name} PRIVATE
+                    $<$<CXX_COMPILER_ID:GNU>:-pie>
+                )
+            endif()
         endif()
     elseif(MSVC)
         target_link_options(${name} PRIVATE

scripts/check-hardening.sh

@@ -0,0 +1,42 @@
+#!/bin/sh
+if [ -z $1 ]; then
+    echo "Usage: $0 builddir" >&2;
+    exit;
+fi
+
+which hardening-check >> /dev/null;
+if [ $? != "0" ]; then
+    echo "hardening-check not found - on Ubuntu it is from the 'devscripts' package." >&2;
+    exit;
+fi
+
+RET=0;
+
+for file in $1/bin/*; do
+    case "$file" in
+        */urtrace)
+            # This is a python script
+            true;;
+        *)
+            hardening-check -q --nocfprotection --nofortify $file;;
+    esac
+    RET=$(($RET + $?))
+done;
+
+for file in $1/lib/*.so; do
+    case "$file" in
+        */libOpenCL*)
+            # This is not built as part of UR
+            true;;
+        */libzeCallMap.so | */libur_mock_headers.so)
+            # Only used in testing, and are too simple for many of the hardening flags to have an effect.
+            true;;
+        *)
+            hardening-check -q --nocfprotection --nofortify $file;;
+    esac
+    RET=$(($RET + $?))
+done;
+
+if [ $RET != "0" ]; then
+    exit 1;
+fi

source/adapters/level_zero/CMakeLists.txt

@@ -66,6 +66,21 @@ if(UR_BUILD_ADAPTER_L0)
         )
     endif()
 
+    # Ensure UR flags are propagated to level zero
+    # Note: UR compile options cause issues under MSVC
+    if(NOT MSVC)
+        foreach(TARGET IN ITEMS ze_loader ze_validation_layer ze_tracing_layer ze_null)
+            if (TARGET TARGET)
+                add_ur_target_compile_options(${TARGET})
+                add_ur_target_link_options(${TARGET})
+                target_compile_options(${TARGET} PRIVATE
+                    $<$<CXX_COMPILER_ID:GNU,Clang,Intel,IntelLLVM>:-Wno-error -Wno-unused-parameter>
+                    $<$<CXX_COMPILER_ID:MSVC>:/WX- /UUNICODE>
+                )
+            endif()
+        endforeach()
+    endif()
+
     if(NOT WIN32)
         target_sources(ur_adapter_level_zero
             PRIVATE

source/loader/layers/tracing/ur_tracing_layer.cpp

@@ -37,7 +37,7 @@ struct XptiContextManager {
 static std::shared_ptr<XptiContextManager> xptiContextManagerGet() {
     static auto contextManager = std::make_shared<XptiContextManager>();
     return contextManager;
-};
+}
 static thread_local xpti_td *activeEvent;
 
 ///////////////////////////////////////////////////////////////////////////////

test/adapters/level_zero/zeCallMap.cpp

@@ -9,4 +9,5 @@
 // Map used by L0 adapter to count the number of calls to each L0 function
 // Lifetime is managed by the adapter, this variable is defined here
 // only so that we can read it from the tests.
-std::map<std::string, int> *ZeCallCount = nullptr;
+__attribute__((visibility("default"))) std::map<std::string, int> *ZeCallCount =
+    nullptr;

test/conformance/exp_command_buffer/fixtures.h

@@ -126,7 +126,8 @@ struct urUpdatableCommandBufferExpTest : uur::urQueueTest {
 
         // Create a command-buffer with update enabled.
         ur_exp_command_buffer_desc_t desc{
-            UR_STRUCTURE_TYPE_EXP_COMMAND_BUFFER_DESC, nullptr, true};
+            UR_STRUCTURE_TYPE_EXP_COMMAND_BUFFER_DESC, nullptr, true, false,
+            false};
 
         ASSERT_SUCCESS(urCommandBufferCreateExp(context, device, &desc,
                                                 &updatable_cmd_buf_handle));