-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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.
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
build: add ClusterFuzzLite setup #4286
base: v1.x
Are you sure you want to change the base?
Conversation
The goal is to enable continuous fuzzing of libuv. There is not a lot of parsing or complex data handling in libuv so I think integrating with OSS-Fuzz is too much. However, ClusterFuzzLite can be used to run fuzzers for a small amount of seconds for each PR to ensure nothing breaks. This commit adds ClusterFuzzLite setup as well as a fuzzer targeting various operations. The fuzzer can be extended, e.g. it would be nice to have more complex FS fuzzing, but I thought I would keep it as is for now and see if you're interested in having fuzzing. Signed-off-by: David Korczynski <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some comments. Thanks for the pull request!
|
||
COPY . $SRC/libuv | ||
COPY .clusterfuzzlite/build.sh $SRC/build.sh | ||
WORKDIR $SRC/libuv |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is unused, right?
make | ||
make install |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make | |
make install | |
make -j2 install |
Should be a little faster.
|
||
static void dummy_cb(uv_fs_t *req) { (void)req; } | ||
|
||
void test_idna(const uint8_t *data, size_t size) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
General style observation: star leans left, e.g. uint8_t*
instead of uint8_t *
static void dummy_cb(uv_fs_t *req) { (void)req; } | ||
|
||
void test_idna(const uint8_t *data, size_t size) { | ||
char *new_str = (char *)malloc(size + 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cast isn't necessary in C (ditto on line 97, 120)
r = uv_fs_open(NULL, &open_req1, "test_file", UV_FS_O_WRONLY | UV_FS_O_CREAT, | ||
S_IWUSR | S_IRUSR, NULL); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Either check the return value or remove the r =
to make it more obvious that the return value isn't checked on purpose.
uv_fs_req_cleanup(&write_req); | ||
|
||
/* Close after writing */ | ||
r = uv_fs_close(NULL, &close_req, open_req1.result, NULL); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r = uv_fs_close(NULL, &close_req, open_req1.result, NULL); | |
r = uv_fs_close(NULL, &close_req, open_req1.result, NULL); | |
uv_fs_req_cleanup(&close_req); |
At the moment it's not strictly necessary for fs_close requests but it's good form. Ditto on line 101.
if (size == 0) { | ||
return 0; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nittiest of nits but idiomatic libuv drops braces for single-line if statements.
if (size == 0) { | |
return 0; | |
} | |
if (size == 0) | |
return 0; |
/* Allocate a null-terminated string that can be used in various fuzz | ||
* operations. | ||
*/ | ||
char *new_str = (char *)malloc(size + 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test_idna makes a copy too. I don't know how big the fuzzer inputs are but maybe it's better to do it only once. Less likely to hit out-of-memory conditions.
id: run | ||
uses: google/clusterfuzzlite/actions/run_fuzzers@v1 | ||
with: | ||
github-token: ${{ secrets.GITHUB_TOKEN }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Forgot to ask: what is this needed for?
The goal is to enable continuous fuzzing of libuv. There is not a lot of parsing or complex data handling in libuv so I think integrating with OSS-Fuzz is too much. However, ClusterFuzzLite can be used to run fuzzers for a small amount of seconds (set to 100 for now) for each PR to ensure nothing breaks. This commit adds ClusterFuzzLite setup as well as a fuzzer targeting various operations. The fuzzer can be extended, e.g. it would be nice to have more complex FS fuzzing, but I thought I would keep it as is for now and see if you're interested in having fuzzing.