Commit f1c65ff8 authored by Dominik Charousset's avatar Dominik Charousset

Fix heap-use-after-free bug

parent 1899b17f
...@@ -22,6 +22,9 @@ is based on [Keep a Changelog](https://keepachangelog.com). ...@@ -22,6 +22,9 @@ is based on [Keep a Changelog](https://keepachangelog.com).
- Using `allow(...).with(...)` in unit tests without a matching message crashed - Using `allow(...).with(...)` in unit tests without a matching message crashed
the program. By adding a missing NULL-check, `allow` is now always safe to the program. By adding a missing NULL-check, `allow` is now always safe to
use. use.
- Passing a response promise to a run-delayed continuation could result in a
heap-use-after-free if the actor terminates before the action runs. The
destructor of the promise now checks for this case.
### Changed ### Changed
......
...@@ -197,7 +197,7 @@ private: ...@@ -197,7 +197,7 @@ private:
void delegate_impl(abstract_actor* receiver, message msg); void delegate_impl(abstract_actor* receiver, message msg);
mutable size_t ref_count = 1; mutable size_t ref_count = 1;
local_actor* self; weak_actor_ptr weak_self;
strong_actor_ptr source; strong_actor_ptr source;
forwarding_stack stages; forwarding_stack stages;
message_id id; message_id id;
......
...@@ -40,7 +40,7 @@ response_promise::response_promise(local_actor* self, strong_actor_ptr source, ...@@ -40,7 +40,7 @@ response_promise::response_promise(local_actor* self, strong_actor_ptr source,
// anonymous messages since there's nowhere to send the message to anyway. // anonymous messages since there's nowhere to send the message to anyway.
if (requires_response(mid)) { if (requires_response(mid)) {
state_ = make_counted<state>(); state_ = make_counted<state>();
state_->self = self; state_->weak_self = self->ctrl();
state_->source.swap(source); state_->source.swap(source);
state_->stages.swap(stages); state_->stages.swap(stages);
state_->id = mid; state_->id = mid;
...@@ -60,7 +60,7 @@ bool response_promise::async() const noexcept { ...@@ -60,7 +60,7 @@ bool response_promise::async() const noexcept {
} }
bool response_promise::pending() const noexcept { bool response_promise::pending() const noexcept {
return state_ != nullptr && state_->self != nullptr; return state_ != nullptr && state_->weak_self != nullptr;
} }
strong_actor_ptr response_promise::source() const noexcept { strong_actor_ptr response_promise::source() const noexcept {
...@@ -122,7 +122,7 @@ void response_promise::respond_to(local_actor* self, mailbox_element* request, ...@@ -122,7 +122,7 @@ void response_promise::respond_to(local_actor* self, mailbox_element* request,
if (request && requires_response(*request) if (request && requires_response(*request)
&& has_response_receiver(*request)) { && has_response_receiver(*request)) {
state tmp; state tmp;
tmp.self = self; tmp.weak_self = self->ctrl();
tmp.source.swap(request->sender); tmp.source.swap(request->sender);
tmp.stages.swap(request->stages); tmp.stages.swap(request->stages);
tmp.id = request->mid; tmp.id = request->mid;
...@@ -136,7 +136,7 @@ void response_promise::respond_to(local_actor* self, mailbox_element* request, ...@@ -136,7 +136,7 @@ void response_promise::respond_to(local_actor* self, mailbox_element* request,
if (request && requires_response(*request) if (request && requires_response(*request)
&& has_response_receiver(*request)) { && has_response_receiver(*request)) {
state tmp; state tmp;
tmp.self = self; tmp.weak_self = self->ctrl();
tmp.source.swap(request->sender); tmp.source.swap(request->sender);
tmp.stages.swap(request->stages); tmp.stages.swap(request->stages);
tmp.id = request->mid; tmp.id = request->mid;
...@@ -148,18 +148,28 @@ void response_promise::respond_to(local_actor* self, mailbox_element* request, ...@@ -148,18 +148,28 @@ void response_promise::respond_to(local_actor* self, mailbox_element* request,
// -- state -------------------------------------------------------------------- // -- state --------------------------------------------------------------------
response_promise::state::~state() { response_promise::state::~state() {
if (self) { // Note: the state may get destroyed outside of the actor. For example, when
// storing the promise in a run-later continuation. Hence, we can't call
// deliver_impl here since it calls self->context().
if (weak_self && source) {
CAF_LOG_DEBUG("broken promise!"); CAF_LOG_DEBUG("broken promise!");
deliver_impl(make_message(make_error(sec::broken_promise))); auto element = make_mailbox_element(weak_self.lock(), id.response_id(),
no_stages,
make_error(sec::broken_promise));
source->enqueue(std::move(element), nullptr);
} }
} }
void response_promise::state::cancel() { void response_promise::state::cancel() {
self = nullptr; weak_self = nullptr;
} }
void response_promise::state::deliver_impl(message msg) { void response_promise::state::deliver_impl(message msg) {
CAF_LOG_TRACE(CAF_ARG(msg)); CAF_LOG_TRACE(CAF_ARG(msg));
// Even though we are holding a weak pointer, we can access the pointer
// without any additional check here because only the actor itself is allowed
// to call this function.
auto self = static_cast<local_actor*>(weak_self.get()->get());
if (msg.empty() && id.is_async()) { if (msg.empty() && id.is_async()) {
CAF_LOG_DEBUG("drop response: empty response to asynchronous input"); CAF_LOG_DEBUG("drop response: empty response to asynchronous input");
} else if (!stages.empty()) { } else if (!stages.empty()) {
...@@ -178,6 +188,7 @@ void response_promise::state::delegate_impl(abstract_actor* receiver, ...@@ -178,6 +188,7 @@ void response_promise::state::delegate_impl(abstract_actor* receiver,
message msg) { message msg) {
CAF_LOG_TRACE(CAF_ARG(msg)); CAF_LOG_TRACE(CAF_ARG(msg));
if (receiver != nullptr) { if (receiver != nullptr) {
auto self = static_cast<local_actor*>(weak_self.get()->get());
detail::profiled_send(self, std::move(source), receiver, id, detail::profiled_send(self, std::move(source), receiver, id,
std::move(stages), self->context(), std::move(msg)); std::move(stages), self->context(), std::move(msg));
} else { } else {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment