[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.


================
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:
> 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.
r352823


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D57490