From 220b7dd77905b7d573ded093cf88d2dc51f57c69 Mon Sep 17 00:00:00 2001
From: sin-ack <sin-ack@users.noreply.github.com>
Date: Thu, 16 Sep 2021 00:10:03 +0000
Subject: [PATCH] Kernel: Weakly hold on to the file in LocalSocket

Because we were holding a strong ref to the OpenFileDescription in
LocalSocket and a strong ref to the LocalSocket in Inode, we were
creating a reference cycle in the event of the socket being cleaned up
after the file description did (i.e. unlinking the file before closing
the socket), because the file description never got destructed.
---
 Kernel/FileSystem/Inode.h  |  3 ++-
 Kernel/Net/LocalSocket.cpp | 33 ++++++++++++++++++++++-----------
 Kernel/Net/LocalSocket.h   |  4 ++--
 3 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/Kernel/FileSystem/Inode.h b/Kernel/FileSystem/Inode.h
index c078d83608..971a2f57ac 100644
--- a/Kernel/FileSystem/Inode.h
+++ b/Kernel/FileSystem/Inode.h
@@ -23,7 +23,8 @@
 
 namespace Kernel {
 
-class Inode : public ListedRefCounted<Inode> {
+class Inode : public ListedRefCounted<Inode>
+    , public Weakable<Inode> {
     friend class VirtualFileSystem;
     friend class FileSystem;
 
diff --git a/Kernel/Net/LocalSocket.cpp b/Kernel/Net/LocalSocket.cpp
index f5dc0cc9f4..74dd109598 100644
--- a/Kernel/Net/LocalSocket.cpp
+++ b/Kernel/Net/LocalSocket.cpp
@@ -131,12 +131,13 @@ KResult LocalSocket::bind(Userspace<const sockaddr*> user_address, socklen_t add
     }
 
     auto file = move(result.value());
+    auto inode = file->inode();
 
-    VERIFY(file->inode());
-    if (!file->inode()->bind_socket(*this))
+    VERIFY(inode);
+    if (!inode->bind_socket(*this))
         return set_so_error(EADDRINUSE);
 
-    m_file = move(file);
+    m_inode = inode;
 
     m_path = move(path);
     m_bound = true;
@@ -168,10 +169,12 @@ KResult LocalSocket::connect(OpenFileDescription& description, Userspace<const s
     auto path = maybe_path.release_nonnull();
     dbgln_if(LOCAL_SOCKET_DEBUG, "LocalSocket({}) connect({})", this, *path);
 
-    m_file = SOCKET_TRY(VirtualFileSystem::the().open(path->view(), O_RDWR, 0, Process::current().current_directory()));
+    auto file = SOCKET_TRY(VirtualFileSystem::the().open(path->view(), O_RDWR, 0, Process::current().current_directory()));
+    auto inode = file->inode();
+    m_inode = inode;
 
-    VERIFY(m_file->inode());
-    if (!m_file->inode()->socket())
+    VERIFY(inode);
+    if (!inode->socket())
         return set_so_error(ECONNREFUSED);
 
     m_path = move(path);
@@ -179,7 +182,7 @@ KResult LocalSocket::connect(OpenFileDescription& description, Userspace<const s
     VERIFY(m_connect_side_fd == &description);
     set_connect_side_role(Role::Connecting);
 
-    auto peer = m_file->inode()->socket();
+    auto peer = file->inode()->socket();
     auto result = peer->queue_connection_from(*this);
     if (result.is_error()) {
         set_connect_side_role(Role::None);
@@ -244,6 +247,12 @@ void LocalSocket::detach(OpenFileDescription& description)
     } else {
         VERIFY(m_accept_side_fd_open);
         m_accept_side_fd_open = false;
+
+        if (m_bound) {
+            auto inode = m_inode.strong_ref();
+            if (inode)
+                inode->unbind_socket();
+        }
     }
 
     evaluate_block_conditions();
@@ -425,8 +434,9 @@ KResult LocalSocket::ioctl(OpenFileDescription& description, unsigned request, U
 
 KResult LocalSocket::chmod(OpenFileDescription&, mode_t mode)
 {
-    if (m_file)
-        return m_file->chmod(mode);
+    auto inode = m_inode.strong_ref();
+    if (inode)
+        return inode->chmod(mode);
 
     m_prebind_mode = mode & 0777;
     return KSuccess;
@@ -434,8 +444,9 @@ KResult LocalSocket::chmod(OpenFileDescription&, mode_t mode)
 
 KResult LocalSocket::chown(OpenFileDescription&, UserID uid, GroupID gid)
 {
-    if (m_file)
-        return m_file->chown(uid, gid);
+    auto inode = m_inode.strong_ref();
+    if (inode)
+        return inode->chown(uid, gid);
 
     auto& current_process = Process::current();
     if (!current_process.is_superuser() && (current_process.euid() != uid || !current_process.in_group(gid)))
diff --git a/Kernel/Net/LocalSocket.h b/Kernel/Net/LocalSocket.h
index f1327944e0..00e8a4b4ff 100644
--- a/Kernel/Net/LocalSocket.h
+++ b/Kernel/Net/LocalSocket.h
@@ -71,8 +71,8 @@ private:
 
     KResult try_set_path(StringView);
 
-    // An open socket file on the filesystem.
-    RefPtr<OpenFileDescription> m_file;
+    // The inode this socket is bound to.
+    WeakPtr<Inode> m_inode;
 
     UserID m_prebind_uid { 0 };
     GroupID m_prebind_gid { 0 };