crimson/mon/MonClient: fix use-after-free in run_command

run_command() captured a reference to the just-emplaced mon_commands
element into the send_message() continuation. mon_commands is a
std::vector, so a concurrent run_command() could reallocate it and
invalidate the reference before the continuation ran, dereferencing
freed memory.

No caller issues concurrent commands today, but the pattern is unsafe.

Take the result future before issuing the message, then coroutinize
the method so it is awaited directly instead of being captured into a
continuation lambda.

Signed-off-by: Sun Yuechi <sunyuechi@iscas.ac.cn>

diff --git a/src/crimson/mon/MonClient.cc b/src/crimson/mon/MonClient.cc
index fb60645f8688ef10e9966ae0666518d132f5c40d..b6aeb5a1ae0846d323cd3903defa1eea73fa4ac8 100644 (file)
--- a/src/crimson/mon/MonClient.cc
+++ b/src/crimson/mon/MonClient.cc
@@ -5,6 +5,7 @@
 
 #include <random>
 #include <fmt/ranges.h>
+#include <seastar/core/coroutine.hh>
 #include <seastar/core/future-util.hh>
 #include <seastar/core/lowres_clock.hh>
 #include <seastar/core/shared_future.hh>
@@ -1076,10 +1077,10 @@ Client::run_command(std::string&& cmd,
   m->set_tid(tid);
   m->cmd = {std::move(cmd)};
   m->set_data(std::move(bl));
-  auto& command = mon_commands.emplace_back(crimson::make_message<MMonCommand>(*m));
-  return send_message(std::move(m)).then([&result=command.result] {
-    return result.get_future();
-  });
+  auto fut = mon_commands.emplace_back(crimson::make_message<MMonCommand>(*m))
+                 .result.get_future();
+  co_await send_message(std::move(m));
+  co_return co_await std::move(fut);
 }
 
 seastar::future<> Client::send_message(MessageURef m)