This is an archive of the discontinued LLVM Phabricator instance.

[hwasan] Display causes in order of probability.
ClosedPublic

Authored by fmayer on Jun 23 2021, 6:21 AM.

Details

Summary

A heap or global buffer that is far away from the faulting address is
unlikely to be the cause, especially if there is a potential
use-after-free as well, so we want to show it after the other
causes.

Diff Detail

Event Timeline

fmayer requested review of this revision.Jun 23 2021, 6:21 AM
fmayer created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptJun 23 2021, 6:21 AM
Herald added a subscriber: Restricted Project. · View Herald Transcript
fmayer updated this revision to Diff 353950.Jun 23 2021, 6:36 AM

Only treat linear overflows as very likely.

@pcc @hctim
I'd like to have a test that a far overflow is not printed where there is a valid use after free.
You can do it by

  • allocating in a loop until you get too allocations that are nearby but not adjacent
  • faking the tags so they match (__hwasan_tag_memory can do that)
  • deallocating one and touching it

hwasan does not record the original allocation tag anywhere (but it records "free" in the history buffer)

compiler-rt/lib/hwasan/hwasan_report.cpp
318

Make it "Cause:" to match the MTE output:
https://cs.android.com/android/platform/superproject/+/master:system/core/debuggerd/libdebuggerd/tombstone_proto_to_text.cpp;l=263;drc=78f0670ddadb98c270cc0a0115bb4da4e5d80a95

Add an explanation about multiple possible causes somewhere. It seems hard to count them before printing, so I'm ok with the note going to the end of the report.

420

The only way to "describe" a stack location is through the history buffer. ShowCandidate should iterate over it looking for a matching tag and address (but unlike use-after-return, the address would be the suspected overflow address, not the fault address, and the tag would match the current memory tag).

This becomes complicated in the offline symbolization case (see the hwasan_symbolize script). Unsymbolized online reporting would need to print tag and address of the suspected original allocation that can be matched against offline symbolized history.

This is out of scope of this change.

474

I suggest we do not print "far" overflow candidates if there is any other alternative at all (i.e. num_descriptions > 0).

fmayer updated this revision to Diff 354260.Jun 24 2021, 8:10 AM
fmayer marked 3 inline comments as done.

Add test.

Added a test. Verified it fails with the previous logic and passes with the new one.

fmayer updated this revision to Diff 354261.Jun 24 2021, 8:12 AM

Remove leftover print in test.

eugenis added a reviewer: pcc.Jun 25 2021, 2:39 PM

LGTM with the test changes

compiler-rt/test/hwasan/TestCases/use-after-free-and-overflow.c
24

distance is in bytes, you probably want something more than 1.

41

break if other != nullptr?

47

This can flake if any of the adjacent allocations have a tag of 3.

Maybe iterate over possible tag values until __hwasan_test_shadow both on the left and on the right returns an error?

fmayer updated this revision to Diff 354826.Jun 28 2021, 3:24 AM
fmayer marked 3 inline comments as done.

Improve test.

compiler-rt/test/hwasan/TestCases/use-after-free-and-overflow.c
47

I think it's easier to just tag the two surrounding bytes to prevent the flake.

fmayer updated this revision to Diff 354852.Jun 28 2021, 5:23 AM

Add comment to test.

eugenis accepted this revision.Jun 28 2021, 4:10 PM

LGTM

compiler-rt/test/hwasan/TestCases/use-after-free-and-overflow.c
19

nit: single statement blocks do not use {} in LLVM

This revision is now accepted and ready to land.Jun 28 2021, 4:10 PM
fmayer updated this revision to Diff 355175.Jun 29 2021, 4:04 AM
fmayer marked an inline comment as done.

Address formatting nits.

This revision was landed with ongoing or failed builds.Jun 29 2021, 5:00 AM
This revision was automatically updated to reflect the committed changes.