forward_service: prevent heap use-after-free of forward_aggregates

Currently, we create `forward_aggregates` inside a function that
returns the result of a future lambda that captures these aggregates
by reference. As a result, the aggregates may be destructed before
the lambda finishes, resulting in a heap use-after-free.

To prolong the lifetime of these aggregates, we cannot use a move
capture, because the lambda is wrapped in a with_thread_if_needed()
call on these aggregates. Instead, we fix this by wrapping the
entire return statement in a do_with().

Fixes #12528

Closes #12533
This commit is contained in:
Wojciech Mitros
2023-01-16 13:49:08 +01:00
committed by Nadav Har'El
parent 15ebd59071
commit 5f45b32bfa

View File

@@ -556,7 +556,6 @@ future<query::forward_result> forward_service::dispatch(query::forward_request r
[&req, addr = std::move(addr), &result_, tr_state_ = std::move(tr_state_)] (
query::forward_result partial_result
) mutable {
forward_aggregates aggrs(req);
auto partial_printer = seastar::value_of([&req, &partial_result] {
return query::forward_result::printer {
.functions = get_functions(req),
@@ -566,8 +565,10 @@ future<query::forward_result> forward_service::dispatch(query::forward_request r
tracing::trace(tr_state_, "Received forward_result={} from {}", partial_printer, addr);
flogger.debug("received forward_result={} from {}", partial_printer, addr);
return aggrs.with_thread_if_needed([&result_, &aggrs, partial_result = std::move(partial_result)] () mutable {
aggrs.merge(result_, std::move(partial_result));
return do_with(forward_aggregates(req), [&result_, partial_result = std::move(partial_result)] (forward_aggregates& aggrs) mutable {
return aggrs.with_thread_if_needed([&result_, &aggrs, partial_result = std::move(partial_result)] () mutable {
aggrs.merge(result_, std::move(partial_result));
});
});
});
}