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

[elfutils] build zlib with MSan #7401

Merged
merged 1 commit into from
Mar 24, 2022

Conversation

evverx
Copy link
Contributor

@evverx evverx commented Mar 19, 2022

Unlike fuzz-dwfl-core, the new fuzz targets actually use zlib so
instead of just linking against zlib to make them compile they
should use the library instrumented with MSan. Without it OSS-Fuzz
reports bogus issues like https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=45630
and https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=45631.

To hopefully make it easier to figure out how to add new fuzz targets
going forward I also added the following comment to the build script

When new fuzz targets are added it usually makes sense to notify the maintainers of
the elfutils project using the mailing list: elfutils-devel@sourceware.org. There
fuzz targets can be reviewed properly (to make sure they don't fail to compile with -Werror
for example), their names can be chosen accordingly (so as not to spam the mailing
list with bogus bug reports that are opened and closed once they are renamed) and so
on. Also since a lot of bug reports coming out of the blue aren't exactly helpful
fuzz targets should probably be added one at a time to make it easier to keep track
of them.

It's a follow-up to #7395
and #7393.

@DavidKorczynski
Copy link
Collaborator

DavidKorczynski commented Mar 19, 2022

I think the set up should perhaps be refined. I reached out to the maintainers of elfutils by email about contributing fuzzers to the project and got a positive response, but they also wrote "we have been getting various mysterious emails from something called "ClusterFuzz-External via monorail" to the public mailinglist. They seem related to the ossfuzz project but don't contain enough information to really understand what they are about.". It seems that the maintainers have no real access to the OSS-Fuzz artifacts or communication channels (e.g. monorail) and I am skeptical of such integrations (based on personal experience).

In the context of adding fuzzers a next meaningful step would be to add the previously removed fuzzer (#6944) and targets for all the other tools -- but this will likely come with a lot of noise but also have a lot higher chance of finding true vulnerabilities.

@evverx
Copy link
Contributor Author

evverx commented Mar 19, 2022

I reached out to the maintainers of elfutils by email about contributing fuzzers to the project and got a positive response

Looking at https://sourceware.org/pipermail/elfutils-devel/2022q1/004688.html and https://sourceware.org/pipermail/elfutils-devel/2022q1/004690.html it seems to me that at least those bug reports were absolutely unexpected.

It seems that the maintainers have no real access to the OSS-Fuzz artifacts or communication channels (e.g. monorail)

I think access to OSS-Fuzz bug reports was discussed on the mailing list somewhere but basically I copied bug reports from Monorail to the elfutils bug tracker (after verifying that they were actually valid) (for example https://sourceware.org/bugzilla/show_bug.cgi?id=28715) so it wasn't necessary for the elfutils maintainers to create gmail accounts to just look at the reports.

@evverx
Copy link
Contributor Author

evverx commented Mar 19, 2022

In the context of adding fuzzers a next meaningful step would be to add the previously removed fuzzer (#6944) and targets for all the other tools.

I think it would be better to discuss that on the mailing list but looking at https://sourceware.org/pipermail/elfutils-devel/2021q4/004295.html I think the idea was to fuzz the libraries using their API instead of fuzzing the command line tools.

@evverx
Copy link
Contributor Author

evverx commented Mar 19, 2022

I think the set up should perhaps be refined.

I've been thinking about it and I'm not sure how it can be done. The problem is that the elfutils project (like many other projects) has its own bug tracker where issues are supposed to be reported and all the maintainers obviously have access to it so ideally it should be possible to somehow sync that bug tracker with Monorail automatically but unfortunately #3925 (along with all its variations) has never been addressed as far as I know so the only way to somehow sync the bug trackers is to send bug reports to the mailing list where they can be picked up manually (or semi-automatically) and move them to https://sourceware.org/bugzilla/. If there are other ways to achieve that I'd appreciate it if you could point me in the right direction.

@DavidKorczynski
Copy link
Collaborator

First I think we should ensure that the emails sent out by clusterfuzz are understood and the maintainers can reproduce by way of the OSS-Fuzz runtime. Second, I would try to get the maintainers to discuss issues by way of Monorail -- am not sure what their response was initially to this - but I would prefer not to have a too big detachment from the OSS-Fuzz infrastructure to when the maintainers can see details and reproduce the issues.

I CCed you in the email correspondence I have ongoing with Mark and Frank from Elfutils.

@evverx
Copy link
Contributor Author

evverx commented Mar 19, 2022

I think we should ensure that the emails sent out by clusterfuzz are understood and the maintainers can reproduce by way of the OSS-Fuzz runtime

I wonder if you have tried to reproduce for example https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=45628 using the OSS-Fuzz runtime following the OSS-Fuzz documentation? Anyway, while I agree it would be great if everybody could just launch ./infra/helper and be done with it I actually spend a lot of time trying to make it possible to build fuzzers without all that docker machinery to make it easier for maintainers of various project (including myself) to reproduce and fix bug reports.

Second, I would try to get the maintainers to discuss issues by way of Monorail -- am not sure what their response was initially to this

Here's what I was able to find following the links I posted here:

google really should just report these issues upstream instead of hiding them in their own bug tracker.

I have used OSS-Fuzz for some other projects, but found it
very painful to get any results out if you don't have a google
account. If you can set it up so that it posts the results and
artifacts to the mailinglists that would be great.

@evverx
Copy link
Contributor Author

evverx commented Mar 19, 2022

I CCed you in the email correspondence I have ongoing with Mark and Frank from Elfutils.

Thanks. I'm not sure why it isn't posted to the mailing list though. That's where everything is discussed and where it can be found later.

@evverx
Copy link
Contributor Author

evverx commented Mar 20, 2022

I've just been subscribed to that thread and it seems at this point I can't add anything new there.

FWIW just to avoid reinventing the wheel the patch where the fuzzing infrastructure was integrated into the the elfutils build system is waiting for review at https://sourceware.org/pipermail/elfutils-devel/2021q4/004615.html. It is battle-tested, compatible with OSS-Fuzz, the elfutils CI, afl, hongfuzz, --enable-sanitize-{address,undefined} and/or --enable-valgrind and all the other tools used by the elfutils maintainers mentioned in that thread and the new fuzz targets can be easily integrated there.

@evverx
Copy link
Contributor Author

evverx commented Mar 20, 2022

As promised in https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=45628#c3 I've just opened #7403.

@DavidKorczynski
Copy link
Collaborator

FWIW just to avoid reinventing the wheel the patch where the fuzzing infrastructure was integrated into the the elfutils build system is waiting for review at https://sourceware.org/pipermail/elfutils-devel/2021q4/004615.html. It is battle-tested, compatible with OSS-Fuzz, the elfutils CI, afl, hongfuzz, --enable-sanitize-{address,undefined} and/or --enable-valgrind and all the other tools used by the elfutils maintainers mentioned in that thread and the new fuzz targets can be easily integrated there.

Ah, this is perfect and yes it should be straightforward integrating new fuzz targets to that. Sounds like you are pushing elfutils forward nicely so I will leave you to it!

Copy link
Collaborator

@DavidKorczynski DavidKorczynski left a comment

Choose a reason for hiding this comment

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

LGTM

@evverx
Copy link
Contributor Author

evverx commented Mar 23, 2022

ideally it should be possible to somehow sync that bug tracker with Monorail automatically but unfortunately #3925 (along with all its variations) has never been addressed as far as I know

Looks like it was an overstatement. Looking at #7023 it seems GitHub should be covered soon. It doesn't help in this particular case but I felt I had to correct myself.

I rebased the PR on top of the master branch and force-pushed the code. I think it should be good to go.

@evverx evverx marked this pull request as ready for review March 23, 2022 16:30
Unlike fuzz-dwfl-core, the new fuzz targets actually use zlib so
instead of just linking against zlib to make them compile they
should use the library instrumented with MSan. Without it OSS-Fuzz
reports bogus issues like https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=45630
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=45631 and
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=45633.

To hopefully make it easier to figure out how to add new fuzz targets
going forward I also added the following comment to the build script
```
When new fuzz targets are added it usually makes sense to notify the maintainers of
the elfutils project using the mailing list: elfutils-devel@sourceware.org. There
fuzz targets can be reviewed properly (to make sure they don't fail to compile with -Werror
for example), their names can be chosen accordingly (so as not to spam the mailing
list with bogus bug reports that are opened and closed once they are renamed) and so
on. Also since a lot of bug reports coming out of the blue aren't exactly helpful
fuzz targets should probably be added one at a time to make it easier to keep track
of them.
```

It's a follow-up to google#7395
and google#7393.
@jonathanmetzman jonathanmetzman merged commit 2b62a90 into google:master Mar 24, 2022
@evverx evverx deleted the elfutils-follow-up branch March 25, 2022 17:32
MartinPetkov pushed a commit to MartinPetkov/oss-fuzz that referenced this pull request Aug 15, 2022
Unlike fuzz-dwfl-core, the new fuzz targets actually use zlib so
instead of just linking against zlib to make them compile they
should use the library instrumented with MSan. Without it OSS-Fuzz
reports bogus issues like https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=45630
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=45631 and
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=45633.

To hopefully make it easier to figure out how to add new fuzz targets
going forward I also added the following comment to the build script
```
When new fuzz targets are added it usually makes sense to notify the maintainers of
the elfutils project using the mailing list: elfutils-devel@sourceware.org. There
fuzz targets can be reviewed properly (to make sure they don't fail to compile with -Werror
for example), their names can be chosen accordingly (so as not to spam the mailing
list with bogus bug reports that are opened and closed once they are renamed) and so
on. Also since a lot of bug reports coming out of the blue aren't exactly helpful
fuzz targets should probably be added one at a time to make it easier to keep track
of them.
```

It's a follow-up to google#7395
and google#7393.
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.

None yet

3 participants