smitebot: tmux integration + strategy distribution for start#139
Conversation
Ashish-Kumar-Dash
force-pushed
the
tmux-start
branch
from
b798b1f to
be57059
Compare
Ashish-Kumar-Dash
changed the title
Ashish-Kumar-Dash
force-pushed
the
tmux-start
branch
from
be57059 to
49f7cbb
Compare
|
Tested locally with 2 runners on lnd/encrypted_bytes: 
Build + Nyx sharedir setup + tmux session launch logs 
Runner 0 ([fast], primary -M), ~440 execs/sec, 119 corpus, trim disabled 
Runner 1 ([explore], secondary -S), ~449 execs/sec, 108 corpus, 7 imported Strategy distribution, window naming, and Nyx mode all working as expected. Killed the session with tmux kill-session -t , stop command will automate this. |
| Some(if free_mb > 32_000 { | ||
| 500 | ||
| } else if free_mb > 8_000 { | ||
| 250 | ||
| } else { | ||
| 50 | ||
| }) |
There was a problem hiding this comment.
I know 50–500 MB is recommended, but is there any reason behind a threshold like >8_000/32_000?
There was a problem hiding this comment.
They're arbitrary heuristics, if there's a specific threshold you want me to check for then do inform
NishantBansal2003
left a comment
NishantBansal2003
left a comment
There was a problem hiding this comment.
- I think we need a longer timeout for verification. When I first tried running smitebot, it reported an error, but when I checked the tmux windows, both fuzzers were running normally. I haven’t seen this issue again since then, though:
INFO [smitebot::commands::start] Nyx sharedir ready at /tmp/smitebot/smite-nyx
INFO [smitebot::commands::start] starting 2 runners in tmux session 'lnd-encrypted_bytes-1783323599'
INFO [smitebot::commands::start] verifying runners started
ERROR [smitebot::commands::start] runner 0 did not produce fuzzer_stats within timeout
ERROR [smitebot::commands::start] runner 1 did not produce fuzzer_stats within timeout
ERROR [smitebot::commands::start] not all runners produced fuzzer_stats within timeout
INFO [smitebot::commands::start] inspect tmux session 'lnd-encrypted_bytes-1783323599' for error output, then: tmux kill-session -t lnd-encrypted_bytes-1783323599
- Also, when startup succeeds, tmux takes over the terminal (with last runner). Does it make sense to keep printing the smitebot logs normally and run tmux in the background instead? Then, once something like
attaching to tmux session 'lnd-encrypted_bytes-1783324069'is printed, we could inspect the session ourselves if needed
Ashish-Kumar-Dash
force-pushed
the
tmux-start
branch
from
c370d0c to
711157e
Compare
The old The fix stops timing the outcome. It now decides on process state: a fresh Also closed an adjacent latent bug: on a reused |
|
IIUC, if the From what I understood from your reply in #119 (comment), Leaving that case aside, we're only left with starting a fresh campaign, either as a cold or warm start, which I believe should be covered by simply increasing the timeout to 60s (or appropriate). WDYT? |
Yes, that should be covered in
no, It was a plain fresh start.
I'd push back on a fixed 60s. Liveness doesn't depend on guessing how long calibration takes. It fails fast when a runner's window dies, and waits while it's alive, so a healthy-but-slow runner is never reported as failed, regardless of corpus size or cold/warm machine. A fixed 60s is a bet on calibration finishing in time, my measured run was already ~45s, so 60s has ~15s of margin and maybe loses it on a colder machine or larger corpus. The 15-min ceiling is only a checkpost for a truly hung runner, not the success criteria. That's the reason why I made the fix, 60s can wrongly fail a working campaign, liveness only "fails" a runner that has actually died or hung. If the liveness check ever feels like too much, we can always fall back to 60s/fixed timeout, it's a small change. But since this PR's real weight is the tmux integration + strategy distribution, I'd happy to revisit the timeout approach in a follow-up. When people start using |
Launch parallel afl-fuzz runners in a tmux session (one window per runner) with a deterministic per-runner strategy distribution. Verify startup by polling fuzzer_stats: fail fast when a runner's window dies, and bound an alive-but-hung runner with a generous ceiling rather than a fixed short timeout. Reject a pre-populated output_dir (start does not resume) and tmux session names containing ':', '.', or '#'.
Ashish-Kumar-Dash
force-pushed
the
tmux-start
branch
from
6b970c3 to
a4550b4
Compare
morehouse
left a comment
morehouse
left a comment
There was a problem hiding this comment.
This is a nice improvement. I think we still want to tweak this a bit, but let's do it in small follow-up PRs instead of review-looping on this big PR.
| // AFL++ docs: the strategy knobs below apply to "the other secondaries", | ||
| // not the primary. See fuzzing_in_depth.md §c "Using multiple cores". | ||
| if runner_id > 0 { |
There was a problem hiding this comment.
What about the -P, -L, and -Z options described in #138?
| let schedule = POWER_SCHEDULES[runner_id as usize % POWER_SCHEDULES.len()]; | ||
| flags.extend(["-p".to_string(), schedule.to_string()]); |
There was a problem hiding this comment.
IIUC, the AFL doc recommends only setting -p for secondaries, not the master.
| fn runner_strategy_primary_gets_final_sync() { | ||
| let (flags, envs) = runner_strategy(0, 8, None); | ||
| assert!(envs.iter().any(|(k, v)| k == "AFL_FINAL_SYNC" && v == "1")); | ||
| assert!(flags.contains(&"-p".to_string())); |
There was a problem hiding this comment.
The second assert doesn't look related to the test name.
| assert!(flags.contains(&"-p".to_string())); |
| /// Custom tmux session name; defaults to the campaign ID when absent. | ||
| pub tmux_session: Option<String>, |
There was a problem hiding this comment.
sample-campaign.toml needs to be updated with this option.
| @@ -1,65 +1,70 @@ | |||
| # smitebot | |||
There was a problem hiding this comment.
Part of the reason review cycles take so long is that the PRs themselves are large and may require multiple hours of the reviewer's time to properly review. I know I won't even think about sitting down to fully review a complex 1k line PR unless I have a couple hours of time available for it. If many changes are needed, that time investment multiplies on each round of review (code review is quadratic).
I think it's generally more efficient to split any unrelated changes into their own small PRs. In fact, I would merge the unrelated README changes immediately if they were in a separate PR. But since they're bundled in here it takes much longer for those changes to get in.
| // -a binary on ~70% of secondary runners. | ||
| let secondary_count = runner_count.saturating_sub(1) as usize; | ||
| let binary_count = (secondary_count * 7).div_ceil(10); | ||
| if ((runner_id - 1) as usize) < binary_count { | ||
| flags.extend(["-a".to_string(), "binary".to_string()]); | ||
| } | ||
|
|
||
| for (child, runner) in children.iter_mut().zip(runners.iter()) { | ||
| match child.try_wait() { | ||
| Ok(Some(_)) => { | ||
| log::error!( | ||
| "runner {} (pid {}) died during startup", | ||
| runner.id, | ||
| runner.pid | ||
| ); | ||
| return false; | ||
| } | ||
| Ok(None) => {} | ||
| Err(e) => { | ||
| log::error!( | ||
| "failed to check runner {} (pid {}): {e}", | ||
| runner.id, | ||
| runner.pid | ||
| ); | ||
| return false; | ||
| } | ||
| } | ||
| // AFL_DISABLE_TRIM on ~60% of secondary runners. | ||
| let trim_count = (secondary_count * 6).div_ceil(10); | ||
| if ((runner_id - 1) as usize) < trim_count { | ||
| envs.push(("AFL_DISABLE_TRIM".to_string(), "1".to_string())); | ||
| } |
There was a problem hiding this comment.
I think we want to shuffle the flag assignments. Currently the first 60% of secondaries disable trim and the first 70% set -a binary. That means there are very few runners that have only one of those options and not the other -- pretty much all runners have both or neither.
Something like this might work well:
- create a list of option "modifiers" that are assigned some probability of being applied between 0 and 100.
- for each runner, for each modifier, compute
X = hash(runner_id || modifier_name) % 100and apply the modifier to that runner ifX < modifier_probability.
This would still be deterministic and reproducible while giving more variety to the flag combinations.
Ashish-Kumar-Dash
changed the title
3 participants
Summary
afl-fuzzrunner in its own tmux window instead of invisible background processes, users see individual AFL++ TUIs, can attach/detach viaCtrl-b d, and reconnect over SSHMultipleCoresmode) for campaign diversity instead of running N identical fuzzersfuzzer_statsolder than launch (leftover in a reusedoutput_dir) is ignored, so a stale PID is never recorded as a live runner.remain-on-exit) instead of killing itStrategy distribution
Round-robin power schedules;
-a binaryon ~70% of secondaries;AFL_DISABLE_TRIMon ~60%;AFL_IMPORT_FIRSTwhen < 16 runners;AFL_FINAL_SYNCon the primary; testcache auto-sized from RAM. IR scenarios auto-inject the custom-mutator env vars. Optionaltmux_sessionconfig key, validated onCampaignConfig.tmux session layout
tmux session: "lnd-encrypted_bytes-1749465600" (or custom name from campaign.toml)
├── window 0: "runner-0" → afl-fuzz -Y -M 0 ...
├── window 1: "runner-1" → afl-fuzz -Y -S 1 ...
├── window 2: "runner-2" → afl-fuzz -Y -S 2 ...
└── ...
Each runner command is built as a shell string with
execso afl-fuzz replaces the shell process directly. Environment variables are prepended asKEY='val'assignments, and paths are single-quote escaped.Features
randdependency)fresh
fuzzer_stats→ runner starteddead tmux window (
pane_dead) → runner failed to launch, reported at onceneither yet → still calibrating; keep polling to a generous 15-min ceiling that only bounds an alive-but-hung runner
remain-on-exitpreserves error output for debugging;,user cleans up manually or via futurestopcommandswitch-clientwhen$TMUXis set,attach-sessionotherwisestd::process::CommandRef. #138 #70