From 96d7b5a4d7d913c918f5ccf7315f17d7470bc516 Mon Sep 17 00:00:00 2001
From: Zhenyao Mo <[email protected]>
Date: Mon, 21 Sep 2026 18:42:04 -0700
Subject: [PATCH] [chromecast] Restrict tracing socket access and authenticate clients

The system tracing daemon (chromecast/tracing:traced) runs with the
privileges required to drive system-wide ftrace: it writes the ftrace
control files under /sys/kernel/tracing and streams the resulting trace
buffer back to whoever asked for it. Its control socket was published
with mode 0666, and the daemon accepted every connection without looking
at who was on the other end, so any local process could enable kernel
tracing and read the result.

That matters most for the GPU process on Cast ARM devices, which is not
chrooted and whose seccomp policy (CrosArmGpuProcessPolicy) allows
socket(AF_UNIX, ...) and connect(). A compromised GPU process could
therefore reach /dev/socket/tracing/tracing, turn on categories such as
"sched", and read microsecond-resolution scheduling data for every
process on the device, which is a high-resolution cross-process side
channel.

Restrict access at both layers:

- Create the socket node with mode 0660 instead of 0666. The umask is
  also narrowed across bind(), because bind() creates the node with
  0777 & ~umask and an inherited permissive umask would otherwise leave
  the node world-writable for the window before chmod() runs. If the
  node still ends up world-accessible, creation fails instead of
  serving on it.
- Check SO_PEERCRED in AcceptConnection() and drop peers that do not run
  as the daemon's user. This is deliberately redundant with the mode:
  the mode is applied to a path under a directory the daemon does not
  own and can be widened by a mis-configured deployment, whereas peer
  credentials come from the kernel and cannot be spoofed by the client.

Socket creation moves from tracing_service_main.cc to
system_tracing_common so that both halves can be unit tested; traced is
a cast_executable and has no test target of its own. The new
cast_tracing_unittests covers the resulting mode under a permissive
umask, umask restoration, replacement of a stale socket node, rejection
of an unwritable path, and authorization of a peer accepted over a real
listening socket.

Also fixes an existing build break in tracing_service_main.cc, which
used the long-removed uint8 type and so did not compile.

Bug: 513417142
Change-Id: Ib531bd80b2554cc5b875ac488630e53bd6ee08a2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8430463
Commit-Queue: Zhenyao Mo <[email protected]>
Reviewed-by: Simeon Anfinrud <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1702296}
---
diff --git a/chromecast/tracing/system_tracing_common.cc b/chromecast/tracing/system_tracing_common.cc
index b25c22f3..a0275e3 100644
--- a/chromecast/tracing/system_tracing_common.cc
+++ b/chromecast/tracing/system_tracing_common.cc
@@ -4,11 +4,17 @@
 
 #include "chromecast/tracing/system_tracing_common.h"
 
+#include <errno.h>
 #include <string.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
 
 #include <iterator>
 
 #include "base/compiler_specific.h"
+#include "base/logging.h"
+#include "base/strings/stringprintf.h"
 
 namespace chromecast {
 namespace tracing {
@@ -16,6 +22,10 @@
 
 const char kSocketPath[] = "/dev/socket/tracing/tracing";
 
+// Access mode for the listening socket node: the service's user and group
+// only. See CreateTracingServerSocket().
+constexpr mode_t kSocketMode = 0660;
+
 }  // namespace
 
 sockaddr_un GetSystemTracingSocketAddress() {
@@ -28,5 +38,85 @@
   return addr;
 }
 
+base::ScopedFD CreateTracingServerSocket(const sockaddr_un& addr) {
+  if (unlink(addr.sun_path) != 0 && errno != ENOENT) {
+    PLOG(ERROR) << "unlink: " << addr.sun_path;
+  }
+
+  base::ScopedFD socket_fd(
+      socket(AF_UNIX, SOCK_SEQPACKET | SOCK_NONBLOCK | SOCK_CLOEXEC, 0));
+  if (!socket_fd.is_valid()) {
+    PLOG(ERROR) << "socket";
+    return base::ScopedFD();
+  }
+
+  // bind() creates the node with mode 0777 & ~umask, so a permissive inherited
+  // umask would publish a world-writable socket for the window between bind()
+  // and the chmod() below. Narrow the umask across the bind() to close that
+  // window. The caller is single threaded at this point, so temporarily
+  // changing the process-wide umask is safe.
+  const mode_t old_umask = umask(0777 & ~kSocketMode);
+  const int bind_result =
+      bind(socket_fd.get(), reinterpret_cast<const struct sockaddr*>(&addr),
+           sizeof(addr));
+  const int bind_errno = errno;
+  umask(old_umask);
+
+  if (bind_result) {
+    errno = bind_errno;
+    PLOG(ERROR) << "bind: " << addr.sun_path;
+    return base::ScopedFD();
+  }
+
+  // Not all filesystems apply the umask to socket nodes, so also set the mode
+  // explicitly.
+  if (chmod(addr.sun_path, kSocketMode)) {
+    PLOG(WARNING) << "chmod: " << addr.sun_path;
+  }
+
+  // Whatever the reason a restricted mode could not be applied, refuse to
+  // serve on a node that arbitrary local processes can reach.
+  struct stat st = {};
+  if (stat(addr.sun_path, &st)) {
+    PLOG(ERROR) << "stat: " << addr.sun_path;
+    return base::ScopedFD();
+  }
+  if (st.st_mode & S_IRWXO) {
+    LOG(ERROR) << "Refusing to listen on world-accessible socket "
+               << addr.sun_path << " "
+               << base::StringPrintf("(mode 0%o)",
+                                     static_cast<unsigned>(st.st_mode & 07777));
+    return base::ScopedFD();
+  }
+
+  static constexpr int kBacklog = 10;
+  if (listen(socket_fd.get(), kBacklog)) {
+    PLOG(ERROR) << "listen: " << addr.sun_path;
+    return base::ScopedFD();
+  }
+
+  return socket_fd;
+}
+
+bool IsTracingClientAuthorized(int socket_fd) {
+  struct ucred cred = {};
+  socklen_t cred_size = sizeof(cred);
+  if (getsockopt(socket_fd, SOL_SOCKET, SO_PEERCRED, &cred, &cred_size) < 0) {
+    PLOG(ERROR) << "getsockopt(SO_PEERCRED)";
+    return false;
+  }
+  if (cred_size != sizeof(cred)) {
+    LOG(ERROR) << "getsockopt(SO_PEERCRED) returned " << cred_size
+               << " bytes, expected " << sizeof(cred);
+    return false;
+  }
+  if (cred.uid != geteuid()) {
+    LOG(WARNING) << "Rejecting tracing client with uid " << cred.uid << " (pid "
+                 << cred.pid << ")";
+    return false;
+  }
+  return true;
+}
+
 }  // namespace tracing
 }  // namespace chromecast
diff --git a/chromecast/tracing/system_tracing_common.h b/chromecast/tracing/system_tracing_common.h
index af66b05..8fc0fc0 100644
--- a/chromecast/tracing/system_tracing_common.h
+++ b/chromecast/tracing/system_tracing_common.h
@@ -10,6 +10,7 @@
 
 #include <array>
 
+#include "base/files/scoped_file.h"
 #include "base/trace_event/common/trace_event_common.h"
 
 namespace chromecast {
@@ -21,6 +22,28 @@
 
 sockaddr_un GetSystemTracingSocketAddress();
 
+// Creates the listening socket for the system tracing service, bound to
+// |addr.sun_path|. Any pre-existing node at that path is removed first.
+//
+// The node is created with mode 0660. The service runs with the privileges
+// required to drive system-wide ftrace, so any process that can connect can
+// enable kernel tracing and read the trace buffer; a world-accessible node
+// would hand that capability to every local process. Creation fails rather
+// than falling back to a permissive mode.
+//
+// Returns an invalid fd on failure.
+base::ScopedFD CreateTracingServerSocket(const sockaddr_un& addr);
+
+// Returns whether the peer connected on |socket_fd| is allowed to use the
+// tracing service. Only peers running as the same user as the service are
+// accepted.
+//
+// This is deliberately redundant with the socket mode: the mode is applied to
+// a path under a directory this process does not own, so it can be widened by
+// a mis-configured deployment, whereas SO_PEERCRED is supplied by the kernel
+// and cannot be spoofed by the client.
+bool IsTracingClientAuthorized(int socket_fd);
+
 }  // namespace tracing
 }  // namespace chromecast
 
diff --git a/chromecast/tracing/tracing_service_main.cc b/chromecast/tracing/tracing_service_main.cc
index 91da0ea..1610f35 100644
--- a/chromecast/tracing/tracing_service_main.cc
+++ b/chromecast/tracing/tracing_service_main.cc
@@ -5,9 +5,7 @@
 #include <fcntl.h>
 #include <signal.h>
 #include <sys/socket.h>
-#include <sys/stat.h>
 #include <sys/types.h>
-#include <sys/un.h>
 
 #include <memory>
 #include <string_view>
@@ -41,37 +39,6 @@
 
 constexpr size_t kMessageSize = 4096;
 
-base::ScopedFD CreateServerSocket() {
-  struct sockaddr_un addr = GetSystemTracingSocketAddress();
-
-  if (unlink(addr.sun_path) != 0 && errno != ENOENT)
-    PLOG(ERROR) << "unlink: " << addr.sun_path;
-
-  base::ScopedFD socket_fd(
-      socket(AF_UNIX, SOCK_SEQPACKET | SOCK_NONBLOCK | SOCK_CLOEXEC, 0));
-  if (!socket_fd.is_valid()) {
-    PLOG(ERROR) << "socket";
-    return base::ScopedFD();
-  }
-
-  if (bind(socket_fd.get(), reinterpret_cast<struct sockaddr*>(&addr),
-           sizeof(addr))) {
-    PLOG(ERROR) << "bind: " << addr.sun_path;
-    return base::ScopedFD();
-  }
-
-  if (chmod(addr.sun_path, 0666))
-    PLOG(WARNING) << "chmod: " << addr.sun_path;
-
-  static constexpr int kBacklog = 10;
-  if (listen(socket_fd.get(), kBacklog)) {
-    PLOG(ERROR) << "listen: " << addr.sun_path;
-    return base::ScopedFD();
-  }
-
-  return socket_fd;
-}
-
 std::vector<std::string> ParseCategories(std::string_view message) {
   std::vector<std::string> requested_categories = base::SplitString(
       message, ",", base::KEEP_WHITESPACE, base::SPLIT_WANT_ALL);
@@ -296,7 +263,7 @@
     base::ScopedFD read_end(pipefd[0]);
     base::ScopedFD write_end(pipefd[1]);
 
-    constexpr uint8 response[] = {0};
+    constexpr uint8_t response[] = {0};
     std::vector<int> send_fds;
     send_fds.push_back(read_end.get());
     if (!base::UnixDomainSocket::SendMsg(connection_fd_.get(), response,
@@ -352,7 +319,7 @@
   ~TracingService() override {}
 
   bool Init() {
-    server_socket_ = CreateServerSocket();
+    server_socket_ = CreateTracingServerSocket(GetSystemTracingSocketAddress());
     if (!server_socket_.is_valid())
       return false;
 
@@ -379,6 +346,13 @@
       return;
     }
 
+    // Controlling this service means controlling system-wide kernel tracing,
+    // so drop connections from peers that are not allowed to do that instead
+    // of starting a session for them.
+    if (!IsTracingClientAuthorized(connection_fd.get())) {
+      return;
+    }
+
     trace_connection_ = std::make_unique<TraceConnection>(
         std::move(connection_fd),
         base::BindOnce(&TracingService::OnConnectionClosed,
diff --git a/chromecast/tracing/system_tracing_common_unittest.cc b/chromecast/tracing/system_tracing_common_unittest.cc
new file mode 100644
index 0000000..03fdc603
--- /dev/null
+++ b/chromecast/tracing/system_tracing_common_unittest.cc
@@ -0,0 +1,135 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chromecast/tracing/system_tracing_common.h"
+
+#include <sys/socket.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <sys/un.h>
+#include <unistd.h>
+
+#include "base/containers/span.h"
+#include "base/files/file_path.h"
+#include "base/files/file_util.h"
+#include "base/files/scoped_file.h"
+#include "base/files/scoped_temp_dir.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace chromecast {
+namespace tracing {
+namespace {
+
+sockaddr_un MakeSocketAddress(const base::FilePath& path) {
+  sockaddr_un addr = {};
+  addr.sun_family = AF_UNIX;
+  // |addr| is value initialized, so the remainder of sun_path stays NUL.
+  base::span(addr.sun_path).copy_prefix_from(path.value());
+  return addr;
+}
+
+// Returns the permission bits of the node at |path|, or -1 if it can't be
+// stat()ed.
+int GetMode(const base::FilePath& path) {
+  struct stat st = {};
+  if (stat(path.value().c_str(), &st)) {
+    return -1;
+  }
+  return st.st_mode & 07777;
+}
+
+class SystemTracingCommonTest : public testing::Test {
+ protected:
+  void SetUp() override {
+    ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
+    socket_path_ = temp_dir_.GetPath().Append("tracing");
+    // sun_path is a fixed size buffer; bail out clearly rather than silently
+    // truncating if the bot's temp dir is unusually deep.
+    ASSERT_LT(socket_path_.value().size(), sizeof(sockaddr_un::sun_path));
+  }
+
+  base::ScopedTempDir temp_dir_;
+  base::FilePath socket_path_;
+};
+
+// The tracing service can drive system-wide ftrace, so its socket must never
+// be reachable by arbitrary local processes. Run with a fully permissive
+// umask, which is what made the socket world-writable before.
+TEST_F(SystemTracingCommonTest, ServerSocketIsNotWorldAccessible) {
+  const mode_t old_umask = umask(0);
+  base::ScopedFD server_fd =
+      CreateTracingServerSocket(MakeSocketAddress(socket_path_));
+  umask(old_umask);
+
+  ASSERT_TRUE(server_fd.is_valid());
+  EXPECT_EQ(0660, GetMode(socket_path_));
+}
+
+// The umask is process wide, so it has to be put back.
+TEST_F(SystemTracingCommonTest, ServerSocketRestoresUmask) {
+  const mode_t old_umask = umask(022);
+  base::ScopedFD server_fd =
+      CreateTracingServerSocket(MakeSocketAddress(socket_path_));
+  const mode_t umask_after = umask(old_umask);
+
+  ASSERT_TRUE(server_fd.is_valid());
+  EXPECT_EQ(022u, umask_after);
+}
+
+// The service is restarted over a socket node left behind by a previous run.
+TEST_F(SystemTracingCommonTest, ServerSocketReplacesStaleNode) {
+  ASSERT_TRUE(base::WriteFile(socket_path_, "stale"));
+
+  base::ScopedFD server_fd =
+      CreateTracingServerSocket(MakeSocketAddress(socket_path_));
+
+  ASSERT_TRUE(server_fd.is_valid());
+  EXPECT_EQ(0660, GetMode(socket_path_));
+}
+
+TEST_F(SystemTracingCommonTest, ServerSocketFailsOnUnwritablePath) {
+  base::ScopedFD server_fd = CreateTracingServerSocket(
+      MakeSocketAddress(temp_dir_.GetPath().Append("no_such_dir/tracing")));
+
+  EXPECT_FALSE(server_fd.is_valid());
+}
+
... (file diff truncated)

diff --git a/chromecast/tracing/system_tracing_common_unittest.cc b/chromecast/tracing/system_tracing_common_unittest.cc
new file mode 100644
index 0000000..03fdc603
--- /dev/null
+++ b/chromecast/tracing/system_tracing_common_unittest.cc
@@ -0,0 +1,135 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chromecast/tracing/system_tracing_common.h"
+
+#include <sys/socket.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <sys/un.h>
+#include <unistd.h>
+
+#include "base/containers/span.h"
+#include "base/files/file_path.h"
+#include "base/files/file_util.h"
+#include "base/files/scoped_file.h"
+#include "base/files/scoped_temp_dir.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace chromecast {
+namespace tracing {
+namespace {
+
+sockaddr_un MakeSocketAddress(const base::FilePath& path) {
+  sockaddr_un addr = {};
+  addr.sun_family = AF_UNIX;
+  // |addr| is value initialized, so the remainder of sun_path stays NUL.
+  base::span(addr.sun_path).copy_prefix_from(path.value());
+  return addr;
+}
+
+// Returns the permission bits of the node at |path|, or -1 if it can't be
+// stat()ed.
+int GetMode(const base::FilePath& path) {
+  struct stat st = {};
+  if (stat(path.value().c_str(), &st)) {
+    return -1;
+  }
+  return st.st_mode & 07777;
+}
+
+class SystemTracingCommonTest : public testing::Test {
+ protected:
+  void SetUp() override {
+    ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
+    socket_path_ = temp_dir_.GetPath().Append("tracing");
+    // sun_path is a fixed size buffer; bail out clearly rather than silently
+    // truncating if the bot's temp dir is unusually deep.
+    ASSERT_LT(socket_path_.value().size(), sizeof(sockaddr_un::sun_path));
+  }
+
+  base::ScopedTempDir temp_dir_;
+  base::FilePath socket_path_;
+};
+
+// The tracing service can drive system-wide ftrace, so its socket must never
+// be reachable by arbitrary local processes. Run with a fully permissive
+// umask, which is what made the socket world-writable before.
+TEST_F(SystemTracingCommonTest, ServerSocketIsNotWorldAccessible) {
+  const mode_t old_umask = umask(0);
+  base::ScopedFD server_fd =
+      CreateTracingServerSocket(MakeSocketAddress(socket_path_));
+  umask(old_umask);
+
+  ASSERT_TRUE(server_fd.is_valid());
+  EXPECT_EQ(0660, GetMode(socket_path_));
+}
+
+// The umask is process wide, so it has to be put back.
+TEST_F(SystemTracingCommonTest, ServerSocketRestoresUmask) {
+  const mode_t old_umask = umask(022);
+  base::ScopedFD server_fd =
+      CreateTracingServerSocket(MakeSocketAddress(socket_path_));
+  const mode_t umask_after = umask(old_umask);
+
+  ASSERT_TRUE(server_fd.is_valid());
+  EXPECT_EQ(022u, umask_after);
+}
+
+// The service is restarted over a socket node left behind by a previous run.
+TEST_F(SystemTracingCommonTest, ServerSocketReplacesStaleNode) {
+  ASSERT_TRUE(base::WriteFile(socket_path_, "stale"));
+
+  base::ScopedFD server_fd =
+      CreateTracingServerSocket(MakeSocketAddress(socket_path_));
+
+  ASSERT_TRUE(server_fd.is_valid());
+  EXPECT_EQ(0660, GetMode(socket_path_));
+}
+
+TEST_F(SystemTracingCommonTest, ServerSocketFailsOnUnwritablePath) {
+  base::ScopedFD server_fd = CreateTracingServerSocket(
+      MakeSocketAddress(temp_dir_.GetPath().Append("no_such_dir/tracing")));
+
+  EXPECT_FALSE(server_fd.is_valid());
+}
+
+// End to end: a client connecting over the real listening socket is accepted
+// and authorized, since the test process is its own peer.
+TEST_F(SystemTracingCommonTest, AuthorizesPeerOnAcceptedConnection) {
+  const sockaddr_un addr = MakeSocketAddress(socket_path_);
+  base::ScopedFD server_fd = CreateTracingServerSocket(addr);
+  ASSERT_TRUE(server_fd.is_valid());
+
+  base::ScopedFD client_fd(
+      socket(AF_UNIX, SOCK_SEQPACKET | SOCK_CLOEXEC, /*protocol=*/0));
+  ASSERT_TRUE(client_fd.is_valid());
+  ASSERT_EQ(0, connect(client_fd.get(),
+                       reinterpret_cast<const struct sockaddr*>(&addr),
+                       sizeof(addr)));
+
+  base::ScopedFD connection_fd(
+      accept4(server_fd.get(), nullptr, nullptr, SOCK_CLOEXEC));
+  ASSERT_TRUE(connection_fd.is_valid());
+
+  EXPECT_TRUE(IsTracingClientAuthorized(connection_fd.get()));
+}
+
+TEST_F(SystemTracingCommonTest, AuthorizesSameUserPeer) {
+  int fds[2] = {-1, -1};
+  ASSERT_EQ(0, socketpair(AF_UNIX, SOCK_SEQPACKET, /*protocol=*/0, fds));
+  base::ScopedFD server_end(fds[0]);
+  base::ScopedFD client_end(fds[1]);
+
+  EXPECT_TRUE(IsTracingClientAuthorized(server_end.get()));
+}
+
+// Credentials that can't be read must not be treated as authorized.
+TEST_F(SystemTracingCommonTest, RejectsPeerWithoutCredentials) {
+  EXPECT_FALSE(IsTracingClientAuthorized(-1));
+}
+
+}  // namespace
+}  // namespace tracing
+}  // namespace chromecast