This is an archive of the discontinued LLVM Phabricator instance.

Enhance support for NetBSD in SafeStack
ClosedPublic

Authored by krytarowski on Jan 24 2019, 12:15 PM.

Details

Summary

Always try to detect and call internal or real libc symbols instead of
locally installed interceptors.

This covers:

  • GetTid()
  • TgKill()
  • Mmap()
  • Munmap()
  • Mprotect()

This cherry-picks code from sanitizer_common/sanitizer_netbsd.cc.

Diff Detail

Event Timeline

krytarowski created this revision.Jan 24 2019, 12:15 PM
vitalybuka added inline comments.Jan 24 2019, 12:55 PM
lib/safestack/safestack_platform.h
35

Why do we need weak here?

45

maybe merge this and "#if SANITIZER_NETBSD" above?

83

Why do you rely need DEFINE__REAL? I just copied TgKill for other platforms. Not sure that syscalls are necessary.
mmap/protect is a different story, see below

114

We don't want to hit interpector here, because it can be called called from preinit_array.
DEFINE__REAL will avoid interceptors, however dlsym from preinit_array may crash as it can to try to allocate memory.
So we use syscall on linux.
sanitizer_common/sanitizer_netbsd still had such problem, so I didn't bother to copy DEFINE_REAL.

krytarowski marked 3 inline comments as done.Jan 24 2019, 1:13 PM
krytarowski added inline comments.
lib/safestack/safestack_platform.h
35

It was needed for sanitizer common but it seems to work fine for safestack without the weak attribute.

83

For LWP functions this is optional, for consistence I prefer to keep it.

114

NetBSD does not use/support preinit_array, except on archs that demand it (amd64/i386 aren't in the set).

How does it crash with allocating the memory? By calling real malloc? Dynamic loader on NetBSD uses its internal copy of xmalloc() for allocations.

According to my quick testing with gdb(1) and an example program, dlsym(3) does not allocate anything through xmalloc() (at least for RTLD_SELF).

vitalybuka accepted this revision.Jan 24 2019, 1:29 PM
vitalybuka added inline comments.
lib/safestack/safestack_platform.h
96

This SFS_CHECK (like the on in sanitizer_common) is not useful with or without weak
It will crash anyway with obvious stack on null pointer.

114

Yes, on linux it crashes with malloc which can be not initialized (if its non standard allocator)

This revision is now accepted and ready to land.Jan 24 2019, 1:29 PM
krytarowski marked 2 inline comments as done.Jan 24 2019, 1:37 PM
krytarowski added inline comments.
lib/safestack/safestack_platform.h
45

I prefer to keep this function namespaced.

114

So there is no problem on NetBSD, at least in the current circumstances nothing is known to be faulty.

This revision was automatically updated to reflect the committed changes.