[PATCH] D57490: hwasan: Add __hwasan_init_static() function.

[Peter Collingbourne via Phabricator via llvm-commits]

pcc marked an inline comment as done.
pcc added inline comments.
Herald added a subscriber: Sanitizers.


================
Comment at: compiler-rt/include/sanitizer/hwasan_interface.h:26
+  // have been applied by the time the function is called. This also means that
+  // the function should be called before libc applies RELRO.
   // Does not call libc unless there is an error.
----------------
pcc wrote:
> eugenis wrote:
> > 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?
> > 
> > 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.
> 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.


Repository:
  rCRT Compiler Runtime

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57490/new/

https://reviews.llvm.org/D57490