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.

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

Create interface for OSS-Fuzz users. #93

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

jonathanmetzman
Copy link
Collaborator

@evverx
Copy link
Contributor

evverx commented Mar 31, 2022

@jonathanmetzman I wonder if the idea is to make it possible to replace CIFuzz with CFLite with the "oss-fuzz" setting? If not, I think it would be better if it was possible to use corpora, old builds and coverage separately. For example the branches I fuzz with CFLite diverge from the main branch so usually I don't need coverage and old builds. I always download public OSS-Fuzz corpora though.

@evverx
Copy link
Contributor

evverx commented Mar 31, 2022

As far as I understand if CFlite could download corpora itself if something like oss-fuzz-corpora was set #84 would be kind of automatically fixed.

@jonathanmetzman
Copy link
Collaborator Author

@jonathanmetzman I wonder if the idea is to make it possible to replace CIFuzz with CFLite with the "oss-fuzz" setting?

It is sort of my intent. It isn't quite a drop-in replacement because you still need the .clusterfuzzlite/Dockerfile and project.yaml.

If not, I think it would be better if it was possible to use corpora, old builds and coverage separately. For example the branches I fuzz with CFLite diverge from the main branch so usually I don't need coverage and old builds. I always download public OSS-Fuzz corpora though.

I'm making a feature to skip the old builds.
That would only leave you with the corpus downloading and coverage (which is broken right now for your project anyway, leading to all fuzzers being run unless there is a crash).

You would still need to do the .clusterfuzzlite stuff (which I guess you are doing anyway to cover branches) so doesn't the above suit your needs?

I understand that a download oss-fuzz corpus feature would make your life easiest, but it would either be ugly or require significantly changing how corpora are dealt with currently (we have classes for oss-fuzz and (standard) CFL users
that control the interaction with the outside world (e.g. uploading crashes, downloading/uploading corpora, downloading/uploading builds, downloading/uploading coverage). I'm open to changing this design but it will probably take longer than for me to implement this.

@evverx
Copy link
Contributor

evverx commented Apr 1, 2022

coverage (which is broken right now for your project anyway, leading to all fuzzers being run unless there is a crash)

I think systemd is broken but for example elfutils is fine and there I'd prefer to avoid using coverage. I can do that by setting the "keep-unaffected-fuzz-targets" setting explicitly though.

doesn't the above suit your needs?

I'm not sure. I need to take a closer look.

@evverx
Copy link
Contributor

evverx commented Apr 1, 2022

FWIW looking at libjpeg-turbo/libjpeg-turbo#559 (comment) it seems the libjpeg-turbo project doesn't use PRs so as far as I understand coverage and old builds aren't going to be used there either.

@jonathanmetzman
Copy link
Collaborator Author

Why couldn't it use these on commits? Those can also be fuzzed with code-change fuzzing (though I don't think this is documented).

@evverx
Copy link
Contributor

evverx commented Apr 1, 2022

Why couldn't it use these on commits? Those can also be fuzzed with code-change fuzzing (though I don't think this is documented).

In theory it should be possible but for that to work CFlite should somehow keep track of what changes between two push events. Without that it can just run all the fuzz targets.

@jonathanmetzman
Copy link
Collaborator Author

Why couldn't it use these on commits? Those can also be fuzzed with code-change fuzzing (though I don't think this is documented).

In theory it should be possible but for that to work CFlite should somehow keep track of what changes between two push events. Without that it can just run all the fuzz targets.

I don't think it strictly needs to keep track of what happens between pushes. It just needs to know what branch we are merging into and what's the diff from that branch that a change introduces. Once it knows this, it just needs some coverage to download.

@evverx
Copy link
Contributor

evverx commented Apr 1, 2022

It just needs to know what branch we are merging into and what's the diff from that branch that a change introduces

My understanding is that commits aren't merged there. They are pushed directly to various branches and CFLite can't get those diffs in its current form.

@jonathanmetzman
Copy link
Collaborator Author

It just needs to know what branch we are merging into and what's the diff from that branch that a change introduces

My understanding is that commits aren't merged there. They are pushed directly to various branches and CFLite can't get those diffs in its current form.

Right I misused the term "merge". Anyway I think based on you running CIFuzz against commits, this should work but I could be wrong. I'll stop speculating on it though :-)

@evverx
Copy link
Contributor

evverx commented Apr 1, 2022

FWIW I think google/oss-fuzz#7479 can already be covered with CFLite with a for-loop like systemd/systemd#22302 so I'd just stop using CIFuzz there and fix the build script without any changes to CFLite. Then again, I'm not sure how the branches and the fuzzing infrastructure are maintained there. My use cases are different in the sense that I need to support forks and multiple branches changed via PRs in systemd and PRs opened against a fork of elfutils so I'm not sure I'm going to use the "oss-fuzz" setting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants