base: Fix off-by-one in abstract unix socket names
The code in UnixSocket was appending an extra \0 to the
socket name. So PERFETTO_CONSUMER_SOCK_NAME=@foo ended up
with a socket named "foo\0". This worked fine because both
client and server were affected consistently by the same bug.
However, this breaks interoperatbility with other tools,
most notably UI integration with adb, which doesn't support
ADB forwarding of non-ASCII abstract socket names.
Filesystem linked sockets are not affected by the bug, as
man 7 unix explicily says they must be null-terminated.
Note: this change is technically breaking the ABI of the
socket. We just think that the risk of anybody overriding the
socket with PERFETTO_CONSUMER_SOCK_NAME=@xxx is extremely low
given that abstract sockets are mostly used in testing purposes.
An internal code search and PSA on the mailing list returned
no potential breakages.
Bug: 239725760
Test: perfetto_unittests --gtest_filter=UnixSocketTest.Sockaddr*
Test: manual: start traced; socat 'UNIX-CONNECT:/tmp/perfetto-producer' /dev/stdout
Test: manual: start PERFETTO_CONSUMER_SOCK_NAME=@foo traced; socat 'ABSTRACT-CONNECT:foo' /dev/stdout
Change-Id: I478e5ff09aa78dc417c372c8906f241cb2cf0046
diff --git a/src/base/unix_socket_unittest.cc b/src/base/unix_socket_unittest.cc
index c3f2ac7..dd63504 100644
--- a/src/base/unix_socket_unittest.cc
+++ b/src/base/unix_socket_unittest.cc
@@ -32,6 +32,7 @@
#include "perfetto/ext/base/file_utils.h"
#include "perfetto/ext/base/periodic_task.h"
#include "perfetto/ext/base/pipe.h"
+#include "perfetto/ext/base/string_utils.h"
#include "perfetto/ext/base/temp_file.h"
#include "perfetto/ext/base/utils.h"
#include "src/base/test/test_task_runner.h"
@@ -965,6 +966,51 @@
#endif // !OS_WIN
+#if PERFETTO_BUILDFLAG(PERFETTO_OS_LINUX) || \
+ PERFETTO_BUILDFLAG(PERFETTO_OS_ANDROID) || \
+ PERFETTO_BUILDFLAG(PERFETTO_OS_MAC)
+
+// Regression test for b/239725760.
+TEST_F(UnixSocketTest, Sockaddr_FilesystemLinked) {
+ TempDir tmp_dir = TempDir::Create();
+ std::string sock_path = tmp_dir.path() + "/test.sock";
+ auto srv = UnixSocket::Listen(sock_path, &event_listener_, &task_runner_,
+ SockFamily::kUnix, SockType::kStream);
+ ASSERT_TRUE(srv->is_listening());
+ ASSERT_TRUE(FileExists(sock_path));
+
+ // Create a raw socket and manually connect to that (to avoid getting affected
+ // by accidental future bugs in the logic that populates struct sockaddr_un).
+ auto cli = UnixSocketRaw::CreateMayFail(SockFamily::kUnix, SockType::kStream);
+ struct sockaddr_un addr {};
+ addr.sun_family = AF_UNIX;
+ StringCopy(addr.sun_path, sock_path.c_str(), sizeof(addr.sun_path));
+ ASSERT_EQ(0, connect(cli.fd(), reinterpret_cast<struct sockaddr*>(&addr),
+ sizeof(addr)));
+ cli.Shutdown();
+ remove(sock_path.c_str());
+}
+
+// Regression test for b/239725760.
+TEST_F(UnixSocketTest, Sockaddr_AbstractUnix) {
+ StackString<128> sock_name("@perfetto_test_%d_%d", getpid(), rand() % 100000);
+ auto srv =
+ UnixSocket::Listen(sock_name.ToStdString(), &event_listener_,
+ &task_runner_, SockFamily::kUnix, SockType::kStream);
+ ASSERT_TRUE(srv->is_listening());
+
+ auto cli = UnixSocketRaw::CreateMayFail(SockFamily::kUnix, SockType::kStream);
+ struct sockaddr_un addr {};
+ addr.sun_family = AF_UNIX;
+ StringCopy(addr.sun_path, sock_name.c_str(), sizeof(addr.sun_path));
+ addr.sun_path[0] = '\0';
+ auto addr_len = static_cast<socklen_t>(
+ __builtin_offsetof(sockaddr_un, sun_path) + sock_name.len());
+ ASSERT_EQ(0, connect(cli.fd(), reinterpret_cast<struct sockaddr*>(&addr),
+ addr_len));
+}
+#endif // OS_LINUX || OS_ANDROID || OS_MAC
+
} // namespace
} // namespace base
} // namespace perfetto
