This is an archive of the discontinued LLVM Phabricator instance.

hwasan: Add __hwasan_init_static() function.
ClosedPublic

Authored by pcc on Jan 30 2019, 4:49 PM.

Details

Summary

This function initializes enough of the runtime to be able to run
instrumented code in a statically linked executable. It replaces
__hwasan_shadow_init() which wasn't doing enough initialization for
instrumented code that uses either TLS or IFUNC to work.

Diff Detail

Event Timeline

pcc created this revision.Jan 30 2019, 4:49 PM
eugenis added inline comments.Jan 31 2019, 2:50 PM
compiler-rt/include/sanitizer/hwasan_interface.h
26 ↗(On Diff #184411)

It should fine to call this function in dynamic executables if __rela_iplt symbols are declared weak.

In that case, would it be possible to get rid of hwasan_init_static, and simply call hwasan_init early enough? If not, is there a corresponding bionic patch to use__hwasan_init_static?

pcc marked an inline comment as done.Jan 31 2019, 3:12 PM
pcc added inline comments.
compiler-rt/include/sanitizer/hwasan_interface.h
26 ↗(On Diff #184411)

It should fine to call this function in dynamic executables if __rela_iplt symbols are declared weak.

They don't even need to be declared weak, the symbols point to the IRELATIVEs in dynamic executables as well. In PIEs and DSOs I guess it's possible (though extremely unlikely) for the r_offset for an unrelated ifunc to match the address of __hwasan_shadow, which would end up calling the wrong resolver (and segfaulting if RELRO was applied). And this will definitely segfault if the executable is a dynamically linked non-PIE (which isn't supported on Android but could be on other platforms).

In that case, would it be possible to get rid of hwasan_init_static, and simply call hwasan_init early enough?

I think that won't work because the call to madvise in MadviseShadow requires libc to be initialized more than it is currently at the point where __hwasan_init is currently called in bionic (which AFAICT was the motivation for adding __hwasan_init_shadow in D50581).

If not, is there a corresponding bionic patch to use__hwasan_init_static?

It's basically just s/__hwasan_init/__hwasan_init_static/g in bionic. I'll send it out in a bit.

Herald added a project: Restricted Project. · View Herald TranscriptJan 31 2019, 3:12 PM
eugenis accepted this revision.Jan 31 2019, 3:28 PM

LGTM
Doing less things while libc is not fully initialized is a step in the right direction.

This revision is now accepted and ready to land.Jan 31 2019, 3:28 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJan 31 2019, 3:37 PM
pcc marked an inline comment as done.Jan 31 2019, 4:32 PM
pcc added inline comments.
compiler-rt/include/sanitizer/hwasan_interface.h
26 ↗(On Diff #184411)

They don't even need to be declared weak, the symbols point to the IRELATIVEs in dynamic executables as well.

Hmm, actually it's only lld which has this behaviour. I'll make them weak.

Herald added a subscriber: Restricted Project. · View Herald TranscriptJan 31 2019, 4:32 PM
pcc marked an inline comment as done.Jan 31 2019, 4:43 PM
pcc added inline comments.
compiler-rt/include/sanitizer/hwasan_interface.h
26 ↗(On Diff #184411)

r352823