Make adb connector wait for adb daemon to start on the guest
This avoids a race in the kernel where attempts to connect to a
vsocket before the modules are completely loaded cause a null pointer
dereference.
Bug: 131864854
Bug: 131184297
Test: run locally
Change-Id: I042a2cb1f41619a9a6e062daaa843c4c17b07de2
Merged-In: I042a2cb1f41619a9a6e062daaa843c4c17b07de2
diff --git a/host/commands/kernel_log_monitor/kernel_log_server.cc b/host/commands/kernel_log_monitor/kernel_log_server.cc
index 0ecf5fe..84b2694 100644
--- a/host/commands/kernel_log_monitor/kernel_log_server.cc
+++ b/host/commands/kernel_log_monitor/kernel_log_server.cc
@@ -39,6 +39,8 @@
monitor::BootEvent::MobileNetworkConnected},
{"VIRTUAL_DEVICE_NETWORK_WIFI_CONNECTED",
monitor::BootEvent::WifiNetworkConnected},
+ // TODO(b/131864854): Replace this with a string less likely to change
+ {"init: starting service 'adbd'", monitor::BootEvent::AdbdStarted},
};
void ProcessSubscriptions(
diff --git a/host/commands/kernel_log_monitor/kernel_log_server.h b/host/commands/kernel_log_monitor/kernel_log_server.h
index 0cc8920..f71ebb7 100644
--- a/host/commands/kernel_log_monitor/kernel_log_server.h
+++ b/host/commands/kernel_log_monitor/kernel_log_server.h
@@ -32,6 +32,7 @@
BootFailed = 2,
WifiNetworkConnected = 3,
MobileNetworkConnected = 4,
+ AdbdStarted = 5,
};
enum class SubscriptionAction {
diff --git a/host/commands/launch/launch.cc b/host/commands/launch/launch.cc
index b4de6ae..bc57025 100644
--- a/host/commands/launch/launch.cc
+++ b/host/commands/launch/launch.cc
@@ -135,20 +135,27 @@
// subscription to boot events from the kernel log monitor will be created and
// events will appear on *boot_events_pipe
cvd::Command GetKernelLogMonitorCommand(const vsoc::CuttlefishConfig& config,
- cvd::SharedFD* boot_events_pipe) {
+ cvd::SharedFD* boot_events_pipe,
+ cvd::SharedFD* adbd_events_pipe) {
auto log_name = config.kernel_log_socket_name();
auto server = cvd::SharedFD::SocketLocalServer(log_name.c_str(), false,
SOCK_STREAM, 0666);
cvd::Command kernel_log_monitor(config.kernel_log_monitor_binary());
kernel_log_monitor.AddParameter("-log_server_fd=", server);
- if (boot_events_pipe) {
- cvd::SharedFD pipe_write_end;
- if (!cvd::SharedFD::Pipe(boot_events_pipe, &pipe_write_end)) {
- LOG(ERROR) << "Unable to create boot events pipe: " << strerror(errno);
- std::exit(LauncherExitCodes::kPipeIOError);
- }
- kernel_log_monitor.AddParameter("-subscriber_fds=", pipe_write_end);
+
+ cvd::SharedFD boot_pipe_write_end;
+ if (!cvd::SharedFD::Pipe(boot_events_pipe, &boot_pipe_write_end)) {
+ LOG(ERROR) << "Unable to create boot events pipe: " << strerror(errno);
+ std::exit(LauncherExitCodes::kPipeIOError);
}
+ cvd::SharedFD adbd_pipe_write_end;
+ if (!cvd::SharedFD::Pipe(adbd_events_pipe, &adbd_pipe_write_end)) {
+ LOG(ERROR) << "Unable to create adbd events pipe: " << strerror(errno);
+ std::exit(LauncherExitCodes::kPipeIOError);
+ }
+ kernel_log_monitor.AddParameter("-subscriber_fds=", boot_pipe_write_end, ",",
+ adbd_pipe_write_end);
+
return kernel_log_monitor;
}
@@ -250,16 +257,22 @@
}
void LaunchAdbConnectorIfEnabled(cvd::ProcessMonitor* process_monitor,
- const vsoc::CuttlefishConfig& config) {
+ const vsoc::CuttlefishConfig& config,
+ cvd::SharedFD adbd_events_pipe) {
+ bool launch = false;
+ cvd::Command adb_connector(config.adb_connector_binary());
+ adb_connector.AddParameter("-adbd_events_fd=", adbd_events_pipe);
+
if (AdbTcpConnectorEnabled(config)) {
- cvd::Command adb_connector(config.adb_connector_binary());
+ launch = true;
adb_connector.AddParameter(GetAdbConnectorTcpArg());
- process_monitor->StartSubprocess(std::move(adb_connector),
- GetOnSubprocessExitCallback(config));
}
if (AdbVsockConnectorEnabled(config)) {
- cvd::Command adb_connector(config.adb_connector_binary());
+ launch = true;
adb_connector.AddParameter(GetAdbConnectorVsockArg(config));
+ }
+
+ if (launch) {
process_monitor->StartSubprocess(std::move(adb_connector),
GetOnSubprocessExitCallback(config));
}
diff --git a/host/commands/launch/launch.h b/host/commands/launch/launch.h
index 550ce02..ae79de2 100644
--- a/host/commands/launch/launch.h
+++ b/host/commands/launch/launch.h
@@ -12,7 +12,8 @@
cvd::Command GetIvServerCommand(const vsoc::CuttlefishConfig& config);
cvd::Command GetKernelLogMonitorCommand(const vsoc::CuttlefishConfig& config,
- cvd::SharedFD* boot_events_pipe);
+ cvd::SharedFD* boot_events_pipe,
+ cvd::SharedFD* adbd_events_pipe);
void LaunchLogcatReceiverIfEnabled(const vsoc::CuttlefishConfig& config,
cvd::ProcessMonitor* process_monitor);
void LaunchUsbServerIfEnabled(const vsoc::CuttlefishConfig& config,
@@ -24,7 +25,8 @@
cvd::ProcessMonitor* process_monitor,
std::function<bool(cvd::MonitorEntry*)> callback);
void LaunchAdbConnectorIfEnabled(cvd::ProcessMonitor* process_monitor,
- const vsoc::CuttlefishConfig& config);
+ const vsoc::CuttlefishConfig& config,
+ cvd::SharedFD adbd_events_pipe);
void LaunchSocketForwardProxyIfEnabled(cvd::ProcessMonitor* process_monitor,
const vsoc::CuttlefishConfig& config);
void LaunchSocketVsockProxyIfEnabled(cvd::ProcessMonitor* process_monitor,
diff --git a/host/commands/launch/main.cc b/host/commands/launch/main.cc
index ff2c7dd..9c0e3c3 100644
--- a/host/commands/launch/main.cc
+++ b/host/commands/launch/main.cc
@@ -413,12 +413,10 @@
cvd::ProcessMonitor process_monitor;
cvd::SharedFD boot_events_pipe;
+ cvd::SharedFD adbd_events_pipe;
// Only subscribe to boot events if running as daemon
process_monitor.StartSubprocess(
- GetKernelLogMonitorCommand(*config,
- config->run_as_daemon()
- ? &boot_events_pipe
- : nullptr),
+ GetKernelLogMonitorCommand(*config, &boot_events_pipe, &adbd_events_pipe),
GetOnSubprocessExitCallback(*config));
SetUpHandlingOfBootEvents(&process_monitor, boot_events_pipe,
@@ -443,7 +441,7 @@
GetOnSubprocessExitCallback(*config));
LaunchStreamAudioIfEnabled(*config, &process_monitor,
GetOnSubprocessExitCallback(*config));
- LaunchAdbConnectorIfEnabled(&process_monitor, *config);
+ LaunchAdbConnectorIfEnabled(&process_monitor, *config, adbd_events_pipe);
ServerLoop(launcher_monitor_socket, vm_manager); // Should not return
LOG(ERROR) << "The server loop returned, it should never happen!!";
diff --git a/host/frontend/adb_connector/main.cpp b/host/frontend/adb_connector/main.cpp
index e046b3d..93a929d 100644
--- a/host/frontend/adb_connector/main.cpp
+++ b/host/frontend/adb_connector/main.cpp
@@ -25,12 +25,18 @@
#include <gflags/gflags.h>
#include <unistd.h>
+#include <host/commands/kernel_log_monitor/kernel_log_server.h>
+#include "common/libs/fs/shared_fd.h"
#include "common/libs/strings/str_split.h"
#include "host/libs/config/cuttlefish_config.h"
#include "host/libs/adb_connection_maintainer/adb_connection_maintainer.h"
-DEFINE_string(addresses, "", "Comma-separated list of addresses to 'adb connect' to");
+DEFINE_string(addresses, "", "Comma-separated list of addresses to "
+ "'adb connect' to");
+DEFINE_int32(adbd_events_fd, -1, "A file descriptor. If set it will wait for "
+ "AdbdStarted boot event from the kernel log "
+ "monitor before trying to connect adb");
namespace {
void LaunchConnectionMaintainerThread(const std::string& address) {
@@ -49,12 +55,36 @@
sleep(std::numeric_limits<unsigned int>::max());
}
}
+
+void WaitForAdbdToBeStarted(int events_fd) {
+ auto evt_shared_fd = cvd::SharedFD::Dup(events_fd);
+ close(events_fd);
+ while (evt_shared_fd->IsOpen()) {
+ monitor::BootEvent event;
+ auto bytes_read = evt_shared_fd->Read(&event, sizeof(event));
+ if (bytes_read != sizeof(event)) {
+ LOG(ERROR) << "Fail to read a complete event, read " << bytes_read
+ << " bytes only instead of the expected " << sizeof(event);
+ // The file descriptor can't be trusted anymore, stop waiting and try to
+ // connect
+ return;
+ }
+ if (event == monitor::BootEvent::AdbdStarted) {
+ LOG(INFO) << "Adbd has started in the guest, connecting adb";
+ return;
+ }
+ }
+}
} // namespace
int main(int argc, char* argv[]) {
gflags::ParseCommandLineFlags(&argc, &argv, true);
CHECK(!FLAGS_addresses.empty()) << "Must specify --addresses flag";
+ if (FLAGS_adbd_events_fd >= 0) {
+ WaitForAdbdToBeStarted(FLAGS_adbd_events_fd);
+ }
+
for (auto address : ParseAddressList(FLAGS_addresses)) {
LaunchConnectionMaintainerThread(address);
}
