RGBCube/serenity
1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-07-25 19:07:35 +00:00
Code Issues Activity Actions

Kernel: Weakly hold on to the file in LocalSocket

Browse source 
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.
This commit is contained in:
sin-ack 2021-09-16 00:10:03 +00:00 • committed by Andreas Kling
parent 0ccef94a49
commit 220b7dd779
3 changed files with 26 additions and 14 deletions
Download patch file Download diff file Expand all files Collapse all files

3
Kernel/FileSystem/Inode.h
View file

@ -23,7 +23,8 @@
namespace Kernel { namespace Kernel {
class Inode : public ListedRefCounted<Inode> { class Inode : public ListedRefCounted<Inode>
, public Weakable<Inode> {
friend class VirtualFileSystem; friend class VirtualFileSystem;
friend class FileSystem; friend class FileSystem;

33
Kernel/Net/LocalSocket.cpp
View file

@ -131,12 +131,13 @@ KResult LocalSocket::bind(Userspace<const sockaddr*> user_address, socklen_t add
} }
auto file = move(result.value()); auto file = move(result.value());
auto inode = file->inode();
VERIFY(file->inode()); VERIFY(inode);
if (!file->inode()->bind_socket(*this)) if (!inode->bind_socket(*this))
return set_so_error(EADDRINUSE); return set_so_error(EADDRINUSE);
m_file = move(file); m_inode = inode;
m_path = move(path); m_path = move(path);
m_bound = true; m_bound = true;
@ -168,10 +169,12 @@ KResult LocalSocket::connect(OpenFileDescription& description, Userspace<const s
auto path = maybe_path.release_nonnull(); auto path = maybe_path.release_nonnull();
dbgln_if(LOCAL_SOCKET_DEBUG, "LocalSocket({}) connect({})", this, *path); 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()); VERIFY(inode);
if (!m_file->inode()->socket()) if (!inode->socket())
return set_so_error(ECONNREFUSED); return set_so_error(ECONNREFUSED);
m_path = move(path); m_path = move(path);
@ -179,7 +182,7 @@ KResult LocalSocket::connect(OpenFileDescription& description, Userspace<const s
VERIFY(m_connect_side_fd == &description); VERIFY(m_connect_side_fd == &description);
set_connect_side_role(Role::Connecting); set_connect_side_role(Role::Connecting);
auto peer = m_file->inode()->socket(); auto peer = file->inode()->socket();
auto result = peer->queue_connection_from(*this); auto result = peer->queue_connection_from(*this);
if (result.is_error()) { if (result.is_error()) {
set_connect_side_role(Role::None); set_connect_side_role(Role::None);
@ -244,6 +247,12 @@ void LocalSocket::detach(OpenFileDescription& description)
} else { } else {
VERIFY(m_accept_side_fd_open); VERIFY(m_accept_side_fd_open);
m_accept_side_fd_open = false; m_accept_side_fd_open = false;
if (m_bound) {
auto inode = m_inode.strong_ref();
if (inode)
inode->unbind_socket();
}
} }
evaluate_block_conditions(); evaluate_block_conditions();
@ -425,8 +434,9 @@ KResult LocalSocket::ioctl(OpenFileDescription& description, unsigned request, U
KResult LocalSocket::chmod(OpenFileDescription&, mode_t mode) KResult LocalSocket::chmod(OpenFileDescription&, mode_t mode)
{ {
if (m_file) auto inode = m_inode.strong_ref();
return m_file->chmod(mode); if (inode)
return inode->chmod(mode);
m_prebind_mode = mode & 0777; m_prebind_mode = mode & 0777;
return KSuccess; return KSuccess;
@ -434,8 +444,9 @@ KResult LocalSocket::chmod(OpenFileDescription&, mode_t mode)
KResult LocalSocket::chown(OpenFileDescription&, UserID uid, GroupID gid) KResult LocalSocket::chown(OpenFileDescription&, UserID uid, GroupID gid)
{ {
if (m_file) auto inode = m_inode.strong_ref();
return m_file->chown(uid, gid); if (inode)
return inode->chown(uid, gid);
auto& current_process = Process::current(); auto& current_process = Process::current();
if (!current_process.is_superuser() && (current_process.euid() != uid || !current_process.in_group(gid))) if (!current_process.is_superuser() && (current_process.euid() != uid || !current_process.in_group(gid)))

4
Kernel/Net/LocalSocket.h
View file

@ -71,8 +71,8 @@ private:
KResult try_set_path(StringView); KResult try_set_path(StringView);
// An open socket file on the filesystem. // The inode this socket is bound to.
RefPtr<OpenFileDescription> m_file; WeakPtr<Inode> m_inode;
UserID m_prebind_uid { 0 }; UserID m_prebind_uid { 0 };
GroupID m_prebind_gid { 0 }; GroupID m_prebind_gid { 0 };