Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shut down if all initial inputs crash #276

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Shut down if all initial inputs crash #276

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2cfeb82
Shut down if all initial inputs crash
ThorbjoernSchulz Oct 9, 2019
240e64a
Break at the right spot
ThorbjoernSchulz Mar 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion go-fuzz/worker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,19 @@ func (w *Worker) loop() {
w.triageInput(input)
for {
x := atomic.LoadUint32(&w.hub.initialTriage)
if x == 0 || atomic.CompareAndSwapUint32(&w.hub.initialTriage, x, x-1) {
if x == 0 {
break
}
if atomic.CompareAndSwapUint32(&w.hub.initialTriage, x, x-1) {
// After the last initial input was processed, check if anything was actually added to the corpus.
// If the corpus is empty at this point, we have nothing to feed to our workers.
if x == 1 {
// Give the hub time to store the initial inputs
time.Sleep(100 * time.Millisecond)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I am reading this correctly, we can fail spuriously if hub is delayed a bit (overloaded machine, virtualization, etc).
Also we don't generally ask user to add anything to corpus, fine to start with an empty one. Also if just part of inputs crash, we will continue working, this radical difference in behavior between part crash and all crash feels wrong. Also the corpus may contain, say, just 1 input and it happened to crash after code update. Then fuzzer won't start.
I agree the current behavior is bad. But I think a more useful way to fix this would be to let workers work even with empty corpus. They could generate random completely random inputs, it does not seem we can do anything better in absence of any seeds.
Then if just part of inputs crash (and we were unlucky with corpus, I think I saw cases where the initial empty input crash, maybe due to a bug in the fuzz function itself), the fuzzer will gracefully recover. Otherwise, the crash count will go up infinitely, which is, well, the expected behavior in such case.

It may be a bit tricky to preserve this part then:

		if len(ro.corpus) == 0 {
			// Some other worker triages corpus inputs.
			time.Sleep(100 * time.Millisecond)
			continue
		}

because we want to wait for hub for initial inputs, but not if there are really 0 inputs. I think this bit has to do with corpus inflation problem mentioned above:

			// Other workers are still triaging initial inputs.
			// Wait until they finish, otherwise we can generate
			// as if new interesting inputs that are not actually new
			// and thus unnecessary inflate corpus on every run.
			time.Sleep(100 * time.Millisecond)

But I hope there is some way to preserve it.

if ro := w.hub.ro.Load().(*ROData); len(ro.corpus) == 0 {
log.Fatal("all initial inputs crashed. provide at least one non-crashing input.")
}
}
break
}
}
Expand Down