Boost C++ Libraries: Ticket #11097: test_scheduled_tp - ThreadSanitizer: heap-use-after-free

status, component changed; owner set

viboes — Tue, 10 Mar 2015 18:42:47 GMT

owner set to viboes
status new → assigned
component None → thread

daira@… — Fri, 12 Aug 2016 18:45:42 GMT

https://github.com/zcash/zcash/issues/1241

Any response to this? Use-after-free is often an exploitable security bug, and boost is used in security-critical software such as Bitcoin.

viboes — Mon, 15 Aug 2016 17:27:00 GMT

Hi,

sorry for the inconvenience. Any help on this will be much appreciated.

daira@… — Sun, 21 Aug 2016 12:57:08 GMT

Well, the problem is likely in sync_timed_queue, but I've no idea what its locking policy is.

daira@… — Sun, 21 Aug 2016 13:00:25 GMT

Sorry, was confusing this with a race condition; locking policy might not be relevant. In any case, I'm not familiar enough with the internals of boost to help.

daira@… — Sun, 21 Aug 2016 13:07:39 GMT

It seems as though the report in the description was not compiled with enough debug information to give line numbers. The report in the zcash bug does have line numbers; note that it is not necessarily exactly the same bug.

viboes — Wed, 31 Aug 2016 16:54:18 GMT

Why do you think the issue comes from sync_timed_queue?

patrik@… — Wed, 31 Aug 2016 18:20:10 GMT

We're experiencing this behavior too, in boost 1.61.0.

daira@… — Thu, 01 Sep 2016 21:46:05 GMT

viboes: I no longer remember at all why I thought that.

viboes — Sat, 03 Sep 2016 18:40:56 GMT

I believe that I've found where the problem is. The call to super::data_.top().time

        super::not_empty_.wait_until(lk, super::data_.top().time);

pass the time by reference while we need to copy it, as the storage can be re-arranged.

  template <class T, class Clock>
  bool sync_timed_queue<T, Clock>::wait_until_not_empty_time_reached_or_closed(unique_lock<mutex>& lk)
  {
    for (;;)
    {
      if (super::closed(lk)) return true;
      while (! super::empty(lk)) {
        if (! time_not_reached(lk)) return false;
-        super::not_empty_.wait_until(lk, super::data_.top().time);
+        time_point tp = super::data_.top().time;
+        super::not_empty_.wait_until(lk, tp);
        if (super::closed(lk)) return true;
      }
      if (super::closed(lk)) return true;
      super::not_empty_.wait(lk);
    }
    //return false;
  }

See this commit

https://github.com/boostorg/thread/commit/c52a34c2cf1b2e9807a70065c03b0555895963df

milestone changed

viboes — Sat, 03 Sep 2016 18:41:09 GMT

milestone To Be Determined → Boost 1.62.0

status changed; resolution set

viboes — Tue, 06 Sep 2016 16:51:42 GMT

status assigned → closed
resolution → fixed