Skip to content

Commit

Permalink
Fix use-after-free in CBPortal.
Browse files Browse the repository at this point in the history
There has been a heuristics to detect user bugs where a reference-like object
passed through CBPortal was not consumed timely. This turned out not to work
nicely, as in the most straightforward use case:

    const string& s = co_await untilCBCalled(
        [&](Callback<void(const string&)> cb) { /*...*/ };

-- the involved `CBPortal`, being a temporary, will get destroyed before
`CBPortal::wakeUp()` returns, leaving no opportunity to check whether the value
has been consumed.

The new test, `cb-reflike`, readily triggers a use-after-free when run with
`-fsanitize=address` (I, however, had to move `CBPortal` in heap in that test
to have adress sanitizer reliably detect the use-after-free).

Fix that by removing the heuristics.
  • Loading branch information
dprokoptsev committed Aug 7, 2024
1 parent 234d572 commit d125962
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 16 deletions.
17 changes: 1 addition & 16 deletions corral/CBPortal.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,15 +48,6 @@ template <> struct MaybeTuple<> {
static void fromTuple(std::tuple<>&&) {}
};

// Heuristics for identifying arguments to the CBPortal bridge callback
// that might be invalid to use after the callback has returned.
template <class T> constexpr bool LooksLikeReference = false;
template <class T> constexpr bool LooksLikeReference<T&> = true;
template <class T> constexpr bool LooksLikeReference<T&&> = true;
template <class T> constexpr bool LooksLikeReference<T*> = true;
template <class T> constexpr bool LooksLikeReference<std::span<T>> = true;
template <> constexpr inline bool LooksLikeReference<std::string_view> = true;

template <class InitiateFn, class CancelFn, class... CBPortals>
class CBPortalProxy;
enum class CBPortalProxyStatus : uint8_t;
Expand Down Expand Up @@ -356,13 +347,7 @@ template <class... Ts>
void CBPortal<Ts...>::Callback::operator()(Ts... values) const {
CORRAL_ASSERT(!portal_->value_.has_value() && "consumer not keeping up");
portal_->value_.emplace(std::forward<Ts>(values)...);
portal_->wakeUp();
if constexpr ((detail::LooksLikeReference<Ts> || ...)) {
CORRAL_ASSERT(!portal_->value_.has_value() &&
"reference arguments will dangle if not consumed "
"immediately; are you combining CBPortals using "
"something other than anyOf() with only CBPortal args?");
}
portal_->wakeUp(); // may destroy `portal_`
}

template <class... Ts> void CBPortal<Ts...>::wakeUp() {
Expand Down
19 changes: 19 additions & 0 deletions test/basic_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -824,6 +824,25 @@ CORRAL_TEST_CASE("cb-return-type") {
CATCH_CHECK(&rr == &x);
}

CORRAL_TEST_CASE("cb-reflike") {
std::function<void(const std::string&)> cb;
co_await allOf(
[&]() -> Task<> {
auto portal = std::make_unique<CBPortal<const std::string&>>();
std::string s = co_await untilCBCalled(
[&](std::function<void(const std::string&)> c) {
cb = c;
},
*portal);
CATCH_CHECK(s == "hello");
portal.reset();
},
[&]() -> Task<> {
co_await t.sleep(1ms);
cb("hello");
});
}

template <class T> class MyLightweightFn;
template <class... Args> class MyLightweightFn<void(Args...)> {
void (*fn_)(size_t, Args...);
Expand Down

0 comments on commit d125962

Please sign in to comment.