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

Why is nonnull-attribute disabled on OSS-Fuzz #2518

Closed
evverx opened this issue Jun 17, 2019 · 7 comments
Closed

Why is nonnull-attribute disabled on OSS-Fuzz #2518

evverx opened this issue Jun 17, 2019 · 7 comments

Comments

@evverx
Copy link
Contributor

evverx commented Jun 17, 2019

When the fuzz targets from the systemd project were ported to Fuzzit, -fsanitize=undefined was initially passed to the compiler, which caused a few crashes there: systemd/systemd#12771 (comment). To temporarily silence UBsan, in systemd/systemd#12761 I turned on a subset of https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#available-checks I borrowed from

# Set of '-fsanitize' flags matches '-fno-sanitize-recover' + 'unsigned-integer-overflow'.
ENV SANITIZER_FLAGS_undefined "-fsanitize=bool,array-bounds,float-divide-by-zero,function,integer-divide-by-zero,return,shift,signed-integer-overflow,unsigned-integer-overflow,vla-bound,vptr -fno-sanitize-recover=bool,array-bounds,float-divide-by-zero,function,integer-divide-by-zero,return,shift,signed-integer-overflow,vla-bound,vptr"

I'm going to turn nonnull-attribute on on Fuzzit in systemd/systemd#12810, but I'm wondering why it's not used on ClusterFuzz. Is it not considered a bug or does it produce a lot of false positives?

@evverx
Copy link
Contributor Author

evverx commented Jun 17, 2019

Generally, I'd say it's not obvious to me why some of the checks are disabled. For example, I found #232 (comment) where object-size was turned on and then turned off again. If those checks aren't exactly reliable wouldn't it be better to turn them off by default on the clang side when -fsanitize=undefined is used?

@evverx
Copy link
Contributor Author

evverx commented Jun 17, 2019

In a sense this issue is a duplicate of #232, which has been inactive for more than 2 years, so, in principle, it can probably be closed and the discussion can be moved there if it's more convenient.

@inferno-chromium
Copy link
Collaborator

Probably nonnull-attribute is new, dont remember seeing this. We can enable this, might also need a parsing change in https://github.com/google/clusterfuzz/blob/master/src/python/crash_analysis/stack_parsing/stack_symbolizer.py

@evverx
Copy link
Contributor Author

evverx commented Jun 17, 2019

I'm not sure it's new. Judging by systemd/systemd#9738, It's been crashing our unit tests since the summer of 2018 :-)

@evverx
Copy link
Contributor Author

evverx commented Jun 17, 2019

At first glance, the issue reported by UBSan with pointer-overflow looks correct as well. The pointer seems to point to an address beyond "defined" limits. Luckily, it doesn't seem to be dereferenced anywhere afterwards. Everything is fine :-)

@evverx
Copy link
Contributor Author

evverx commented Jun 17, 2019

FWIW It turns out that the fuzz targets in the systemd project can withstand all default UBSan checks except for "pointer-overflow","object-size" and "float-cast-overflow". Not that these three checks produce false positives though. I'll turn them on later when the fuzz targets are ready.

@evverx
Copy link
Contributor Author

evverx commented Jun 19, 2019

Since I no longer use OSS-Fuzz as a primary source of bugs discovered by UBSan and I turned on the checks I need elsewhere, I think this issue can be closed. #232 refers to this issue so when someone gets round to it, it should be relatively easy to find it.

@evverx evverx closed this as completed Jun 19, 2019
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

No branches or pull requests

2 participants