From 46f5c040d83a977aa10e0871183298a93e9d4963 Mon Sep 17 00:00:00 2001
From: Alex Crichton <alex@alexcrichton.com>
Date: Wed, 15 Jan 2025 13:23:16 -0800
Subject: [PATCH] Further limit memory growth during fuzzing

Previously a limit was added which prevented more than N growths over
time but this wasn't sufficient to prevent a test case from continuously
growing memory by just enough that it never went over N but the byte
sizes in question were big enough that the fuzz test case timed out on
OSS-Fuzz.

This commit changes the check to limit "bytes moved" instead of the
quantity of growths over time. This is a coarse approximation of what's
happening but should hopefully still allow interesting behavior while
additionally ensuring we don't spent the whole time moving around
gigabytes of data.
---
 crates/fuzzing/src/oracles.rs | 48 ++++++++++++++++++++++-------------
 1 file changed, 31 insertions(+), 17 deletions(-)

diff --git a/crates/fuzzing/src/oracles.rs b/crates/fuzzing/src/oracles.rs
index cc77617bb0de..51ecb3453b21 100644
--- a/crates/fuzzing/src/oracles.rs
+++ b/crates/fuzzing/src/oracles.rs
@@ -70,43 +70,57 @@ pub struct StoreLimits(Arc<LimitsState>);
 struct LimitsState {
     /// Remaining memory, in bytes, left to allocate
     remaining_memory: AtomicUsize,
-    /// Remaining times memories/tables can be grown
-    remaining_growths: AtomicUsize,
+    /// Remaining amount of memory that's allowed to be copied via a growth.
+    remaining_copy_allowance: AtomicUsize,
     /// Whether or not an allocation request has been denied
     oom: AtomicBool,
 }
 
+/// Allow up to 1G which is well below the 2G limit on OSS-Fuzz and should allow
+/// most interesting behavior.
+const MAX_MEMORY: usize = 1 << 30;
+
+/// Allow up to 4G of bytes to be copied (conservatively) which should enable
+/// growth up to `MAX_MEMORY` or at least up to a relatively large amount.
+const MAX_MEMORY_MOVED: usize = 4 << 30;
+
 impl StoreLimits {
     /// Creates the default set of limits for all fuzzing stores.
     pub fn new() -> StoreLimits {
         StoreLimits(Arc::new(LimitsState {
-            // Limits tables/memories within a store to at most 1gb for now to
-            // exercise some larger address but not overflow various limits.
-            remaining_memory: AtomicUsize::new(1 << 30),
-            // Also limit the number of times a memory or table may be grown.
-            // Otherwise infinite growths can exhibit quadratic behavior. For
-            // example Wasmtime could be configured with dynamic memories and no
-            // guard regions to grow into, meaning each memory growth could be a
-            // `memcpy`. As more data is added over time growths get more and
-            // more expensive meaning that fuel may not be effective at limiting
-            // execution time.
-            remaining_growths: AtomicUsize::new(1000),
+            remaining_memory: AtomicUsize::new(MAX_MEMORY),
+            remaining_copy_allowance: AtomicUsize::new(MAX_MEMORY_MOVED),
             oom: AtomicBool::new(false),
         }))
     }
 
     fn alloc(&mut self, amt: usize) -> bool {
         log::trace!("alloc {amt:#x} bytes");
+
+        // Assume that on each allocation of memory that all previous
+        // allocations of memory are moved. This is pretty coarse but is used to
+        // help prevent against fuzz test cases that just move tons of bytes
+        // around continuously. This assumes that all previous memory was
+        // allocated in a single linear memory and growing by `amt` will require
+        // moving all the bytes to a new location. This isn't actually required
+        // all the time nor does it accurately reflect what happens all the
+        // time, but it's a coarse approximation that should be "good enough"
+        // for allowing interesting fuzz behaviors to happen while not timing
+        // out just copying bytes around.
+        let prev_size = MAX_MEMORY - self.0.remaining_memory.load(SeqCst);
         if self
             .0
-            .remaining_growths
-            .fetch_update(SeqCst, SeqCst, |remaining| remaining.checked_sub(1))
+            .remaining_copy_allowance
+            .fetch_update(SeqCst, SeqCst, |remaining| remaining.checked_sub(prev_size))
             .is_err()
         {
             self.0.oom.store(true, SeqCst);
-            log::debug!("too many growths, rejecting allocation");
+            log::debug!("-> too many bytes moved, rejecting allocation");
             return false;
         }
+
+        // If we're allowed to move the bytes, then also check if we're allowed
+        // to actually have this much residence at once.
         match self
             .0
             .remaining_memory
@@ -115,7 +129,7 @@ impl StoreLimits {
             Ok(_) => true,
             Err(_) => {
                 self.0.oom.store(true, SeqCst);
-                log::debug!("OOM hit");
+                log::debug!("-> OOM hit");
                 false
             }
         }