Page MenuHomePhabricator

SafeStack: Delay thread stack clean-up
ClosedPublic

Authored by vlad.tsyrklevich on Aug 7 2018, 2:02 PM.

Details

Summary

glibc can call SafeStack instrumented code even after the last pthread
data destructor has run. Delay cleaning-up unsafe stacks for threads
until the thread is dead by having future threads clean-up prior threads
stacks.

Diff Detail

Repository
rL LLVM

Event Timeline

lib/safestack/safestack.cc
162 ↗(On Diff #159593)

Wasn't sure if I was allowed to use libpthread from a runtime. I think I could replace the mutex with the __atomic_exchange() built-in if I need to.

eugenis added a subscriber: eugenis.Aug 8 2018, 2:27 PM
eugenis added inline comments.
lib/safestack/safestack.cc
162 ↗(On Diff #159593)

We are using it already, so it should be fine.
I think it would only matter if we instrument libc... let's not worry about that for now.

202 ↗(On Diff #159593)

You are trying to append to the end of temp_stacks list.

But if we managed to destroy all pending stacks, then stackp points to temp_stacks itself, which is a local variable, and this store is dead.

  • Fix logic bug found by eugenis

Random thought: isn't the introduction of malloc here (as opposed to an OS backed alternative like mmap) gonna mess compatibility with other Sanitizers that intercept it? (thinking of Scudo which is currently compatible with SafeStack but I haven't tested).

Random thought: isn't the introduction of malloc here (as opposed to an OS backed alternative like mmap) gonna mess compatibility with other Sanitizers that intercept it? (thinking of Scudo which is currently compatible with SafeStack but I haven't tested).

malloc() would only be called during thread destruction, so it doesn't seem like there should be an issue unless malloc() causes a thread to destruct and even then this function shouldn't re-enter because of the order of when pthread_setspecific() is called. Perhaps I'm just failing to imagine a situation under which this could be an issue?

malloc() would only be called during thread destruction, so it doesn't seem like there should be an issue unless malloc() causes a thread to destruct and even then this function shouldn't re-enter because of the order of when pthread_setspecific() is called. Perhaps I'm just failing to imagine a situation under which this could be an issue?

Ack. Tried out a few things, as far as I can tell it works as intended.

eugenis accepted this revision.Aug 9 2018, 2:01 PM
This revision is now accepted and ready to land.Aug 9 2018, 2:01 PM
This revision was automatically updated to reflect the committed changes.