Skip to content

Commit

Permalink
Further limit memory growth during fuzzing
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
alexcrichton committed Jan 15, 2025
1 parent 48f4621 commit 46f5c04
Showing 1 changed file with 31 additions and 17 deletions.
48 changes: 31 additions & 17 deletions crates/fuzzing/src/oracles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -115,7 +129,7 @@ impl StoreLimits {
Ok(_) => true,
Err(_) => {
self.0.oom.store(true, SeqCst);
log::debug!("OOM hit");
log::debug!("-> OOM hit");
false
}
}
Expand Down

0 comments on commit 46f5c04

Please sign in to comment.