This is an archive of the discontinued LLVM Phabricator instance.

hwasan: fix inline instrumentation
ClosedPublic

Authored by andreyknvl on Feb 9 2018, 11:08 AM.

Details

Summary

This patch changes hwasan inline instrumentation:

  1. Fixes address untagging for shadow address calculation (use 0xFF instead of 0x00 for the top byte).
  2. Emits brk instruction instead of hlt for the kernel and user space.
  3. Use 0x900 instead of 0x100 for brk immediate (0x100 - 0x800 are unavailable in the kernel).
  4. Fixes and adds appropriate tests.

Diff Detail

Event Timeline

andreyknvl created this revision.Feb 9 2018, 11:08 AM
kcc added a comment.Feb 9 2018, 11:15 AM

please add a lit test.

LGTM with a test.
Please don't copy all the tests for different hlt# values, just one would be enough to capture the difference in 0x900.

kcc added a reviewer: ramana.Feb 13 2018, 9:48 AM

Andrey, do you mind switching userspace to brk as well? You'd need to change the decoding in hwasan_linux.cc as well, but it could be even less work than writing a kernel-only lit test.

andreyknvl edited the summary of this revision. (Show Details)

Switched user space hwasan to brk.
Fixed and added tests.

andreyknvl retitled this revision from hwasan: fix kernel inline instrumentation to hwasan: fix inline instrumentation.Feb 21 2018, 4:52 AM

PTAL

It seems that hwasan_linux.cc is in a different repo (compiler-rt) and is not picked up by svn diff. Do I need to submit that change as a separate patch?

You can pass multiple repositories as arguments to svn diff, or upload a separate revision, or use git monorepo.

lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
314

Could we use 0x900 in the userspace as well? I assume 0x100 is not available in the kernel for some reason. There are no ABI concerns yet.

andreyknvl edited the summary of this revision. (Show Details)

Use 0x900 instead of 0x100 for brk immediate (0x100 - 0x800 are unavailable in the kernel).

eugenis added inline comments.Feb 21 2018, 10:01 AM
projects/compiler-rt/lib/hwasan/hwasan_linux.cc
191 ↗(On Diff #135278)

0x9XY

255 ↗(On Diff #135278)

I don't think we have a SIGTRAP handler.

Install SIGTRAP handler.

eugenis added inline comments.Feb 21 2018, 11:06 AM
projects/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cc
221 ↗(On Diff #135292)

It won't do anything, see GetHandleSignalModeImpl

Try harder to install SIGTRAP handler.

eugenis accepted this revision.Feb 21 2018, 11:25 AM

Looks great, thank you!
I can land it for you.

This revision is now accepted and ready to land.Feb 21 2018, 11:25 AM

If to land means to commit, then yes, please :)

r325711.
I had to fix userspace callbacks - they were still doing hlt with 0x100 instead of brk with 0x900.

This revision was automatically updated to reflect the committed changes.
This revision was automatically updated to reflect the committed changes.
Herald added subscribers: Restricted Project, llvm-commits. · View Herald TranscriptFeb 21 2018, 11:55 AM
Herald added a subscriber: delcypher. · View Herald Transcript