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.

Sign up for GitHub

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

Jump to bottom

Enables Conditional Compiler Protections #6743

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
+33 −1
Draft

Enables Conditional Compiler Protections #6743

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
24288a4
Enables Conditional Compiler Protections
thanasisk Mar 6, 2024
24360a3
Merge branch 'timescale:main' into binprot
thanasisk Mar 6, 2024
951bec6
Merge branch 'timescale:main' into binary-protections-bench
thanasisk Mar 12, 2024
f914add
Fortify And CFI Are Enabled Only Conditionally
thanasisk Mar 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 21 additions & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,8 +274,28 @@ if(CMAKE_C_COMPILER_ID MATCHES "GNU|AppleClang|Clang")
"The compiler ${CMAKE_C_COMPILER_ID} does not support -fvisibility=hidden"
)
endif(NOT CC_SUPPORTS_VISIBILITY_HIDDEN)
# security options go here - these are not supported by all compilers
# first and foremost, do we have LTO?
check_c_compiler_flag(-flto CC_SUPPORTS_LTO)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can be pretty sure that if the -fsanitize=cfi works, -flto will work as well, so no need to check this separately unless you want to provide a very specific error message saying what is wrong (and you do not have such a message here).

To enable Clang’s available CFI schemes, use the flag -fsanitize=cfi. You can also enable a subset of available schemes. As currently implemented, all schemes rely on link-time optimization (LTO); so it is required to specify -flto, and the linker used must support LTO, for example via the gold plugin.

# if we are building on a GLIBC environment, FORTIFY is available
check_c_compiler_flag(-D_FORTIFY_SOURCE CC_SUPPORTS_FORTIFY)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this ever fail? Since you're just setting the _FORTIFY_SOURCE preprocessor symbol, it should not be possible for this to fail since the flag is always accepted for C compilers.

You probably need to construct a small program that should generate warning, compile it, and set this flag if the program emits a warning. Something like this:

Suggested change
# if we are building on a GLIBC environment, FORTIFY is available
check_c_compiler_flag(-D_FORTIFY_SOURCE CC_SUPPORTS_FORTIFY)
include(CheckCSourceCompiles)
set(CMAKE_REQUIRED_FLAGS -Werror)
set(CMAKE_REQUIRED_DEFINITIONS -D_FORTIFY_SOURCE)
# If this build succeeds, _FORTIFY_SOURCE is not available
check_c_source_compiles("
#include <string.h>
int main() {
char buffer[8];
strcpy(buffer, \"hello world\");
}
" FORTIFY_NOT_AVAILABLE)


if(CC_SUPPORTS_FORTIFY)
add_compile_options(-D_FORTIFY_SOURCE=2)
if(CC_SUPPORTS_LTO)
add_compile_options(-flto)
endif()
else()
message(STATUS "Compiler ${CMAKE_C_COMPILER_ID} does not support -D_FORTIFY_SOURCE=1")
endif()
# CFI is currently supported by clang
check_c_compiler_flag(-CFI CC_SUPPORTS_CFI)
Copy link
Contributor

Choose a reason for hiding this comment

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

When using CLang it does not have any -CFI option. I see the following options though:

mats@fury:~$ clang --help | grep cfi
  -fno-sanitize-cfi-canonical-jump-tables
  -fno-sanitize-cfi-cross-dso
  -fsanitize-cfi-canonical-jump-tables
  -fsanitize-cfi-cross-dso
  -fsanitize-cfi-icall-generalize-pointers
  -mlvi-cfi               Enable only control-flow mitigations for Load Value Injection (LVI)
  -mno-lvi-cfi            Disable control-flow mitigations for Load Value Injection (LVI)

if(CC_SUPPORTS_CFI AND CC_SUPPORTS_LTO)
add_compile_options(-flto -fsanitize=cfi)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you need the flag -fvisibility=hidden here as well for this to compile. I get an error when trying:

mats@fury:~/lang/cc/examples$ clang -flto -fsanitize=cfi struct.c 
clang: error: invalid argument '-fsanitize=cfi' only allowed with '-fvisibility='
mats@fury:~/lang/cc/examples$ clang -flto -fvisibility=hidden -fsanitize=cfi struct.c 
mats@fury:~/lang/cc/examples$

Copy link
Author

Choose a reason for hiding this comment

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

@mkindahl which platform you compiled on? Fedora 39/gcc/x86_64 was not able to reproduce

Copy link
Contributor

@mkindahl mkindahl Mar 7, 2024 •
edited

Choose a reason for hiding this comment

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

I am using Ubuntu 22.04 LTS.

mats@fury:~/lang/cc/examples$ lsb_release --all
No LSB modules are available.
Distributor ID: Ubuntu
Description:    Ubuntu 22.04.4 LTS
Release:        22.04
Codename:       jammy
mats@fury:~/lang/cc/examples$ clang --version
Ubuntu clang version 14.0.0-1ubuntu1.1
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

else()
message(STATUS "Compiler ${CMAKE_C_COMPILER_ID} does not support CFI")
endif()
endif()

# On Windows, default to only include Release builds so MSBuild.exe 'just works'
if(WIN32 AND NOT CMAKE_CONFIGURATION_TYPES)
set(CMAKE_CONFIGURATION_TYPES
Expand Down