This is an archive of the discontinued LLVM Phabricator instance.

Fix TSAN signal interceptor out-of-bound access
ClosedPublic

Authored by scw on Jul 21 2021, 11:05 AM.

Details

Summary

signal(2) and sigaction(2) have defined behaviors for invalid signal number
(EINVAL) and some programs rely on it.

The added test case also reveals that MSAN is too strict in this regard.

Test case passed on x86_64 Linux and AArch64 Linux.

Diff Detail

Event Timeline

scw requested review of this revision.Jul 21 2021, 11:05 AM
scw created this revision.
Herald added a subscriber: Restricted Project. · View Herald TranscriptJul 21 2021, 11:05 AM

Can you please add some basic test, ideally under compiler-rt/test/sanitizer_common/TestCases/Posix/ so we "ninja check-sanitizer" tests asan and msan as well

scw updated this revision to Diff 360892.Jul 22 2021, 11:08 AM
scw edited the summary of this revision. (Show Details)

Added test case, needed to update MSAN for it as well. Tested on x86_86 Linux and AArch64 Linux.

scw updated this revision to Diff 360895.Jul 22 2021, 11:10 AM

Oops, that was the old file. This is the actual update.

vitalybuka added inline comments.Jul 22 2021, 11:29 AM
compiler-rt/lib/msan/msan_interceptors.cpp
1377 ↗(On Diff #360895)

If so, why not on compiler-rt/lib/sanitizer_common/sanitizer_signal_interceptors.inc:54 before GetHandleSignalMode check?

compiler-rt/test/sanitizer_common/TestCases/Posix/signal.cpp
219 ↗(On Diff #360895)

Thanks if it works or all platforms.
I expected something basic which just triggers both branches of "if (signo <= 0 || signo >= kMaxSignals)"

scw added inline comments.Jul 22 2021, 11:58 AM
compiler-rt/lib/msan/msan_interceptors.cpp
1377 ↗(On Diff #360895)

sanitizer_signal_interceptors.inc is also included by asan_interceptors.cpp which does not intercept signals and does not define kMaxSignals.

compiler-rt/test/sanitizer_common/TestCases/Posix/signal.cpp
219 ↗(On Diff #360895)

Actually missed a change on relaxing checking the results of SIGCHLD. I'll try this and simplify it if build bots for different platforms complain.

scw updated this revision to Diff 360915.Jul 22 2021, 11:59 AM

Relaxed SIGCHLD handling.

vitalybuka accepted this revision.Jul 22 2021, 12:05 PM
This revision is now accepted and ready to land.Jul 22 2021, 12:05 PM

Thank you.
LGTM

This revision was landed with ongoing or failed builds.Jul 22 2021, 12:38 PM
This revision was automatically updated to reflect the committed changes.