Skip to content

Commit 3aa5dab

Browse files
authored
Fix UAF in batch_create_stream_feedback_race unittest (#3305)
1 parent 4503a47 commit 3aa5dab

1 file changed

Lines changed: 28 additions & 4 deletions

File tree

test/brpc_streaming_rpc_unittest.cpp

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ struct BatchStreamFeedbackRaceState {
9191
std::atomic<bool> client_got_second_msg{false};
9292
std::atomic<bool> server_write_done{false};
9393
std::atomic<bool> rpc_done{false};
94+
std::atomic<int> client_closed_count{0};
9495

9596
bthread_t server_send_tid{0};
9697
std::atomic<bool> server_send_started{false};
@@ -123,7 +124,9 @@ class BatchStreamClientHandler : public brpc::StreamInputHandler {
123124

124125
void on_idle_timeout(brpc::StreamId /*id*/) override {}
125126

126-
void on_closed(brpc::StreamId /*id*/) override {}
127+
void on_closed(brpc::StreamId /*id*/) override {
128+
_state->client_closed_count.fetch_add(1, std::memory_order_release);
129+
}
127130

128131
void on_failed(brpc::StreamId /*id*/, int /*error_code*/, const std::string& /*error_text*/) override {}
129132

@@ -224,12 +227,17 @@ static void SetAtomicTrue(std::atomic<bool>* f) {
224227
f->store(true, std::memory_order_release);
225228
}
226229

227-
static bool WaitForTrue(const std::atomic<bool>& f, int timeout_ms) {
230+
template <typename Pred>
231+
static bool WaitForTrue(Pred pred, int timeout_ms) {
228232
const int64_t deadline_us = butil::gettimeofday_us() + (int64_t)timeout_ms * 1000L;
229-
while (!f.load(std::memory_order_acquire) && butil::gettimeofday_us() < deadline_us) {
233+
while (!pred() && butil::gettimeofday_us() < deadline_us) {
230234
usleep(1000);
231235
}
232-
return f.load(std::memory_order_acquire);
236+
return pred();
237+
}
238+
239+
static bool WaitForTrue(const std::atomic<bool>& f, int timeout_ms) {
240+
return WaitForTrue([&f]() { return f.load(std::memory_order_acquire); }, timeout_ms);
233241
}
234242

235243
TEST_F(StreamingRpcTest, sanity) {
@@ -307,6 +315,22 @@ TEST_F(StreamingRpcTest, batch_create_stream_feedback_race) {
307315
}
308316
server.Stop(0);
309317
server.Join();
318+
319+
// Release the SocketUniquePtr held above so the fake socket can be
320+
// recycled. Otherwise BeforeRecycle / on_closed for the extra stream
321+
// is deferred until `client_extra_ptr` destructs at scope exit, which
322+
// happens *after* `client_handler` and `state` are destroyed -> UAF
323+
// inside Stream::Consume on Linux.
324+
client_extra_ptr.reset();
325+
326+
// on_closed() runs asynchronously on each client stream's consumer
327+
// bthread. Wait for both before letting handler/state go out of
328+
// scope, otherwise Stream::Consume will dereference freed memory.
329+
int expected_closed = request_streams.size();
330+
WaitForTrue([&state, expected_closed]() {
331+
return state.client_closed_count.load(std::memory_order_acquire)
332+
>= expected_closed;
333+
}, 2000);
310334
};
311335

312336
test::EchoService_Stub stub(&channel);

0 commit comments

Comments
 (0)