From aaa88b8e28787354f13fa98b8c16d75f937e2833 Mon Sep 17 00:00:00 2001
From: Eric Severance <esev@esev.com>
Date: Wed, 8 Jan 2025 17:06:42 -0800
Subject: [PATCH] Use loopCanSleep instead of a lock in Router

---
 .clusterfuzzlite/router_fuzzer.cpp | 47 ++++++++++++++++++++++++++----
 src/mesh/Router.cpp                |  4 ---
 src/mesh/Router.h                  | 10 -------
 3 files changed, 41 insertions(+), 20 deletions(-)

diff --git a/.clusterfuzzlite/router_fuzzer.cpp b/.clusterfuzzlite/router_fuzzer.cpp
index b28211a2f2..94711be759 100644
--- a/.clusterfuzzlite/router_fuzzer.cpp
+++ b/.clusterfuzzlite/router_fuzzer.cpp
@@ -1,4 +1,5 @@
 // Fuzzer implementation that sends MeshPackets to Router::enqueueReceivedMessage.
+#include <condition_variable>
 #include <mutex>
 #include <pb_decode.h>
 #include <string>
@@ -15,7 +16,30 @@
 namespace
 {
 constexpr uint32_t nodeId = 0x12345678;
+// Set to true when lateInitVariant finishes. Used to ensure lateInitVariant was called during startup.
 bool hasBeenConfigured = false;
+
+// These are used to block the Arduino loop() function until a fuzzer input is ready. This is
+// an optimization that prevents a sleep from happening before the loop is run. The Arduino loop
+// function calls loopCanSleep() before sleeping. loopCanSleep is implemented here in the fuzzer
+// and blocks until startLoop() is called to signal for the loop to run.
+bool fuzzerRunning = false; // Set to true once LLVMFuzzerTestOneInput has started running.
+bool loopCanRun = true;     // The main Arduino loop() can run when this is true.
+bool loopIsWaiting = false; // The main Arduino loop() is waiting to be signaled to run.
+std::mutex loopLock;
+std::condition_variable loopCV;
+
+// Start the loop for one test case and wait till the loop has completed. This ensures fuzz
+// test cases do not overlap with one another. This helps the fuzzer attribute a crash to the
+// single, currently running, test case.
+void runLoopOnce()
+{
+    std::unique_lock<std::mutex> lck(loopLock);
+    fuzzerRunning = true;
+    loopCanRun = true;
+    loopCV.notify_one();
+    loopCV.wait(lck, [] { return !loopCanRun && loopIsWaiting; });
+}
 } // namespace
 
 // Called just prior to starting Meshtastic. Allows for setting config values before startup.
@@ -71,6 +95,22 @@ void lateInitVariant()
     hasBeenConfigured = true;
 }
 
+// Called in the main Arduino loop function to determine if the loop can delay/sleep before running again.
+// We use this as a way to block the loop from sleeping and to start the loop function immediately when a
+// fuzzer input is ready.
+bool loopCanSleep()
+{
+    std::unique_lock<std::mutex> lck(loopLock);
+    loopIsWaiting = true;
+    loopCV.notify_one();
+    loopCV.wait(lck, [] { return loopCanRun; });
+    loopIsWaiting = false;
+    if (!fuzzerRunning)
+        return true;    // The loop can sleep before the fuzzer starts.
+    loopCanRun = false; // Only run the loop once before waiting again.
+    return false;
+}
+
 extern "C" {
 int portduino_main(int argc, char **argv); // Renamed "main" function from Meshtastic binary.
 
@@ -134,13 +174,8 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t length)
     if (p.pki_encrypted && config.security.admin_key_count)
         memcpy(&p.public_key, &config.security.admin_key[0], sizeof(p.public_key));
 
-    // Ideally only one packet, the one generated by the fuzzer, is being processed by the firmware at
-    // a time. We acquire a lock here, and the router unlocks it after it has processed all queued packets.
-    // Grabbing the lock again, below, should block until the queue has been emptied.
-    router->inProgressLock.lock();
     router->enqueueReceivedMessage(packetPool.allocCopy(p));
-
-    const std::lock_guard<std::mutex> lck(router->inProgressLock);
+    runLoopOnce();
     return 0; // Accept: The input may be added to the corpus.
 }
 }
\ No newline at end of file
diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp
index 4233a8bdc4..f55e7cc5a4 100644
--- a/src/mesh/Router.cpp
+++ b/src/mesh/Router.cpp
@@ -70,10 +70,6 @@ int32_t Router::runOnce()
         perhapsHandleReceived(mp);
     }
 
-#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
-    inProgressLock.unlock();
-    return 5;
-#endif
     // LOG_DEBUG("Sleep forever!");
     return INT32_MAX; // Wait a long time - until we get woken for the message queue
 }
diff --git a/src/mesh/Router.h b/src/mesh/Router.h
index fa7119fe87..0fe2bc5510 100644
--- a/src/mesh/Router.h
+++ b/src/mesh/Router.h
@@ -8,10 +8,6 @@
 #include "RadioInterface.h"
 #include "concurrency/OSThread.h"
 
-#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
-#include <mutex>
-#endif
-
 /**
  * A mesh aware router that supports multiple interfaces.
  */
@@ -90,12 +86,6 @@ class Router : protected concurrency::OSThread
         before us */
     uint32_t rxDupe = 0, txRelayCanceled = 0;
 
-#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
-    // Used by router_fuzzer.cpp to detect when the Router has finished processing a packet.
-    // See LLVMFuzzerTestOneInput in router_fuzzer.cpp & Router::runOnce for how this is used.
-    std::mutex inProgressLock;
-#endif
-
   protected:
     friend class RoutingModule;