From 4aa0ab4e0885922570f95ddfb86648f2120f823c Mon Sep 17 00:00:00 2001
From: Andreas Kling <awesomekling@gmail.com>
Date: Wed, 20 Mar 2019 02:33:51 +0100
Subject: [PATCH] Kernel: Fix race between accept() and connect().

We had a bug where an accepted socket would appear to be EOF/disconnected
on the accepting side until the connecting side had called attach_fd().

Fix this by adding a new SocketRole::Connecting state.
---
 Kernel/FileDescriptor.cpp |  3 +++
 Kernel/LocalSocket.cpp    | 11 ++++++++---
 Kernel/LocalSocket.h      |  1 +
 Kernel/Process.cpp        |  1 +
 Kernel/Socket.h           |  2 +-
 5 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/Kernel/FileDescriptor.cpp b/Kernel/FileDescriptor.cpp
index a678afdfc3..5aea6d181f 100644
--- a/Kernel/FileDescriptor.cpp
+++ b/Kernel/FileDescriptor.cpp
@@ -76,8 +76,11 @@ void FileDescriptor::set_socket_role(SocketRole role)
         return;
 
     ASSERT(m_socket);
+    auto old_socket_role = m_socket_role;
     m_socket_role = role;
     m_socket->attach_fd(role);
+    if (old_socket_role != SocketRole::None)
+        m_socket->detach_fd(old_socket_role);
 }
 
 Retained<FileDescriptor> FileDescriptor::clone()
diff --git a/Kernel/LocalSocket.cpp b/Kernel/LocalSocket.cpp
index d79b1badac..1f7209bdd5 100644
--- a/Kernel/LocalSocket.cpp
+++ b/Kernel/LocalSocket.cpp
@@ -106,6 +106,8 @@ void LocalSocket::attach_fd(SocketRole role)
         ++m_accepted_fds_open;
     } else if (role == SocketRole::Connected) {
         ++m_connected_fds_open;
+    } else if (role == SocketRole::Connecting) {
+        ++m_connecting_fds_open;
     }
 }
 
@@ -117,6 +119,9 @@ void LocalSocket::detach_fd(SocketRole role)
     } else if (role == SocketRole::Connected) {
         ASSERT(m_connected_fds_open);
         --m_connected_fds_open;
+    } else if (role == SocketRole::Connecting) {
+        ASSERT(m_connecting_fds_open);
+        --m_connecting_fds_open;
     }
 }
 
@@ -125,7 +130,7 @@ bool LocalSocket::can_read(SocketRole role) const
     if (role == SocketRole::Listener)
         return can_accept();
     if (role == SocketRole::Accepted)
-        return !m_connected_fds_open || !m_for_server.is_empty();
+        return (!m_connected_fds_open && !m_connecting_fds_open) || !m_for_server.is_empty();
     if (role == SocketRole::Connected)
         return !m_accepted_fds_open || !m_for_client.is_empty();
     ASSERT_NOT_REACHED();
@@ -148,7 +153,7 @@ ssize_t LocalSocket::write(SocketRole role, const byte* data, ssize_t size)
         return m_for_client.write(data, size);
     }
     if (role == SocketRole::Connected) {
-        if (!m_connected_fds_open)
+        if (!m_connected_fds_open && !m_connecting_fds_open)
             return -EPIPE;
         return m_for_server.write(data, size);
     }
@@ -158,7 +163,7 @@ ssize_t LocalSocket::write(SocketRole role, const byte* data, ssize_t size)
 bool LocalSocket::can_write(SocketRole role) const
 {
     if (role == SocketRole::Accepted)
-        return !m_connected_fds_open || m_for_client.bytes_in_write_buffer() < 4096;
+        return (!m_connected_fds_open && !m_connecting_fds_open) || m_for_client.bytes_in_write_buffer() < 4096;
     if (role == SocketRole::Connected)
         return !m_accepted_fds_open || m_for_server.bytes_in_write_buffer() < 4096;
     ASSERT_NOT_REACHED();
diff --git a/Kernel/LocalSocket.h b/Kernel/LocalSocket.h
index f8329cd6eb..4d65391d10 100644
--- a/Kernel/LocalSocket.h
+++ b/Kernel/LocalSocket.h
@@ -32,6 +32,7 @@ private:
     bool m_bound { false };
     int m_accepted_fds_open { 0 };
     int m_connected_fds_open { 0 };
+    int m_connecting_fds_open { 0 };
     sockaddr_un m_address;
 
     DoubleBuffer m_for_client;
diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp
index cef0801b03..a7051c88cb 100644
--- a/Kernel/Process.cpp
+++ b/Kernel/Process.cpp
@@ -2503,6 +2503,7 @@ int Process::sys$connect(int sockfd, const sockaddr* address, socklen_t address_
     if (!descriptor->is_socket())
         return -ENOTSOCK;
     auto& socket = *descriptor->socket();
+    descriptor->set_socket_role(SocketRole::Connecting);
     auto result = socket.connect(address, address_size);
     if (result.is_error())
         return result;
diff --git a/Kernel/Socket.h b/Kernel/Socket.h
index a24fc896ff..2b8866a3f5 100644
--- a/Kernel/Socket.h
+++ b/Kernel/Socket.h
@@ -8,7 +8,7 @@
 #include <Kernel/UnixTypes.h>
 #include <Kernel/KResult.h>
 
-enum class SocketRole { None, Listener, Accepted, Connected };
+enum class SocketRole { None, Listener, Accepted, Connected, Connecting };
 
 class Socket : public Retainable<Socket> {
 public: