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

Enable pointer overflow checking after clang rolls #659

Closed
inferno-chromium opened this issue Jun 8, 2017 · 7 comments
Closed

Enable pointer overflow checking after clang rolls #659

inferno-chromium opened this issue Jun 8, 2017 · 7 comments

Comments

@inferno-chromium
Copy link
Collaborator

https://reviews.llvm.org/D33305

It can be enabled by passing -fsanitize=pointer-overflow -fsanitize-trap=pointer-overflow and will crash the program when a pointer overflow is detected.

@inferno-chromium
Copy link
Collaborator Author

We have the parsing signature added by Marty, he just needs to enable this on OSS-Fuzz.

@evverx
Copy link
Contributor

evverx commented May 21, 2020

@inferno-chromium @jonathanmetzman yesterday I rolled out the pointer overflow check in systemd/systemd#15865. The only place where it's currently off is OSS-Fuzz. Looking at #3123, I suspect it's never going to be turned on there. I wonder if it's safe to append -fsanitize=pointer-overflow -fno-sanitize-recover=pointer-overflow to CFLAGS to make it work?

@evverx
Copy link
Contributor

evverx commented May 21, 2020

Given that I plan to turn on some other checks I also wonder what would happen if the parsing signature wasn't available?

@inferno-chromium
Copy link
Collaborator Author

inferno-chromium commented May 21, 2020

We have parsing for all UBSan checks and if one does not exist (lets say future), we will see exceptions on our infrastructure. See this - https://github.com/google/clusterfuzz/blob/master/src/python/crash_analysis/stack_parsing/stack_analyzer.py#L1383.
In this case, totally safe to turn on pointer overflow for your project, projects can enable more ubsan checks as they like (example -

export CFLAGS="$CFLAGS -fsanitize=unsigned-integer-overflow -fno-sanitize-recover=unsigned-integer-overflow"
)
Only caveat to remember is try running your fuzz targets with those checks enabled for like a minute. If there is a quick crash, it can affect other open bugs (i.e. it will close bug since crash signature wont match anymore). In those cases, good to fix those before enabling it.

@evverx
Copy link
Contributor

evverx commented May 21, 2020

Got it. Thank you!

Only caveat to remember is try running your fuzz targets with those checks enabled for like a minute.

They have been running with the pointer-overflow check enabled for about 6 hours so I think they should be good to go. Though I think it would be much easier to make sure changes like this don't break anything if CIFuzz supported UBSan: #3421.

evverx added a commit to evverx/systemd that referenced this issue May 21, 2020
It's off by default on OSS-Fuzz but it should be safe to turn it on
manually: google/oss-fuzz#659 (comment)

Just a follow-up to systemd#15865.
@evverx
Copy link
Contributor

evverx commented May 31, 2020

Having received the first "pointer-overflow" bug report I think it would be better if there was a way to turn on additional UBSan checks on the OSS-Fuzz side so that it could bisect past the point where those checks were enabled manually.

@inferno-chromium
Copy link
Collaborator Author

Having received the first "pointer-overflow" bug report I think it would be better if there was a way to turn on additional UBSan checks on the OSS-Fuzz side so that it could bisect past the point where those checks were enabled manually.

It is all tracked in #232
Last two attempts to enable everywhere has failed. This apply non-zero offset to null pointer is a common issue across many projects and it breaks builds for those.
https://github.com/llvm-project/compiler-rt/blob/master/lib/ubsan/ubsan_handlers.cpp#L713

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

No branches or pull requests

3 participants