This is an archive of the discontinued LLVM Phabricator instance.

Fix sigaction interceptor to always correctly populate oldact
ClosedPublic

Authored by IanPudney on Aug 11 2020, 5:07 PM.

Details

Summary

This fixes https://bugs.llvm.org/show_bug.cgi?id=47118. Before this change, when the sigaction interceptor prevented a signal from being changed, it also prevented the oldact output parameter from being written to. This resulted in a use-of-uninitialized-variable by any program that used sigaction for the purpose of reading signals.

This change fixes this: the regular sigaction implementation is still called, but with the act parameter nullified, preventing any changes.

Diff Detail

Event Timeline

IanPudney created this revision.Aug 11 2020, 5:07 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 11 2020, 5:07 PM
Herald added a subscriber: Restricted Project. · View Herald Transcript
IanPudney requested review of this revision.Aug 11 2020, 5:07 PM
morehouse added inline comments.Aug 11 2020, 5:45 PM
compiler-rt/test/msan/interception_sigaction_test.cpp
14

Do we need this wrapper? Maybe set MSAN_OPTIONS=handle_segv=2 to force the mode we need, and then attempt to install your own handler and ensure it is never called.

18

Nit: s/11/SIGSEGV

19

Why do we need a loop?

21

Nit: Can we use fprintf?

IanPudney marked 4 inline comments as done.
IanPudney added inline comments.
compiler-rt/test/msan/interception_sigaction_test.cpp
14

So, I copied this pattern from other tests that test interceptors. I believe the purpose of the wrapper is to make sure that the interceptor is being called rather than the original one; that way, we know the test is actually testing what it's supposed to. I suppose your proposal would also achieve that, but this seems simpler and matches what other tests do.

19

Oops, I was planning to iterate over all signals and make sure they all worked, but then decided SIGSEGV was enough. Removed.

21

Done. Can't on the other one though, because it's happening in a signal handler.

This revision is now accepted and ready to land.Aug 12 2020, 8:12 AM
This revision was landed with ongoing or failed builds.Aug 12 2020, 10:12 AM
This revision was automatically updated to reflect the committed changes.