[llvm] [orc-rt] Reject non-socket descriptors in socket:adopt (PR #226354)

[Lang Hames via llvm-commits]

https://github.com/lhames created https://github.com/llvm/llvm-project/pull/226354

The socket connector now checks with getsockopt(SO_TYPE) that the descriptor named by a socket:adopt spec is a socket before wrapping it in a SocketHandle. Non-sockets (pipes, files, etc.) are rejected with an error and left open. Sockets are claimed after verification so that they can be closed on error paths (e.g. if GetAttachInfo fails).

Adds SocketConnectorTest.

>From 232fe8a5719ef7f21c0caf3ba0f925fd59777c40 Mon Sep 17 00:00:00 2001
From: Lang Hames <lhames at gmail.com>
Date: Fri, 25 Sep 2026 14:10:41 +1000
Subject: [PATCH] [orc-rt] Reject non-socket descriptors in socket:adopt

The socket connector now checks with getsockopt(SO_TYPE) that the
descriptor named by a socket:adopt spec is a socket before wrapping it
in a SocketHandle. Non-sockets (pipes, files, etc.) are rejected with an
error and left open. Sockets are claimed after verification so that they
can be closed on error paths (e.g. if GetAttachInfo fails).

Adds SocketConnectorTest.
---
 .../include/orc-rt/bedrock/SocketConnector.h  |  4 +
 .../lib/bedrock/sys/posix/SocketConnector.cpp | 17 +++-
 orc-rt/test/unit/CMakeLists.txt               |  1 +
 .../test/unit/bedrock/SocketConnectorTest.cpp | 89 +++++++++++++++++++
 4 files changed, 110 insertions(+), 1 deletion(-)
 create mode 100644 orc-rt/test/unit/bedrock/SocketConnectorTest.cpp

diff --git a/orc-rt/include/orc-rt/bedrock/SocketConnector.h b/orc-rt/include/orc-rt/bedrock/SocketConnector.h
index 1e28578e1ab62..ec23d17eb241d 100644
--- a/orc-rt/include/orc-rt/bedrock/SocketConnector.h
+++ b/orc-rt/include/orc-rt/bedrock/SocketConnector.h
@@ -19,6 +19,10 @@ namespace orc_rt {
 
 /// Registers the connector for the "socket" transport, whose only action is
 /// "adopt": a stream socket this process was handed, already connected.
+///
+/// If the descriptor named by the spec is a socket, the connector takes
+/// ownership of it whether or not the connection succeeds. Otherwise it is left
+/// untouched.
 Error registerSocketConnector(ConnectorRegistry &R) noexcept;
 
 } // namespace orc_rt
diff --git a/orc-rt/lib/bedrock/sys/posix/SocketConnector.cpp b/orc-rt/lib/bedrock/sys/posix/SocketConnector.cpp
index 7d98ef116f88c..1ddbd474959e0 100644
--- a/orc-rt/lib/bedrock/sys/posix/SocketConnector.cpp
+++ b/orc-rt/lib/bedrock/sys/posix/SocketConnector.cpp
@@ -13,9 +13,12 @@
 #include "orc-rt/bedrock/SocketConnector.h"
 
 #include "orc-rt-internal/support/StringExtras.h"
+#include "orc-rt-internal/support/sys/Errno.h"
 #include "orc-rt/bedrock/sps/SimpleRemoteCAOverSocket.h"
 
+#include <cerrno>
 #include <charconv>
+#include <sys/socket.h>
 
 using namespace orc_rt;
 
@@ -46,10 +49,22 @@ Error socketConnector(ConnectorRegistry::GetAttachInfoFn GetAttachInfo,
   if (FD < 0)
     return BadCS("file descriptor " + std::string(FDStr) + " is negative");
 
+  // A descriptor that is not a socket is not ours to close, so it is checked
+  // before being wrapped. One that is a socket is ours from here on, and is
+  // closed if anything below fails.
+  int Type;
+  socklen_t TypeLen = sizeof(Type);
+  if (::getsockopt(FD, SOL_SOCKET, SO_TYPE, &Type, &TypeLen) != 0) {
+    int ErrNum = errno;
+    return BadCS("file descriptor " + std::string(FDStr) +
+                 " is not a socket (" + sys::strError(ErrNum) + ")");
+  }
+  SocketHandle Sock(FD);
+
   auto AI = GetAttachInfo();
   if (!AI)
     return AI.takeError();
-  auto CA = createSimpleRemoteCAOverSocket(AI->S, SocketHandle(FD));
+  auto CA = createSimpleRemoteCAOverSocket(AI->S, std::move(Sock));
   if (!CA)
     return CA.takeError();
 
diff --git a/orc-rt/test/unit/CMakeLists.txt b/orc-rt/test/unit/CMakeLists.txt
index 6192d504db48b..a5e5f05ad2c34 100644
--- a/orc-rt/test/unit/CMakeLists.txt
+++ b/orc-rt/test/unit/CMakeLists.txt
@@ -88,6 +88,7 @@ add_orc_rt_unittest(SupportTests
 # implementations; see the note in lib/bedrock/CMakeLists.txt. Tests of portable
 # APIs stay in the shared list and reach the system through these.
 set(ORC_RT_BEDROCK_TEST_POSIX_SOURCES
+  bedrock/SocketConnectorTest.cpp
   bedrock/SocketHandleTest.cpp
   bedrock/sys/posix/SocketTestUtils.cpp
   )
diff --git a/orc-rt/test/unit/bedrock/SocketConnectorTest.cpp b/orc-rt/test/unit/bedrock/SocketConnectorTest.cpp
new file mode 100644
index 0000000000000..43d5d7784612f
--- /dev/null
+++ b/orc-rt/test/unit/bedrock/SocketConnectorTest.cpp
@@ -0,0 +1,89 @@
+//===- SocketConnectorTest.cpp --------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Tests for the socket:adopt connector's validation of its descriptor.
+//
+//===----------------------------------------------------------------------===//
+
+#include "orc-rt/bedrock/SocketConnector.h"
+
+#include "ErrorMatchers.h"
+#include "bedrock/SocketTestUtils.h"
+
+#include "gtest/gtest.h"
+
+#include <fcntl.h>
+#include <string>
+#include <unistd.h>
+
+using namespace orc_rt;
+using namespace orc_rt::test;
+
+using ::testing::HasSubstr;
+
+namespace {
+
+/// Runs "socket:adopt=<FD>" through the socket connector. The GetAttachInfo it
+/// supplies records that it was called and then fails, so a descriptor that
+/// passes validation stops there rather than being attached.
+Error adoptFD(int FD, bool &AttachInfoRequested) {
+  ConnectorRegistry R;
+  if (auto Err = registerSocketConnector(R))
+    return Err;
+  auto CS = ConnectionSpec::parse("socket:adopt=" + std::to_string(FD));
+  if (!CS)
+    return CS.takeError();
+  return R.connect(
+      [&]() noexcept -> Expected<ConnectorRegistry::AttachInfo> {
+        AttachInfoRequested = true;
+        return make_error<StringError>("attach info requested");
+      },
+      *CS);
+}
+
+bool isOpen(int FD) { return ::fcntl(FD, F_GETFD) != -1; }
+
+TEST(SocketConnectorTest, RejectsPipe) {
+  int P[2];
+  ASSERT_EQ(::pipe(P), 0);
+
+  bool AttachInfoRequested = false;
+  EXPECT_THAT_ERROR(adoptFD(P[0], AttachInfoRequested),
+                    FailedWithMessage(HasSubstr("is not a socket")));
+  EXPECT_FALSE(AttachInfoRequested);
+  EXPECT_TRUE(isOpen(P[0])) << "a rejected descriptor must be left open";
+
+  ::close(P[0]);
+  ::close(P[1]);
+}
+
+TEST(SocketConnectorTest, RejectsClosedDescriptor) {
+  int P[2];
+  ASSERT_EQ(::pipe(P), 0);
+  ::close(P[0]);
+  ::close(P[1]);
+
+  bool AttachInfoRequested = false;
+  EXPECT_THAT_ERROR(adoptFD(P[0], AttachInfoRequested),
+                    FailedWithMessage(HasSubstr("is not a socket")));
+  EXPECT_FALSE(AttachInfoRequested);
+}
+
+TEST(SocketConnectorTest, TakesOwnershipOfASocketEvenOnFailure) {
+  auto H = makeNativeSocket();
+  ASSERT_TRUE(H.has_value());
+
+  bool AttachInfoRequested = false;
+  EXPECT_THAT_ERROR(adoptFD(*H, AttachInfoRequested),
+                    FailedWithMessage("attach info requested"));
+  EXPECT_TRUE(AttachInfoRequested);
+  EXPECT_FALSE(isNativeSocketOpen(*H))
+      << "an adopted socket must be closed when the connection fails";
+}
+
+} // namespace