[compiler-rt] Work around a C language bug in libFuzzer (PR #96775)

David Benjamin via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 26 09:29:26 PDT 2024


https://github.com/davidben updated https://github.com/llvm/llvm-project/pull/96775

>From 738b83a2287cf9a891703627ab26e2f7b6b84e4f Mon Sep 17 00:00:00 2001
From: David Benjamin <davidben at google.com>
Date: Wed, 26 Jun 2024 10:27:06 -0400
Subject: [PATCH 1/2] Work around a C language bug in libFuzzer

Due to a C language bug, it is UB to call memcmp(NULL, NULL, 0),
memcpy(NULL, NULL, 0), etc. This is a problem because (NULL, 0) is the
natural representation of the empty slice and extremely common in real
world code. As a result, all C code, and C++ code which calls into C
functions, must carefully guard all calls to memcpy.

This is horrible and should be fixed in C (see #49459), but in the
meantime, avoid tripping UBSan in libFuzzer.

Fixes #96772
---
 compiler-rt/lib/fuzzer/FuzzerDictionary.h | 4 +++-
 compiler-rt/lib/fuzzer/FuzzerLoop.cpp     | 7 ++++++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/compiler-rt/lib/fuzzer/FuzzerDictionary.h b/compiler-rt/lib/fuzzer/FuzzerDictionary.h
index 48f063c7ee4e6..b447f3de6a591 100644
--- a/compiler-rt/lib/fuzzer/FuzzerDictionary.h
+++ b/compiler-rt/lib/fuzzer/FuzzerDictionary.h
@@ -29,7 +29,9 @@ template <size_t kMaxSizeT> class FixedWord {
     static_assert(kMaxSizeT <= std::numeric_limits<uint8_t>::max(),
                   "FixedWord::kMaxSizeT cannot fit in a uint8_t.");
     assert(S <= kMaxSize);
-    memcpy(Data, B, S);
+    // memcpy cannot be passed empty slices when the pointers are null.
+    if (S)
+      memcpy(Data, B, S);
     Size = static_cast<uint8_t>(S);
   }
 
diff --git a/compiler-rt/lib/fuzzer/FuzzerLoop.cpp b/compiler-rt/lib/fuzzer/FuzzerLoop.cpp
index 935dd2342e18f..8e229bd0ef322 100644
--- a/compiler-rt/lib/fuzzer/FuzzerLoop.cpp
+++ b/compiler-rt/lib/fuzzer/FuzzerLoop.cpp
@@ -579,6 +579,9 @@ void Fuzzer::CrashOnOverwrittenData() {
 // Compare two arrays, but not all bytes if the arrays are large.
 static bool LooseMemeq(const uint8_t *A, const uint8_t *B, size_t Size) {
   const size_t Limit = 64;
+  // memcmp cannot be passed empty slices when the pointers are null.
+  if (!Size)
+    return 1;
   if (Size <= 64)
     return !memcmp(A, B, Size);
   // Compare first and last Limit/2 bytes.
@@ -596,7 +599,9 @@ ATTRIBUTE_NOINLINE bool Fuzzer::ExecuteCallback(const uint8_t *Data,
   // We copy the contents of Unit into a separate heap buffer
   // so that we reliably find buffer overflows in it.
   uint8_t *DataCopy = new uint8_t[Size];
-  memcpy(DataCopy, Data, Size);
+  // memcpy cannot be passed empty slices when the pointers are null.
+  if (Size)
+    memcpy(DataCopy, Data, Size);
   if (EF->__msan_unpoison)
     EF->__msan_unpoison(DataCopy, Size);
   if (EF->__msan_unpoison_param)

>From 7df36372172237cea00402c160b43309bbd30ff1 Mon Sep 17 00:00:00 2001
From: David Benjamin <davidben at google.com>
Date: Wed, 26 Jun 2024 12:29:11 -0400
Subject: [PATCH 2/2] Use true instead of 1

---
 compiler-rt/lib/fuzzer/FuzzerLoop.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/compiler-rt/lib/fuzzer/FuzzerLoop.cpp b/compiler-rt/lib/fuzzer/FuzzerLoop.cpp
index 8e229bd0ef322..e97dbe097a387 100644
--- a/compiler-rt/lib/fuzzer/FuzzerLoop.cpp
+++ b/compiler-rt/lib/fuzzer/FuzzerLoop.cpp
@@ -581,7 +581,7 @@ static bool LooseMemeq(const uint8_t *A, const uint8_t *B, size_t Size) {
   const size_t Limit = 64;
   // memcmp cannot be passed empty slices when the pointers are null.
   if (!Size)
-    return 1;
+    return true;
   if (Size <= 64)
     return !memcmp(A, B, Size);
   // Compare first and last Limit/2 bytes.



More information about the llvm-commits mailing list