This is an archive of the discontinued LLVM Phabricator instance.

Summary: Add close_fd_mask functionality to AFL driver.
ClosedPublic

Authored by metzman on Apr 5 2019, 12:31 PM.

Details

Summary

Add support for env var AFL_DRIVER_CLOSE_FD_MASK which behaves
the same as libFuzzer's -close_fd_mask=1.

Also add tests.

Event Timeline

metzman created this revision.Apr 5 2019, 12:31 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptApr 5 2019, 12:31 PM
Herald added subscribers: llvm-commits, Restricted Project. · View Herald Transcript
metzman updated this revision to Diff 193942.Apr 5 2019, 12:37 PM
  • organize and improve comments

Updating D60334: Summary:

Add close_fd_mask functionality to AFL driver.

Please take a look.
This should help with some really badly behaved fuzzers in OSS-Fuzz that log so much that the disk fills up (because ClusterFuzz uses AFL_DRIVER_DUPLICATE_STDERR_FILENAME).

ormris removed a subscriber: ormris.Apr 5 2019, 12:43 PM
morehouse added inline comments.Apr 5 2019, 4:46 PM
compiler-rt/lib/fuzzer/afl/afl_driver.cpp
113

Do we need to set visibility here?

118

static

151

static - here and other private functions.

175

Nit: Can we remove some of these blank lines?

180

If you want to submit a follow-up patch, SGTM. But lets leave this TODO out since it's only style-related.

198

Nit: can we remove these blank lines?

compiler-rt/test/fuzzer/AFLDriverCloseFdMaskTest.cpp
5 ↗(On Diff #193942)

Let's move this comment closer to the functions it refers to.

16 ↗(On Diff #193942)

Do we really need this? How about just making it return 0 always?

23 ↗(On Diff #193942)

Do we need this printf?

29 ↗(On Diff #193942)

fflush(stdout)

36 ↗(On Diff #193942)

Maybe just return Data[Size]? Simpler and should still crash.

compiler-rt/test/fuzzer/afl-driver-close-fd-mask.test
5

Can we remove this line?

9

Please remove

20

Nit: Some inconsistent formatting. Can we keep the CHECKs right after the associated RUNs? Rather than sometimes after and sometimes before.

37

Can we combine this into check2?

43

I think this test will fail if the file doesn't exist. Better make it rm -f

metzman updated this revision to Diff 194163.Apr 8 2019, 10:13 AM
metzman marked 18 inline comments as done.
  • first round of fixes
  • some combining
  • improve tests
  • undo accidenal change
  • more reuse
  • fmt
  • fmt

Updating D60334: Summary:

Add close_fd_mask functionality to AFL driver.

metzman updated this revision to Diff 194167.Apr 8 2019, 10:34 AM
metzman marked an inline comment as done.
  • improve tests

Updating D60334: Summary:

Add close_fd_mask functionality to AFL driver.

morehouse accepted this revision.Apr 8 2019, 10:46 AM
morehouse added inline comments.
compiler-rt/test/fuzzer/afl-driver-close-fd-mask.test
5

I don't understand this comment.

This revision is now accepted and ready to land.Apr 8 2019, 10:46 AM
metzman updated this revision to Diff 194171.Apr 8 2019, 10:49 AM
  • improve comment

Updating D60334: Summary:

Add close_fd_mask functionality to AFL driver.

metzman marked 2 inline comments as done.Apr 8 2019, 10:49 AM
metzman added inline comments.
compiler-rt/test/fuzzer/afl-driver-close-fd-mask.test
5

Sorry, left a bunch of debugging code in here.

5

Ah well it was a pretty bad comment. Improved it.

37

I split up almost every message so that I did as much sharing and as little repetition as possible.
WDYT?

43

Good point. Fixed.

This revision was automatically updated to reflect the committed changes.
metzman marked an inline comment as done.