From 117570fbcaf4ce6c4dd939c2eb87ccf34a9421f0 Mon Sep 17 00:00:00 2001 From: Xinhao Yuan Date: Mon, 9 Dec 2024 10:50:14 -0800 Subject: [PATCH] Pass mutation metadata from the domain mutation to `LLVMFuzzerMutate`. To avoid memory safety issues, a manager class is added to restrict the access of the metadata to be only available during the domain mutator's invocation to `LLVMFuzzerCustomMutator` PiperOrigin-RevId: 704343455 --- fuzztest/BUILD | 5 +- fuzztest/CMakeLists.txt | 4 ++ fuzztest/llvm_fuzzer_wrapper.cc | 89 ++++++++++++++++++++++++++++++--- 3 files changed, 89 insertions(+), 9 deletions(-) diff --git a/fuzztest/BUILD b/fuzztest/BUILD index 45c2f2fa..4dcdb3ce 100644 --- a/fuzztest/BUILD +++ b/fuzztest/BUILD @@ -155,15 +155,18 @@ cc_library( testonly = True, srcs = ["llvm_fuzzer_wrapper.cc"], deps = [ - ":coverage", ":domain_core", ":fuzztest", ":fuzztest_macros", ":io", + ":logging", + "@com_google_absl//absl/base:core_headers", + "@com_google_absl//absl/base:no_destructor", "@com_google_absl//absl/flags:flag", "@com_google_absl//absl/log:check", "@com_google_absl//absl/random", "@com_google_absl//absl/random:bit_gen_ref", + "@com_google_absl//absl/synchronization", ], alwayslink = True, ) diff --git a/fuzztest/CMakeLists.txt b/fuzztest/CMakeLists.txt index 0f164e95..d0cfee4c 100644 --- a/fuzztest/CMakeLists.txt +++ b/fuzztest/CMakeLists.txt @@ -128,12 +128,16 @@ fuzztest_cc_library( fuzztest::fuzztest fuzztest::io fuzztest::llvm_fuzzer_main + fuzztest::logging + absl::core_headers absl::flags absl::log + absl::no_destructor absl::random_random absl::random_bit_gen_ref absl::strings absl::string_view + absl::synchronization re2::re2 ) diff --git a/fuzztest/llvm_fuzzer_wrapper.cc b/fuzztest/llvm_fuzzer_wrapper.cc index 647c40ea..f0c59b75 100644 --- a/fuzztest/llvm_fuzzer_wrapper.cc +++ b/fuzztest/llvm_fuzzer_wrapper.cc @@ -2,21 +2,26 @@ #include #include #include +#include #include +#include #include +#include "absl/base/no_destructor.h" +#include "absl/base/thread_annotations.h" #include "absl/flags/declare.h" #include "absl/flags/flag.h" #include "absl/log/check.h" #include "absl/random/bit_gen_ref.h" #include "absl/random/random.h" +#include "absl/synchronization/mutex.h" #include "./fuzztest/fuzztest.h" #include "./fuzztest/fuzztest_macros.h" #include "./fuzztest/internal/domains/arbitrary_impl.h" #include "./fuzztest/internal/domains/container_of_impl.h" #include "./fuzztest/internal/domains/domain_base.h" #include "./fuzztest/internal/io.h" -#include "./fuzztest/internal/coverage.h" +#include "./fuzztest/internal/logging.h" ABSL_DECLARE_FLAG(std::string, llvm_fuzzer_wrapper_dict_file); ABSL_DECLARE_FLAG(std::string, llvm_fuzzer_wrapper_corpus_dir); @@ -125,6 +130,76 @@ class InplaceVector { std::size_t size_; }; +namespace { + +using ::fuzztest::domain_implementor::MutationMetadata; + +// Manager that controls the access of mutation metadata for +// LLVMFuzzerMutate/LLVMFuzzerCustomMutator. +// +// The metadata is supposed to be active only when FuzzTest is calling +// LLVMFuzzerCustomMutator. +// +// If the metadata is active, `Acquire` would return a non-null metadata +// pointer, and the caller should call `Release` after the metadata usage. +// If the metadata is inactive, `Acquire` would abort - this would only happen +// when the fuzzer calls `LLVMFuzzerMutate` outside of +// `LLVMFuzzerCustomMutator`. +class LLVMFuzzerMutateMetadataManager { + public: + void Activate(MutationMetadata mutation_metadata) { + absl::MutexLock lock(&mu_); + FUZZTEST_INTERNAL_CHECK( + !mutation_metadata_.has_value(), + "MutationMetadata is already active before calling Activate()!"); + FUZZTEST_INTERNAL_CHECK( + acquire_count_ == 0, + "MutationMetadata still has readers before being calling Activate()!"); + mutation_metadata_ = std::move(mutation_metadata); + } + + void Deactivate() { + absl::MutexLock lock(&mu_); + FUZZTEST_INTERNAL_CHECK( + mutation_metadata_.has_value(), + "MutationMetadata is not active before calling Deactivate()!"); + FUZZTEST_INTERNAL_CHECK( + acquire_count_ == 0, + "MutationMetadata still has readers before calling Deactivate()!"); + mutation_metadata_ = std::nullopt; + } + + const MutationMetadata& Acquire() { + absl::MutexLock lock(&mu_); + FUZZTEST_INTERNAL_CHECK_PRECONDITION( + mutation_metadata_.has_value(), + "Cannot acquire unavailable mutation metadata, likely due to the " + "fuzzer calling LLVMFuzzerMutate() outside of " + "LLVMFuzzerCustomMutator() invocation, which is not allowed."); + ++acquire_count_; + return *mutation_metadata_; + } + + void Release() { + absl::MutexLock lock(&mu_); + FUZZTEST_INTERNAL_CHECK( + mutation_metadata_.has_value(), + "MutationMetadata is not active before calling Release()!"); + FUZZTEST_INTERNAL_CHECK( + acquire_count_ > 0, + "MutationMetadata has no readers before calling Release()!"); + --acquire_count_; + } + + private: + absl::Mutex mu_; + size_t acquire_count_ ABSL_GUARDED_BY(mu_) = 0; + std::optional mutation_metadata_ ABSL_GUARDED_BY(mu_); +}; + +absl::NoDestructor mutation_metadata_manager; +} // namespace + #ifdef FUZZTEST_USE_CENTIPEDE extern "C" size_t CentipedeLLVMFuzzerMutateCallback(uint8_t* data, size_t size, size_t max_size) { @@ -137,12 +212,9 @@ extern "C" size_t LLVMFuzzerMutate(uint8_t* data, size_t size, domain.WithMaxSize(max_size); absl::BitGen bitgen; InplaceVector val(data, size); - fuzztest::domain_implementor::MutationMetadata metadata; - if (auto* coverage = fuzztest::internal::GetExecutionCoverage(); - coverage != nullptr) { - metadata.cmp_tables = &coverage->GetTablesOfRecentCompares(); - } + const auto& metadata = mutation_metadata_manager->Acquire(); domain.Mutate(val, bitgen, metadata, false); + mutation_metadata_manager->Release(); return val.size(); } @@ -157,13 +229,14 @@ class ArbitraryByteVector ArbitraryByteVector() { WithMaxSize(kByteArrayMaxLen); } void Mutate(corpus_type& val, absl::BitGenRef prng, - const fuzztest::domain_implementor::MutationMetadata& metadata, - bool only_shrink) { + const MutationMetadata& metadata, bool only_shrink) { if (LLVMFuzzerCustomMutator) { const size_t size = val.size(); const size_t max_size = only_shrink ? size : kByteArrayMaxLen; val.resize(max_size); + mutation_metadata_manager->Activate(metadata); val.resize(LLVMFuzzerCustomMutator(val.data(), size, max_size, prng())); + mutation_metadata_manager->Deactivate(); } else { Base::Mutate(val, prng, metadata, only_shrink); }