CDRIVER-5648 fix libfuzzer integration #1686

spencerjackson · 2024-08-06T03:53:47Z

Summary

This patch modifies the CMake rules in order to allow compilation of the already existing libfuzzer entrypoint.

What Changed?

fuzz_test_libbson.c renamed to fuzz_test_init_from_json.c
We could have multiple entrypoints, so it makes sense to name the file after the function we're fuzzing.
fuzz_test_init_from_json.c simplified to remove memory allocation
The faster we can run the entrypoint, the more iterations per second we'll be able to manage.
CMake option MONGO_FUZZ added
When building a fuzzer, all code in all TUs must be compiled with coverage instrumentation. The final link step must statically link libfuzzer's main symbol. Adding sanitizers to all TUs is also beneficial.
This new option enables AUBSan and a sanitizer called "fuzzer-no-link", which enables CovSAN and all the other flags libfuzzer needs to operate. Finally, this option sets MONGO_FUZZ_ENGINE to the linker flags needed to statically link in libfuzzer's main. Any binaries which provide fuzzer entrypoints need to use MONGO_FUZZ_ENGINE in their LINK_FLAGS.
Added src/libbson/fuzz/CMakeLists.txt
This file compiles the fuzzer entrypoint, iff MONGO_FUZZ is enabled.

I haven't used CMake in a while, so feedback is appreciated!

eramongodb · 2024-08-07T19:33:49Z

build/cmake/Fuzzer.cmake

Remove this file in favor of:

reusing MONGO_SANITIZE to set sanitizer options, and

scoping fuzz-testing-related properties and targets to src/libbson/fuzz/CMakeLists.txt.

Described in further detail in other comments.

eramongodb · 2024-08-07T19:36:22Z

src/libbson/CMakeLists.txt

@@ -399,6 +399,8 @@ install (EXPORT bson-targets
 include (LegacyPackage)
 include (CPack)

+add_subdirectory(fuzz)


Suggested change

add_subdirectory(fuzz)

if (MONGO_FUZZ)

add_subdirectory(fuzz)

endif ()

Condition the definition of fuzz targets on MONGO_FUZZ.

eramongodb · 2024-08-07T19:36:54Z

src/libbson/fuzz/CMakeLists.txt

@@ -0,0 +1,9 @@
+if (MONGO_FUZZ)


Suggested change

if (MONGO_FUZZ)

Condition the add_subdirectory() rather than this file's contents.

eramongodb · 2024-08-07T19:37:24Z

src/libbson/fuzz/CMakeLists.txt

+target_include_directories(fuzz_test_init_from_json PRIVATE "src" "${PROJECT_BINARY_DIR}/src/")
+target_link_libraries(fuzz_test_init_from_json ${bson_libs})


Suggested change

target_include_directories(fuzz_test_init_from_json PRIVATE "src" "${PROJECT_BINARY_DIR}/src/")

target_link_libraries(fuzz_test_init_from_json ${bson_libs})

target_link_libraries(fuzz_test_init_from_json PRIVATE bson_static)

Use same library linkage pattern as existing test executables.

eramongodb · 2024-08-07T19:39:54Z

build/cmake/Fuzzer.cmake

+mongo_setting(
+  MONGO_FUZZ "Whether fuzz testing is enabled"
+  TYPE BOOL
+  DEFAULT FALSE
+)


Suggested change

mongo_setting(

MONGO_FUZZ "Whether fuzz testing is enabled"

TYPE BOOL

DEFAULT FALSE

)

Move the MONGO_FUZZ option definition into build/cmake/Sanitizers.cmake prior to the MONGO_SANITIZE option definition and redefine as follows:

mongo_bool_setting( MONGO_FUZZ "Enable LibFuzzer integration" DEFAULT VALUE OFF VALIDATE CODE [[ if (NOT ENABLE_STATIC) message (FATAL_ERROR "MONGO_FUZZ requires ENABLE_STATIC=ON or ENABLE_STATIC=BUILD_ONLY") endif () ]] )

eramongodb · 2024-08-07T20:00:28Z

build/cmake/Fuzzer.cmake

+if (MONGO_FUZZ)
+set(CMAKE_C_FLAGS "-fsanitize=undefined -fsanitize=address -fsanitize=fuzzer-no-link" CACHE STRING "Custom comp" FORCE)
+set(MONGO_FUZZ_ENGINE -fsanitize=fuzzer)
+endif ()


Suggested change

if (MONGO_FUZZ)

set(CMAKE_C_FLAGS "-fsanitize=undefined -fsanitize=address -fsanitize=fuzzer-no-link" CACHE STRING "Custom comp" FORCE)

set(MONGO_FUZZ_ENGINE -fsanitize=fuzzer)

endif ()

Remove in favor of reusing the existing MONGO_SANITIZE option by adding the following after the MONGO_SANITIZE option definition in build/cmake/Sanitizers.cmake:

if (MONGO_FUZZ) set(mongo_fuzz_options "address,undefined,fuzzer-no-link") if (NOT "${MONGO_SANITIZE}" STREQUAL "${mongo_fuzz_options}") message(WARNING "Overriding user-provided MONGO_SANITIZE options in favor of MONGO_SANITIZE=ON") endif() set_property (CACHE MONGO_SANITIZE PROPERTY VALUE "${mongo_fuzz_options}") endif ()

eramongodb · 2024-08-07T20:02:40Z

src/libbson/fuzz/CMakeLists.txt

+add_executable(fuzz_test_init_from_json
+   fuzz_test_init_from_json.c)


Suggested change

add_executable(fuzz_test_init_from_json

fuzz_test_init_from_json.c)

add_executable(fuzz_test_init_from_json EXCLUDE_FROM_ALL fuzz_test_init_from_json.c)

Do not build fuzz executable as part of build-and-install routines.

eramongodb · 2024-08-07T20:03:36Z

src/libbson/fuzz/CMakeLists.txt

+   fuzz_test_init_from_json.c)
+target_include_directories(fuzz_test_init_from_json PRIVATE "src" "${PROJECT_BINARY_DIR}/src/")
+target_link_libraries(fuzz_test_init_from_json ${bson_libs})
+set_target_properties(fuzz_test_init_from_json PROPERTIES LINK_FLAGS ${MONGO_FUZZ_ENGINE})


Suggested change

set_target_properties(fuzz_test_init_from_json PROPERTIES LINK_FLAGS ${MONGO_FUZZ_ENGINE})

set_property(TARGET fuzz_test_init_from_json APPEND PROPERTY LINK_OPTIONS -fsanitize=fuzzer)

Pass fuzzer "with link" option directly and avoid potentially overwriting existing LINK_OPTIONS values.

eramongodb

Thank you very much! One last suggestion remaining; otherwise, LGTM.

eramongodb · 2024-08-09T14:28:04Z

build/cmake/Sanitizers.cmake

+mongo_bool_setting(
+    MONGO_FUZZ "Enable LibFuzzer integration"
+    DEFAULT VALUE OFF
+    VALIDATE CODE [[
+        if (NOT ENABLE_STATIC)
+	    message (FATAL_ERROR "MONGO_FUZZ requires ENABLE_STATIC=ON or ENABLE_STATIC=BUILD_ONLY")
+	endif ()
+    ]]
+)
+
+if (MONGO_FUZZ)
+    set(mongo_fuzz_options "address,undefined,fuzzer-no-link")
+    if (NOT "${MONGO_SANITIZE}" STREQUAL "${mongo_fuzz_options}")
+        message(WARNING "Overriding user-provided MONGO_SANITIZE options in favor of MONGO_SANITIZE=ON")
+    endif ()
+    set_property (CACHE MONGO_SANITIZE PROPERTY VALUE "${mongo_fuzz_options}")
+endif ()


These commands must come before MONGO_SANITIZE is parsed. Move up to come immediately after the MONGO_SANITIZE option definition:

mongo_setting (MONGO_SANITIZE ...) mongo_bool_setting (MONGO_FUZZ ...) if (MONGO_FUZZ) set(mongo_fuzz_options "address,undefined,fuzzer-no-link") ... endif () # Replace commas with semicolons for the genex string(REPLACE ";" "," _sanitize "${MONGO_SANITIZE}") if (_sanitize) ... endif ()

spencerjackson · 2024-08-09T17:05:09Z

Comments addressed, I also tweaked the logic in the MONGO_FUZZ VALIDATE block, so that it errors only if (MONGO_FUZZ AND NOT ENABLE_STATIC)

kevinAlbs

Thank you for the contribution. LGTM with minor CMake suggested tweaks.

kevinAlbs · 2024-08-12T14:51:26Z

build/cmake/Sanitizers.cmake

+if (MONGO_FUZZ)
+    set(mongo_fuzz_options "address,undefined,fuzzer-no-link")
+    if (NOT "${MONGO_SANITIZE}" STREQUAL "${mongo_fuzz_options}")
+        message(WARNING "Overriding user-provided MONGO_SANITIZE options in favor of MONGO_SANITIZE=ON")


Suggested change

message(WARNING "Overriding user-provided MONGO_SANITIZE options in favor of MONGO_SANITIZE=ON")

message(WARNING "Overriding user-provided MONGO_SANITIZE options due to MONGO_FUZZ=ON")

kevinAlbs · 2024-08-12T14:58:52Z

build/cmake/Sanitizers.cmake

+
+if (MONGO_FUZZ)
+    set(mongo_fuzz_options "address,undefined,fuzzer-no-link")
+    if (NOT "${MONGO_SANITIZE}" STREQUAL "${mongo_fuzz_options}")


Suggested change

if (NOT "${MONGO_SANITIZE}" STREQUAL "${mongo_fuzz_options}")

if (MONGO_SANITIZE AND NOT "${MONGO_SANITIZE}" STREQUAL "${mongo_fuzz_options}")

To skip warning when MONGO_SANITIZE is not set.

spencerjackson added 5 commits August 5, 2024 18:16

Implement libfuzzer entrypoint

898ed3c

Simplify libfuzzer entrypoint

2ab5cb0

Structure libbson/CMakeLists.txt to isolate fuzzer definitions

4806959

Rework fuzzer enablement to use a switch

26fcc2d

Lint

d36b42b

kevinAlbs requested review from vector-of-bool, kevinAlbs and eramongodb and removed request for vector-of-bool August 6, 2024 11:54

eramongodb requested changes Aug 7, 2024

View reviewed changes

Address comments

6af6290

spencerjackson requested a review from eramongodb August 8, 2024 22:20

eramongodb approved these changes Aug 9, 2024

View reviewed changes

spencerjackson added 2 commits August 9, 2024 15:19

Fix boolean logic in validation

4a440cf

Set MONGO_FUZZ related options before sanitizers are set

b902434

eramongodb self-assigned this Aug 9, 2024

kevinAlbs approved these changes Aug 12, 2024

View reviewed changes

Address comments

491ba98

eramongodb merged commit 8cc212b into mongodb:master Aug 13, 2024
42 of 43 checks passed

		target_include_directories(fuzz_test_init_from_json PRIVATE "src" "${PROJECT_BINARY_DIR}/src/")
		target_link_libraries(fuzz_test_init_from_json ${bson_libs})

		add_executable(fuzz_test_init_from_json
		fuzz_test_init_from_json.c)

	add_executable(fuzz_test_init_from_json
	fuzz_test_init_from_json.c)
	add_executable(fuzz_test_init_from_json EXCLUDE_FROM_ALL fuzz_test_init_from_json.c)

	set_target_properties(fuzz_test_init_from_json PROPERTIES LINK_FLAGS ${MONGO_FUZZ_ENGINE})
	set_property(TARGET fuzz_test_init_from_json APPEND PROPERTY LINK_OPTIONS -fsanitize=fuzzer)

	message(WARNING "Overriding user-provided MONGO_SANITIZE options in favor of MONGO_SANITIZE=ON")
	message(WARNING "Overriding user-provided MONGO_SANITIZE options due to MONGO_FUZZ=ON")

	if (NOT "${MONGO_SANITIZE}" STREQUAL "${mongo_fuzz_options}")
	if (MONGO_SANITIZE AND NOT "${MONGO_SANITIZE}" STREQUAL "${mongo_fuzz_options}")

CDRIVER-5648 fix libfuzzer integration #1686

CDRIVER-5648 fix libfuzzer integration #1686

Uh oh!

Conversation

spencerjackson commented Aug 6, 2024

Summary

What Changed?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eramongodb Aug 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eramongodb Aug 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eramongodb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spencerjackson commented Aug 9, 2024

Uh oh!

kevinAlbs left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

eramongodb Aug 7, 2024 •

edited

Loading

eramongodb Aug 7, 2024 •

edited

Loading