summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAndreas Kling <kling@serenityos.org>2022-08-21 01:04:35 +0200
committerAndreas Kling <kling@serenityos.org>2022-08-21 12:25:14 +0200
commit728c3fbd14252bd746f97dbb5441063992074b6b (patch)
tree539c32ede90702ab3ef35b984addad93769db340
parent5331d243c690c70e431e2f8d260eacab19946c2b (diff)
downloadserenity-728c3fbd14252bd746f97dbb5441063992074b6b.zip
Kernel: Use RefPtr instead of LockRefPtr for Custody
By protecting all the RefPtr<Custody> objects that may be accessed from multiple threads at the same time (with spinlocks), we remove the need for using LockRefPtr<Custody> (which is basically a RefPtr with a built-in spinlock.)
-rw-r--r--Kernel/FileSystem/Custody.cpp9
-rw-r--r--Kernel/FileSystem/Custody.h12
-rw-r--r--Kernel/FileSystem/Inode.cpp2
-rw-r--r--Kernel/FileSystem/Inode.h2
-rw-r--r--Kernel/FileSystem/Mount.cpp32
-rw-r--r--Kernel/FileSystem/Mount.h7
-rw-r--r--Kernel/FileSystem/OpenFileDescription.cpp24
-rw-r--r--Kernel/FileSystem/OpenFileDescription.h7
-rw-r--r--Kernel/FileSystem/VirtualFileSystem.cpp71
-rw-r--r--Kernel/FileSystem/VirtualFileSystem.h14
-rw-r--r--Kernel/Process.cpp22
-rw-r--r--Kernel/Process.h14
-rw-r--r--Kernel/ProcessSpecificExposed.cpp2
-rw-r--r--Kernel/Syscalls/chdir.cpp5
-rw-r--r--Kernel/Syscalls/chmod.cpp2
-rw-r--r--Kernel/Syscalls/chown.cpp3
-rw-r--r--Kernel/Syscalls/execve.cpp2
-rw-r--r--Kernel/Syscalls/fork.cpp2
-rw-r--r--Kernel/Syscalls/open.cpp3
-rw-r--r--Kernel/Syscalls/stat.cpp3
-rw-r--r--Kernel/Syscalls/unlink.cpp2
-rw-r--r--Kernel/Syscalls/unveil.cpp3
-rw-r--r--Kernel/Syscalls/utimensat.cpp2
23 files changed, 143 insertions, 102 deletions
diff --git a/Kernel/FileSystem/Custody.cpp b/Kernel/FileSystem/Custody.cpp
index 8166cc15f4..0c40a26197 100644
--- a/Kernel/FileSystem/Custody.cpp
+++ b/Kernel/FileSystem/Custody.cpp
@@ -4,6 +4,7 @@
* SPDX-License-Identifier: BSD-2-Clause
*/
+#include <AK/RefPtr.h>
#include <AK/Singleton.h>
#include <AK/StringBuilder.h>
#include <AK/StringView.h>
@@ -20,20 +21,20 @@ SpinlockProtected<Custody::AllCustodiesList>& Custody::all_instances()
return s_all_instances;
}
-ErrorOr<NonnullLockRefPtr<Custody>> Custody::try_create(Custody* parent, StringView name, Inode& inode, int mount_flags)
+ErrorOr<NonnullRefPtr<Custody>> Custody::try_create(Custody* parent, StringView name, Inode& inode, int mount_flags)
{
- return all_instances().with([&](auto& all_custodies) -> ErrorOr<NonnullLockRefPtr<Custody>> {
+ return all_instances().with([&](auto& all_custodies) -> ErrorOr<NonnullRefPtr<Custody>> {
for (Custody& custody : all_custodies) {
if (custody.parent() == parent
&& custody.name() == name
&& &custody.inode() == &inode
&& custody.mount_flags() == mount_flags) {
- return NonnullLockRefPtr { custody };
+ return NonnullRefPtr { custody };
}
}
auto name_kstring = TRY(KString::try_create(name));
- auto custody = TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) Custody(parent, move(name_kstring), inode, mount_flags)));
+ auto custody = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) Custody(parent, move(name_kstring), inode, mount_flags)));
all_custodies.prepend(*custody);
return custody;
});
diff --git a/Kernel/FileSystem/Custody.h b/Kernel/FileSystem/Custody.h
index 26f2e277b6..f72cf071c6 100644
--- a/Kernel/FileSystem/Custody.h
+++ b/Kernel/FileSystem/Custody.h
@@ -8,22 +8,22 @@
#include <AK/Error.h>
#include <AK/IntrusiveList.h>
+#include <AK/RefPtr.h>
#include <Kernel/Forward.h>
#include <Kernel/KString.h>
#include <Kernel/Library/ListedRefCounted.h>
-#include <Kernel/Library/LockRefPtr.h>
#include <Kernel/Locking/SpinlockProtected.h>
namespace Kernel {
-class Custody : public ListedRefCounted<Custody, LockType::Spinlock> {
+class Custody final : public ListedRefCounted<Custody, LockType::Spinlock> {
public:
- static ErrorOr<NonnullLockRefPtr<Custody>> try_create(Custody* parent, StringView name, Inode&, int mount_flags);
+ static ErrorOr<NonnullRefPtr<Custody>> try_create(Custody* parent, StringView name, Inode&, int mount_flags);
~Custody();
- Custody* parent() { return m_parent.ptr(); }
- Custody const* parent() const { return m_parent.ptr(); }
+ RefPtr<Custody> parent() { return m_parent; }
+ RefPtr<Custody const> parent() const { return m_parent; }
Inode& inode() { return *m_inode; }
Inode const& inode() const { return *m_inode; }
StringView name() const { return m_name->view(); }
@@ -35,7 +35,7 @@ public:
private:
Custody(Custody* parent, NonnullOwnPtr<KString> name, Inode&, int mount_flags);
- LockRefPtr<Custody> m_parent;
+ RefPtr<Custody> m_parent;
NonnullOwnPtr<KString> m_name;
NonnullLockRefPtr<Inode> m_inode;
int m_mount_flags { 0 };
diff --git a/Kernel/FileSystem/Inode.cpp b/Kernel/FileSystem/Inode.cpp
index e0bf3d5955..108b47aee7 100644
--- a/Kernel/FileSystem/Inode.cpp
+++ b/Kernel/FileSystem/Inode.cpp
@@ -76,7 +76,7 @@ ErrorOr<NonnullOwnPtr<KBuffer>> Inode::read_entire(OpenFileDescription* descript
return entire_file.release_nonnull();
}
-ErrorOr<NonnullLockRefPtr<Custody>> Inode::resolve_as_link(Custody& base, LockRefPtr<Custody>* out_parent, int options, int symlink_recursion_level) const
+ErrorOr<NonnullRefPtr<Custody>> Inode::resolve_as_link(Custody& base, RefPtr<Custody>* out_parent, int options, int symlink_recursion_level) const
{
// The default implementation simply treats the stored
// contents as a path and resolves that. That is, it
diff --git a/Kernel/FileSystem/Inode.h b/Kernel/FileSystem/Inode.h
index 20c2ae6c49..155a8e2567 100644
--- a/Kernel/FileSystem/Inode.h
+++ b/Kernel/FileSystem/Inode.h
@@ -69,7 +69,7 @@ public:
virtual ErrorOr<void> chmod(mode_t) = 0;
virtual ErrorOr<void> chown(UserID, GroupID) = 0;
virtual ErrorOr<void> truncate(u64) { return {}; }
- virtual ErrorOr<NonnullLockRefPtr<Custody>> resolve_as_link(Custody& base, LockRefPtr<Custody>* out_parent, int options, int symlink_recursion_level) const;
+ virtual ErrorOr<NonnullRefPtr<Custody>> resolve_as_link(Custody& base, RefPtr<Custody>* out_parent, int options, int symlink_recursion_level) const;
virtual ErrorOr<int> get_block_address(int) { return ENOTSUP; }
diff --git a/Kernel/FileSystem/Mount.cpp b/Kernel/FileSystem/Mount.cpp
index e934d2eb71..733147d746 100644
--- a/Kernel/FileSystem/Mount.cpp
+++ b/Kernel/FileSystem/Mount.cpp
@@ -14,7 +14,7 @@ namespace Kernel {
Mount::Mount(FileSystem& guest_fs, Custody* host_custody, int flags)
: m_guest(guest_fs.root_inode())
, m_guest_fs(guest_fs)
- , m_host_custody(host_custody)
+ , m_host_custody(LockRank::None, host_custody)
, m_flags(flags)
{
}
@@ -22,30 +22,36 @@ Mount::Mount(FileSystem& guest_fs, Custody* host_custody, int flags)
Mount::Mount(Inode& source, Custody& host_custody, int flags)
: m_guest(source)
, m_guest_fs(source.fs())
- , m_host_custody(host_custody)
+ , m_host_custody(LockRank::None, host_custody)
, m_flags(flags)
{
}
ErrorOr<NonnullOwnPtr<KString>> Mount::absolute_path() const
{
- if (!m_host_custody)
- return KString::try_create("/"sv);
- return m_host_custody->try_serialize_absolute_path();
+ return m_host_custody.with([&](auto& host_custody) -> ErrorOr<NonnullOwnPtr<KString>> {
+ if (!host_custody)
+ return KString::try_create("/"sv);
+ return host_custody->try_serialize_absolute_path();
+ });
}
-Inode* Mount::host()
+LockRefPtr<Inode> Mount::host()
{
- if (!m_host_custody)
- return nullptr;
- return &m_host_custody->inode();
+ return m_host_custody.with([](auto& host_custody) -> LockRefPtr<Inode> {
+ if (!host_custody)
+ return nullptr;
+ return &host_custody->inode();
+ });
}
-Inode const* Mount::host() const
+LockRefPtr<Inode const> Mount::host() const
{
- if (!m_host_custody)
- return nullptr;
- return &m_host_custody->inode();
+ return m_host_custody.with([](auto& host_custody) -> LockRefPtr<Inode const> {
+ if (!host_custody)
+ return nullptr;
+ return &host_custody->inode();
+ });
}
}
diff --git a/Kernel/FileSystem/Mount.h b/Kernel/FileSystem/Mount.h
index 89f59a5925..de49c64caa 100644
--- a/Kernel/FileSystem/Mount.h
+++ b/Kernel/FileSystem/Mount.h
@@ -6,6 +6,7 @@
#pragma once
+#include <AK/RefPtr.h>
#include <Kernel/FileSystem/Custody.h>
#include <Kernel/Forward.h>
#include <Kernel/Library/NonnullLockRefPtr.h>
@@ -17,8 +18,8 @@ public:
Mount(FileSystem&, Custody* host_custody, int flags);
Mount(Inode& source, Custody& host_custody, int flags);
- Inode const* host() const;
- Inode* host();
+ LockRefPtr<Inode const> host() const;
+ LockRefPtr<Inode> host();
Inode const& guest() const { return *m_guest; }
Inode& guest() { return *m_guest; }
@@ -34,7 +35,7 @@ public:
private:
NonnullLockRefPtr<Inode> m_guest;
NonnullLockRefPtr<FileSystem> m_guest_fs;
- LockRefPtr<Custody> m_host_custody;
+ SpinlockProtected<RefPtr<Custody>> m_host_custody;
int m_flags;
};
diff --git a/Kernel/FileSystem/OpenFileDescription.cpp b/Kernel/FileSystem/OpenFileDescription.cpp
index b7a1c5ecd2..f7405d52a9 100644
--- a/Kernel/FileSystem/OpenFileDescription.cpp
+++ b/Kernel/FileSystem/OpenFileDescription.cpp
@@ -27,7 +27,7 @@ ErrorOr<NonnullLockRefPtr<OpenFileDescription>> OpenFileDescription::try_create(
auto inode_file = TRY(InodeFile::create(custody.inode()));
auto description = TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) OpenFileDescription(move(inode_file))));
- description->m_custody = custody;
+ description->m_state.with([&](auto& state) { state.custody = custody; });
TRY(description->attach());
return description;
}
@@ -72,7 +72,7 @@ ErrorOr<void> OpenFileDescription::attach()
void OpenFileDescription::set_original_custody(Badge<VirtualFileSystem>, Custody& custody)
{
- m_custody = custody;
+ m_state.with([&](auto& state) { state.custody = custody; });
}
Thread::FileBlocker::BlockFlags OpenFileDescription::should_unblock(Thread::FileBlocker::BlockFlags block_flags) const
@@ -355,15 +355,15 @@ ErrorOr<void> OpenFileDescription::close()
ErrorOr<NonnullOwnPtr<KString>> OpenFileDescription::original_absolute_path() const
{
- if (!m_custody)
- return ENOENT;
- return m_custody->try_serialize_absolute_path();
+ if (auto custody = this->custody())
+ return custody->try_serialize_absolute_path();
+ return ENOENT;
}
ErrorOr<NonnullOwnPtr<KString>> OpenFileDescription::pseudo_path() const
{
- if (m_custody)
- return m_custody->try_serialize_absolute_path();
+ if (auto custody = this->custody())
+ return custody->try_serialize_absolute_path();
return m_file->pseudo_path(*this);
}
@@ -538,4 +538,14 @@ off_t OpenFileDescription::offset() const
return m_state.with([](auto& state) { return state.current_offset; });
}
+RefPtr<Custody const> OpenFileDescription::custody() const
+{
+ return m_state.with([](auto& state) { return state.custody; });
+}
+
+RefPtr<Custody> OpenFileDescription::custody()
+{
+ return m_state.with([](auto& state) { return state.custody; });
+}
+
}
diff --git a/Kernel/FileSystem/OpenFileDescription.h b/Kernel/FileSystem/OpenFileDescription.h
index fc513f56cb..c4cdae2d49 100644
--- a/Kernel/FileSystem/OpenFileDescription.h
+++ b/Kernel/FileSystem/OpenFileDescription.h
@@ -8,6 +8,7 @@
#include <AK/AtomicRefCounted.h>
#include <AK/Badge.h>
+#include <AK/RefPtr.h>
#include <Kernel/FileSystem/FIFO.h>
#include <Kernel/FileSystem/Inode.h>
#include <Kernel/FileSystem/InodeMetadata.h>
@@ -88,8 +89,8 @@ public:
Inode* inode() { return m_inode.ptr(); }
Inode const* inode() const { return m_inode.ptr(); }
- Custody* custody() { return m_custody.ptr(); }
- Custody const* custody() const { return m_custody.ptr(); }
+ RefPtr<Custody> custody();
+ RefPtr<Custody const> custody() const;
ErrorOr<Memory::Region*> mmap(Process&, Memory::VirtualRange const&, u64 offset, int prot, bool shared);
@@ -138,12 +139,12 @@ private:
blocker_set().unblock_all_blockers_whose_conditions_are_met();
}
- LockRefPtr<Custody> m_custody;
LockRefPtr<Inode> m_inode;
NonnullLockRefPtr<File> m_file;
struct State {
OwnPtr<OpenFileDescriptionData> data;
+ RefPtr<Custody> custody;
off_t current_offset { 0 };
u32 file_flags { 0 };
bool readable : 1 { false };
diff --git a/Kernel/FileSystem/VirtualFileSystem.cpp b/Kernel/FileSystem/VirtualFileSystem.cpp
index d995c29299..5b426e3c46 100644
--- a/Kernel/FileSystem/VirtualFileSystem.cpp
+++ b/Kernel/FileSystem/VirtualFileSystem.cpp
@@ -6,6 +6,7 @@
#include <AK/AnyOf.h>
#include <AK/GenericLexer.h>
+#include <AK/RefPtr.h>
#include <AK/Singleton.h>
#include <AK/StringBuilder.h>
#include <Kernel/API/POSIX/errno.h>
@@ -38,6 +39,7 @@ VirtualFileSystem& VirtualFileSystem::the()
}
UNMAP_AFTER_INIT VirtualFileSystem::VirtualFileSystem()
+ : m_root_custody(LockRank::None)
{
}
@@ -52,14 +54,15 @@ InodeIdentifier VirtualFileSystem::root_inode_id() const
bool VirtualFileSystem::mount_point_exists_at_inode(InodeIdentifier inode_identifier)
{
return m_mounts.with([&](auto& mounts) -> bool {
- return any_of(mounts, [&inode_identifier](Mount const& existing_mount) {
- return existing_mount.host() && existing_mount.host()->identifier() == inode_identifier;
+ return any_of(mounts, [&inode_identifier](auto const& existing_mount) {
+ return existing_mount->host() && existing_mount->host()->identifier() == inode_identifier;
});
});
}
ErrorOr<void> VirtualFileSystem::mount(FileSystem& fs, Custody& mount_point, int flags)
{
+ auto new_mount = TRY(adopt_nonnull_own_or_enomem(new (nothrow) Mount(fs, &mount_point, flags)));
return m_mounts.with([&](auto& mounts) -> ErrorOr<void> {
auto& inode = mount_point.inode();
dbgln("VirtualFileSystem: Mounting {} at inode {} with flags {}",
@@ -70,14 +73,14 @@ ErrorOr<void> VirtualFileSystem::mount(FileSystem& fs, Custody& mount_point, int
dbgln("VirtualFileSystem: Mounting unsuccessful - inode {} is already a mount-point.", inode.identifier());
return EBUSY;
}
- Mount mount { fs, &mount_point, flags };
- mounts.append(move(mount));
+ mounts.append(move(new_mount));
return {};
});
}
ErrorOr<void> VirtualFileSystem::bind_mount(Custody& source, Custody& mount_point, int flags)
{
+ auto new_mount = TRY(adopt_nonnull_own_or_enomem(new (nothrow) Mount(source.inode(), mount_point, flags)));
return m_mounts.with([&](auto& mounts) -> ErrorOr<void> {
auto& inode = mount_point.inode();
dbgln("VirtualFileSystem: Bind-mounting inode {} at inode {}", source.inode().identifier(), inode.identifier());
@@ -86,8 +89,7 @@ ErrorOr<void> VirtualFileSystem::bind_mount(Custody& source, Custody& mount_poin
mount_point.inode().identifier());
return EBUSY;
}
- Mount mount { source.inode(), mount_point, flags };
- mounts.append(move(mount));
+ mounts.append(move(new_mount));
return {};
});
}
@@ -111,11 +113,11 @@ ErrorOr<void> VirtualFileSystem::unmount(Inode& guest_inode)
return m_mounts.with([&](auto& mounts) -> ErrorOr<void> {
for (size_t i = 0; i < mounts.size(); ++i) {
auto& mount = mounts[i];
- if (&mount.guest() != &guest_inode)
+ if (&mount->guest() != &guest_inode)
continue;
- TRY(mount.guest_fs().prepare_to_unmount());
- dbgln("VirtualFileSystem: Unmounting file system {}...", mount.guest_fs().fsid());
- mounts.unstable_take(i);
+ TRY(mount->guest_fs().prepare_to_unmount());
+ dbgln("VirtualFileSystem: Unmounting file system {}...", mount->guest_fs().fsid());
+ (void)mounts.unstable_take(i);
return {};
}
dbgln("VirtualFileSystem: Nothing mounted on inode {}", guest_inode.identifier());
@@ -130,7 +132,7 @@ ErrorOr<void> VirtualFileSystem::mount_root(FileSystem& fs)
return EEXIST;
}
- Mount mount { fs, nullptr, root_mount_flags };
+ auto new_mount = TRY(adopt_nonnull_own_or_enomem(new (nothrow) Mount(fs, nullptr, root_mount_flags)));
auto& root_inode = fs.root_inode();
if (!root_inode.is_directory()) {
@@ -143,10 +145,13 @@ ErrorOr<void> VirtualFileSystem::mount_root(FileSystem& fs)
dmesgln("VirtualFileSystem: mounted root from {} ({})", fs.class_name(), pseudo_path);
m_mounts.with([&](auto& mounts) {
- mounts.append(move(mount));
+ mounts.append(move(new_mount));
});
- m_root_custody = TRY(Custody::try_create(nullptr, ""sv, *m_root_inode, root_mount_flags));
+ RefPtr<Custody> new_root_custody = TRY(Custody::try_create(nullptr, ""sv, *m_root_inode, root_mount_flags));
+ m_root_custody.with([&](auto& root_custody) {
+ swap(root_custody, new_root_custody);
+ });
return {};
}
@@ -154,8 +159,8 @@ auto VirtualFileSystem::find_mount_for_host(InodeIdentifier id) -> Mount*
{
return m_mounts.with([&](auto& mounts) -> Mount* {
for (auto& mount : mounts) {
- if (mount.host() && mount.host()->identifier() == id)
- return &mount;
+ if (mount->host() && mount->host()->identifier() == id)
+ return mount.ptr();
}
return nullptr;
});
@@ -165,8 +170,8 @@ auto VirtualFileSystem::find_mount_for_guest(InodeIdentifier id) -> Mount*
{
return m_mounts.with([&](auto& mounts) -> Mount* {
for (auto& mount : mounts) {
- if (mount.guest().identifier() == id)
- return &mount;
+ if (mount->guest().identifier() == id)
+ return mount.ptr();
}
return nullptr;
});
@@ -244,7 +249,7 @@ ErrorOr<NonnullLockRefPtr<OpenFileDescription>> VirtualFileSystem::open(StringVi
if ((options & O_CREAT) && (options & O_DIRECTORY))
return EINVAL;
- LockRefPtr<Custody> parent_custody;
+ RefPtr<Custody> parent_custody;
auto custody_or_error = resolve_path(path, base, &parent_custody, options);
if (custody_or_error.is_error()) {
// NOTE: ENOENT with a non-null parent custody signals us that the immediate parent
@@ -332,7 +337,7 @@ ErrorOr<void> VirtualFileSystem::mknod(StringView path, mode_t mode, dev_t dev,
if (!is_regular_file(mode) && !is_block_device(mode) && !is_character_device(mode) && !is_fifo(mode) && !is_socket(mode))
return EINVAL;
- LockRefPtr<Custody> parent_custody;
+ RefPtr<Custody> parent_custody;
auto existing_file_or_error = resolve_path(path, base, &parent_custody);
if (!existing_file_or_error.is_error())
return EEXIST;
@@ -397,7 +402,7 @@ ErrorOr<void> VirtualFileSystem::mkdir(StringView path, mode_t mode, Custody& ba
path = "/"sv;
}
- LockRefPtr<Custody> parent_custody;
+ RefPtr<Custody> parent_custody;
// FIXME: The errors returned by resolve_path_without_veil can leak information about paths that are not unveiled,
// e.g. when the error is EACCESS or similar.
auto result = resolve_path_without_veil(path, base, &parent_custody);
@@ -446,7 +451,7 @@ ErrorOr<void> VirtualFileSystem::access(StringView path, int mode, Custody& base
return {};
}
-ErrorOr<NonnullLockRefPtr<Custody>> VirtualFileSystem::open_directory(StringView path, Custody& base)
+ErrorOr<NonnullRefPtr<Custody>> VirtualFileSystem::open_directory(StringView path, Custody& base)
{
auto custody = TRY(resolve_path(path, base));
auto& inode = custody->inode();
@@ -480,11 +485,11 @@ ErrorOr<void> VirtualFileSystem::chmod(StringView path, mode_t mode, Custody& ba
ErrorOr<void> VirtualFileSystem::rename(StringView old_path, StringView new_path, Custody& base)
{
- LockRefPtr<Custody> old_parent_custody;
+ RefPtr<Custody> old_parent_custody;
auto old_custody = TRY(resolve_path(old_path, base, &old_parent_custody, O_NOFOLLOW_NOERROR));
auto& old_inode = old_custody->inode();
- LockRefPtr<Custody> new_parent_custody;
+ RefPtr<Custody> new_parent_custody;
auto new_custody_or_error = resolve_path(new_path, base, &new_parent_custody);
if (new_custody_or_error.is_error()) {
if (new_custody_or_error.error().code() != ENOENT || !new_parent_custody)
@@ -630,7 +635,7 @@ ErrorOr<void> VirtualFileSystem::link(StringView old_path, StringView new_path,
auto old_custody = TRY(resolve_path(old_path, base));
auto& old_inode = old_custody->inode();
- LockRefPtr<Custody> parent_custody;
+ RefPtr<Custody> parent_custody;
auto new_custody_or_error = resolve_path(new_path, base, &parent_custody);
if (!new_custody_or_error.is_error())
return EEXIST;
@@ -660,7 +665,7 @@ ErrorOr<void> VirtualFileSystem::link(StringView old_path, StringView new_path,
ErrorOr<void> VirtualFileSystem::unlink(StringView path, Custody& base)
{
- LockRefPtr<Custody> parent_custody;
+ RefPtr<Custody> parent_custody;
auto custody = TRY(resolve_path(path, base, &parent_custody, O_NOFOLLOW_NOERROR | O_UNLINK_INTERNAL));
auto& inode = custody->inode();
@@ -690,7 +695,7 @@ ErrorOr<void> VirtualFileSystem::unlink(StringView path, Custody& base)
ErrorOr<void> VirtualFileSystem::symlink(StringView target, StringView linkpath, Custody& base)
{
- LockRefPtr<Custody> parent_custody;
+ RefPtr<Custody> parent_custody;
auto existing_custody_or_error = resolve_path(linkpath, base, &parent_custody);
if (!existing_custody_or_error.is_error())
return EEXIST;
@@ -719,7 +724,7 @@ ErrorOr<void> VirtualFileSystem::symlink(StringView target, StringView linkpath,
ErrorOr<void> VirtualFileSystem::rmdir(StringView path, Custody& base)
{
- LockRefPtr<Custody> parent_custody;
+ RefPtr<Custody> parent_custody;
auto custody = TRY(resolve_path(path, base, &parent_custody));
auto& inode = custody->inode();
@@ -766,7 +771,7 @@ ErrorOr<void> VirtualFileSystem::for_each_mount(Function<ErrorOr<void>(Mount con
{
return m_mounts.with([&](auto& mounts) -> ErrorOr<void> {
for (auto& mount : mounts)
- TRY(callback(mount));
+ TRY(callback(*mount));
return {};
});
}
@@ -776,9 +781,9 @@ void VirtualFileSystem::sync()
FileSystem::sync();
}
-Custody& VirtualFileSystem::root_custody()
+NonnullRefPtr<Custody> VirtualFileSystem::root_custody()
{
- return *m_root_custody;
+ return m_root_custody.with([](auto& root_custody) -> NonnullRefPtr<Custody> { return *root_custody; });
}
UnveilNode const& VirtualFileSystem::find_matching_unveiled_path(StringView path)
@@ -871,7 +876,7 @@ ErrorOr<void> VirtualFileSystem::validate_path_against_process_veil(StringView p
return {};
}
-ErrorOr<NonnullLockRefPtr<Custody>> VirtualFileSystem::resolve_path(StringView path, Custody& base, LockRefPtr<Custody>* out_parent, int options, int symlink_recursion_level)
+ErrorOr<NonnullRefPtr<Custody>> VirtualFileSystem::resolve_path(StringView path, NonnullRefPtr<Custody> base, RefPtr<Custody>* out_parent, int options, int symlink_recursion_level)
{
// FIXME: The errors returned by resolve_path_without_veil can leak information about paths that are not unveiled,
// e.g. when the error is EACCESS or similar.
@@ -899,7 +904,7 @@ static bool safe_to_follow_symlink(Inode const& inode, InodeMetadata const& pare
return false;
}
-ErrorOr<NonnullLockRefPtr<Custody>> VirtualFileSystem::resolve_path_without_veil(StringView path, Custody& base, LockRefPtr<Custody>* out_parent, int options, int symlink_recursion_level)
+ErrorOr<NonnullRefPtr<Custody>> VirtualFileSystem::resolve_path_without_veil(StringView path, NonnullRefPtr<Custody> base, RefPtr<Custody>* out_parent, int options, int symlink_recursion_level)
{
if (symlink_recursion_level >= symlink_recursion_limit)
return ELOOP;
@@ -910,7 +915,7 @@ ErrorOr<NonnullLockRefPtr<Custody>> VirtualFileSystem::resolve_path_without_veil
GenericLexer path_lexer(path);
auto& current_process = Process::current();
- NonnullLockRefPtr<Custody> custody = path[0] == '/' ? root_custody() : base;
+ NonnullRefPtr<Custody> custody = path[0] == '/' ? root_custody() : base;
bool extra_iteration = path[path.length() - 1] == '/';
while (!path_lexer.is_eof() || extra_iteration) {
diff --git a/Kernel/FileSystem/VirtualFileSystem.h b/Kernel/FileSystem/VirtualFileSystem.h
index a9b358eb01..26459474d8 100644
--- a/Kernel/FileSystem/VirtualFileSystem.h
+++ b/Kernel/FileSystem/VirtualFileSystem.h
@@ -67,7 +67,7 @@ public:
ErrorOr<void> utimensat(StringView path, Custody& base, timespec const& atime, timespec const& mtime, int options = 0);
ErrorOr<void> rename(StringView oldpath, StringView newpath, Custody& base);
ErrorOr<void> mknod(StringView path, mode_t, dev_t, Custody& base);
- ErrorOr<NonnullLockRefPtr<Custody>> open_directory(StringView path, Custody& base);
+ ErrorOr<NonnullRefPtr<Custody>> open_directory(StringView path, Custody& base);
ErrorOr<void> for_each_mount(Function<ErrorOr<void>(Mount const&)>) const;
@@ -75,9 +75,9 @@ public:
static void sync();
- Custody& root_custody();
- ErrorOr<NonnullLockRefPtr<Custody>> resolve_path(StringView path, Custody& base, LockRefPtr<Custody>* out_parent = nullptr, int options = 0, int symlink_recursion_level = 0);
- ErrorOr<NonnullLockRefPtr<Custody>> resolve_path_without_veil(StringView path, Custody& base, LockRefPtr<Custody>* out_parent = nullptr, int options = 0, int symlink_recursion_level = 0);
+ NonnullRefPtr<Custody> root_custody();
+ ErrorOr<NonnullRefPtr<Custody>> resolve_path(StringView path, NonnullRefPtr<Custody> base, RefPtr<Custody>* out_parent = nullptr, int options = 0, int symlink_recursion_level = 0);
+ ErrorOr<NonnullRefPtr<Custody>> resolve_path_without_veil(StringView path, NonnullRefPtr<Custody> base, RefPtr<Custody>* out_parent = nullptr, int options = 0, int symlink_recursion_level = 0);
private:
friend class OpenFileDescription;
@@ -92,13 +92,15 @@ private:
bool mount_point_exists_at_inode(InodeIdentifier inode);
+ // FIXME: These functions are totally unsafe as someone could unmount the returned Mount underneath us.
Mount* find_mount_for_host(InodeIdentifier);
Mount* find_mount_for_guest(InodeIdentifier);
LockRefPtr<Inode> m_root_inode;
- LockRefPtr<Custody> m_root_custody;
- SpinlockProtected<Vector<Mount, 16>> m_mounts { LockRank::None };
+ SpinlockProtected<RefPtr<Custody>> m_root_custody;
+
+ SpinlockProtected<Vector<NonnullOwnPtr<Mount>, 16>> m_mounts { LockRank::None };
};
}
diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp
index 5adb6d85e7..c069d0f0ed 100644
--- a/Kernel/Process.cpp
+++ b/Kernel/Process.cpp
@@ -222,7 +222,7 @@ void Process::unprotect_data()
});
}
-ErrorOr<NonnullLockRefPtr<Process>> Process::try_create(LockRefPtr<Thread>& first_thread, NonnullOwnPtr<KString> name, UserID uid, GroupID gid, ProcessID ppid, bool is_kernel_process, LockRefPtr<Custody> current_directory, LockRefPtr<Custody> executable, TTY* tty, Process* fork_parent)
+ErrorOr<NonnullLockRefPtr<Process>> Process::try_create(LockRefPtr<Thread>& first_thread, NonnullOwnPtr<KString> name, UserID uid, GroupID gid, ProcessID ppid, bool is_kernel_process, RefPtr<Custody> current_directory, RefPtr<Custody> executable, TTY* tty, Process* fork_parent)
{
auto space = TRY(Memory::AddressSpace::try_create(fork_parent ? &fork_parent->address_space() : nullptr));
auto unveil_tree = UnveilNode { TRY(KString::try_create("/"sv)), UnveilMetadata(TRY(KString::try_create("/"sv))) };
@@ -232,10 +232,10 @@ ErrorOr<NonnullLockRefPtr<Process>> Process::try_create(LockRefPtr<Thread>& firs
return process;
}
-Process::Process(NonnullOwnPtr<KString> name, NonnullRefPtr<Credentials> credentials, ProcessID ppid, bool is_kernel_process, LockRefPtr<Custody> current_directory, LockRefPtr<Custody> executable, TTY* tty, UnveilNode unveil_tree)
+Process::Process(NonnullOwnPtr<KString> name, NonnullRefPtr<Credentials> credentials, ProcessID ppid, bool is_kernel_process, RefPtr<Custody> current_directory, RefPtr<Custody> executable, TTY* tty, UnveilNode unveil_tree)
: m_name(move(name))
, m_is_kernel_process(is_kernel_process)
- , m_executable(move(executable))
+ , m_executable(LockRank::None, move(executable))
, m_current_directory(LockRank::None, move(current_directory))
, m_tty(tty)
, m_unveil_data(LockRank::None, move(unveil_tree))
@@ -537,9 +537,9 @@ siginfo_t Process::wait_info() const
return siginfo;
}
-NonnullLockRefPtr<Custody> Process::current_directory()
+NonnullRefPtr<Custody> Process::current_directory()
{
- return m_current_directory.with([&](auto& current_directory) -> NonnullLockRefPtr<Custody> {
+ return m_current_directory.with([&](auto& current_directory) -> NonnullRefPtr<Custody> {
if (!current_directory)
current_directory = VirtualFileSystem::the().root_custody();
return *current_directory;
@@ -642,7 +642,7 @@ void Process::finalize()
TimerQueue::the().cancel_timer(m_alarm_timer.release_nonnull());
m_fds.with_exclusive([](auto& fds) { fds.clear(); });
m_tty = nullptr;
- m_executable = nullptr;
+ m_executable.with([](auto& executable) { executable = nullptr; });
m_arguments.clear();
m_environment.clear();
@@ -971,4 +971,14 @@ NonnullRefPtr<Credentials> Process::credentials() const
return *m_protected_values.credentials;
}
+RefPtr<Custody> Process::executable()
+{
+ return m_executable.with([](auto& executable) { return executable; });
+}
+
+RefPtr<Custody const> Process::executable() const
+{
+ return m_executable.with([](auto& executable) { return executable; });
+}
+
}
diff --git a/Kernel/Process.h b/Kernel/Process.h
index 887b80aea0..345cc4e343 100644
--- a/Kernel/Process.h
+++ b/Kernel/Process.h
@@ -449,9 +449,9 @@ public:
u32 m_ticks_in_user_for_dead_children { 0 };
u32 m_ticks_in_kernel_for_dead_children { 0 };
- NonnullLockRefPtr<Custody> current_directory();
- Custody* executable() { return m_executable.ptr(); }
- Custody const* executable() const { return m_executable.ptr(); }
+ NonnullRefPtr<Custody> current_directory();
+ RefPtr<Custody> executable();
+ RefPtr<Custody const> executable() const;
static constexpr size_t max_arguments_size = Thread::default_userspace_stack_size / 8;
static constexpr size_t max_environment_size = Thread::default_userspace_stack_size / 8;
@@ -556,8 +556,8 @@ private:
bool add_thread(Thread&);
bool remove_thread(Thread&);
- Process(NonnullOwnPtr<KString> name, NonnullRefPtr<Credentials>, ProcessID ppid, bool is_kernel_process, LockRefPtr<Custody> current_directory, LockRefPtr<Custody> executable, TTY* tty, UnveilNode unveil_tree);
- static ErrorOr<NonnullLockRefPtr<Process>> try_create(LockRefPtr<Thread>& first_thread, NonnullOwnPtr<KString> name, UserID, GroupID, ProcessID ppid, bool is_kernel_process, LockRefPtr<Custody> current_directory = nullptr, LockRefPtr<Custody> executable = nullptr, TTY* = nullptr, Process* fork_parent = nullptr);
+ Process(NonnullOwnPtr<KString> name, NonnullRefPtr<Credentials>, ProcessID ppid, bool is_kernel_process, RefPtr<Custody> current_directory, RefPtr<Custody> executable, TTY* tty, UnveilNode unveil_tree);
+ static ErrorOr<NonnullLockRefPtr<Process>> try_create(LockRefPtr<Thread>& first_thread, NonnullOwnPtr<KString> name, UserID, GroupID, ProcessID ppid, bool is_kernel_process, RefPtr<Custody> current_directory = nullptr, RefPtr<Custody> executable = nullptr, TTY* = nullptr, Process* fork_parent = nullptr);
ErrorOr<void> attach_resources(NonnullOwnPtr<Memory::AddressSpace>&&, LockRefPtr<Thread>& first_thread, Process* fork_parent);
static ProcessID allocate_pid();
@@ -824,9 +824,9 @@ private:
Atomic<bool, AK::MemoryOrder::memory_order_relaxed> m_is_stopped { false };
bool m_should_generate_coredump { false };
- LockRefPtr<Custody> m_executable;
+ SpinlockProtected<RefPtr<Custody>> m_executable;
- SpinlockProtected<LockRefPtr<Custody>> m_current_directory;
+ SpinlockProtected<RefPtr<Custody>> m_current_directory;
NonnullOwnPtrVector<KString> m_arguments;
NonnullOwnPtrVector<KString> m_environment;
diff --git a/Kernel/ProcessSpecificExposed.cpp b/Kernel/ProcessSpecificExposed.cpp
index 8e25c28c20..82fcf6735e 100644
--- a/Kernel/ProcessSpecificExposed.cpp
+++ b/Kernel/ProcessSpecificExposed.cpp
@@ -333,7 +333,7 @@ mode_t Process::binary_link_required_mode() const
ErrorOr<void> Process::procfs_get_binary_link(KBufferBuilder& builder) const
{
- auto const* custody = executable();
+ auto custody = executable();
if (!custody)
return Error::from_errno(ENOEXEC);
return builder.append(TRY(custody->try_serialize_absolute_path())->view());
diff --git a/Kernel/Syscalls/chdir.cpp b/Kernel/Syscalls/chdir.cpp
index 445a4fc03e..a10a7cbc5b 100644
--- a/Kernel/Syscalls/chdir.cpp
+++ b/Kernel/Syscalls/chdir.cpp
@@ -4,6 +4,7 @@
* SPDX-License-Identifier: BSD-2-Clause
*/
+#include <AK/RefPtr.h>
#include <Kernel/FileSystem/Custody.h>
#include <Kernel/FileSystem/VirtualFileSystem.h>
#include <Kernel/Process.h>
@@ -15,10 +16,10 @@ ErrorOr<FlatPtr> Process::sys$chdir(Userspace<char const*> user_path, size_t pat
VERIFY_NO_PROCESS_BIG_LOCK(this);
TRY(require_promise(Pledge::rpath));
auto path = TRY(get_syscall_path_argument(user_path, path_length));
- auto current_directory = m_current_directory.with([](auto& current_directory) -> NonnullLockRefPtr<Custody> {
+ auto current_directory = m_current_directory.with([](auto& current_directory) -> NonnullRefPtr<Custody> {
return *current_directory;
});
- LockRefPtr<Custody> new_directory = TRY(VirtualFileSystem::the().open_directory(path->view(), *current_directory));
+ RefPtr<Custody> new_directory = TRY(VirtualFileSystem::the().open_directory(path->view(), *current_directory));
m_current_directory.with([&](auto& current_directory) {
// NOTE: We use swap() here to avoid manipulating the ref counts while holding the lock.
swap(current_directory, new_directory);
diff --git a/Kernel/Syscalls/chmod.cpp b/Kernel/Syscalls/chmod.cpp
index 564b87a61e..c1f2e2fc8d 100644
--- a/Kernel/Syscalls/chmod.cpp
+++ b/Kernel/Syscalls/chmod.cpp
@@ -18,7 +18,7 @@ ErrorOr<FlatPtr> Process::sys$chmod(Userspace<Syscall::SC_chmod_params const*> u
auto params = TRY(copy_typed_from_user(user_params));
auto path = TRY(get_syscall_path_argument(params.path));
- LockRefPtr<Custody> base;
+ RefPtr<Custody> base;
if (params.dirfd == AT_FDCWD) {
base = current_directory();
} else {
diff --git a/Kernel/Syscalls/chown.cpp b/Kernel/Syscalls/chown.cpp
index 70c49b2876..4a72df16a2 100644
--- a/Kernel/Syscalls/chown.cpp
+++ b/Kernel/Syscalls/chown.cpp
@@ -4,6 +4,7 @@
* SPDX-License-Identifier: BSD-2-Clause
*/
+#include <AK/RefPtr.h>
#include <Kernel/FileSystem/Custody.h>
#include <Kernel/FileSystem/OpenFileDescription.h>
#include <Kernel/Library/NonnullLockRefPtrVector.h>
@@ -27,7 +28,7 @@ ErrorOr<FlatPtr> Process::sys$chown(Userspace<Syscall::SC_chown_params const*> u
auto params = TRY(copy_typed_from_user(user_params));
auto path = TRY(get_syscall_path_argument(params.path));
- LockRefPtr<Custody> base;
+ RefPtr<Custody> base;
if (params.dirfd == AT_FDCWD) {
base = current_directory();
} else {
diff --git a/Kernel/Syscalls/execve.cpp b/Kernel/Syscalls/execve.cpp
index 0a8dd5eeeb..405c9dbc8c 100644
--- a/Kernel/Syscalls/execve.cpp
+++ b/Kernel/Syscalls/execve.cpp
@@ -554,7 +554,7 @@ ErrorOr<void> Process::do_exec(NonnullLockRefPtr<OpenFileDescription> main_progr
m_space = load_result.space.release_nonnull();
- m_executable = main_program_description->custody();
+ m_executable.with([&](auto& executable) { executable = main_program_description->custody(); });
m_arguments = move(arguments);
m_environment = move(environment);
diff --git a/Kernel/Syscalls/fork.cpp b/Kernel/Syscalls/fork.cpp
index 772b38764b..2009f743a7 100644
--- a/Kernel/Syscalls/fork.cpp
+++ b/Kernel/Syscalls/fork.cpp
@@ -28,7 +28,7 @@ ErrorOr<FlatPtr> Process::sys$fork(RegisterState& regs)
};
auto child_name = TRY(m_name->try_clone());
- auto child = TRY(Process::try_create(child_first_thread, move(child_name), uid(), gid(), pid(), m_is_kernel_process, current_directory(), m_executable, m_tty, this));
+ auto child = TRY(Process::try_create(child_first_thread, move(child_name), uid(), gid(), pid(), m_is_kernel_process, current_directory(), executable(), m_tty, this));
// NOTE: All user processes have a leaked ref on them. It's balanced by Thread::WaitBlockerSet::finalize().
child->ref();
diff --git a/Kernel/Syscalls/open.cpp b/Kernel/Syscalls/open.cpp
index 77294cd0f7..2a0c773255 100644
--- a/Kernel/Syscalls/open.cpp
+++ b/Kernel/Syscalls/open.cpp
@@ -4,6 +4,7 @@
* SPDX-License-Identifier: BSD-2-Clause
*/
+#include <AK/RefPtr.h>
#include <Kernel/Debug.h>
#include <Kernel/FileSystem/Custody.h>
#include <Kernel/FileSystem/VirtualFileSystem.h>
@@ -54,7 +55,7 @@ ErrorOr<FlatPtr> Process::sys$open(Userspace<Syscall::SC_open_params const*> use
dbgln_if(IO_DEBUG, "sys$open(dirfd={}, path='{}', options={}, mode={})", dirfd, path->view(), options, mode);
auto fd_allocation = TRY(allocate_fd());
- LockRefPtr<Custody> base;
+ RefPtr<Custody> base;
if (dirfd == AT_FDCWD) {
base = current_directory();
} else {
diff --git a/Kernel/Syscalls/stat.cpp b/Kernel/Syscalls/stat.cpp
index 39eea5b7e9..89194d32d0 100644
--- a/Kernel/Syscalls/stat.cpp
+++ b/Kernel/Syscalls/stat.cpp
@@ -4,6 +4,7 @@
* SPDX-License-Identifier: BSD-2-Clause
*/
+#include <AK/RefPtr.h>
#include <Kernel/FileSystem/Custody.h>
#include <Kernel/FileSystem/VirtualFileSystem.h>
#include <Kernel/Library/NonnullLockRefPtrVector.h>
@@ -29,7 +30,7 @@ ErrorOr<FlatPtr> Process::sys$stat(Userspace<Syscall::SC_stat_params const*> use
auto path = TRY(get_syscall_path_argument(params.path));
- LockRefPtr<Custody> base;
+ RefPtr<Custody> base;
if (params.dirfd == AT_FDCWD) {
base = current_directory();
} else {
diff --git a/Kernel/Syscalls/unlink.cpp b/Kernel/Syscalls/unlink.cpp
index 21a5edbc3c..5f3a62330e 100644
--- a/Kernel/Syscalls/unlink.cpp
+++ b/Kernel/Syscalls/unlink.cpp
@@ -19,7 +19,7 @@ ErrorOr<FlatPtr> Process::sys$unlink(int dirfd, Userspace<char const*> user_path
if (flags & ~AT_REMOVEDIR)
return Error::from_errno(EINVAL);
- LockRefPtr<Custody> base;
+ RefPtr<Custody> base;
if (dirfd == AT_FDCWD) {
base = current_directory();
} else {
diff --git a/Kernel/Syscalls/unveil.cpp b/Kernel/Syscalls/unveil.cpp
index a993f00952..a27ebefbbb 100644
--- a/Kernel/Syscalls/unveil.cpp
+++ b/Kernel/Syscalls/unveil.cpp
@@ -5,6 +5,7 @@
* SPDX-License-Identifier: BSD-2-Clause
*/
+#include <AK/RefPtr.h>
#include <AK/StringView.h>
#include <Kernel/FileSystem/Custody.h>
#include <Kernel/FileSystem/VirtualFileSystem.h>
@@ -79,7 +80,7 @@ ErrorOr<FlatPtr> Process::sys$unveil(Userspace<Syscall::SC_unveil_params const*>
// However, if the user specified unveil() with "c" permissions, we don't set errno if ENOENT is encountered,
// because they most likely intend the program to create the file for them later on.
// If this case is encountered, the parent node of the path is returned and the custody of that inode is used instead.
- LockRefPtr<Custody> parent_custody; // Parent inode in case of ENOENT
+ RefPtr<Custody> parent_custody; // Parent inode in case of ENOENT
OwnPtr<KString> new_unveiled_path;
auto custody_or_error = VirtualFileSystem::the().resolve_path_without_veil(path->view(), VirtualFileSystem::the().root_custody(), &parent_custody);
if (!custody_or_error.is_error()) {
diff --git a/Kernel/Syscalls/utimensat.cpp b/Kernel/Syscalls/utimensat.cpp
index 45bfd1cb1b..73b13ecb2e 100644
--- a/Kernel/Syscalls/utimensat.cpp
+++ b/Kernel/Syscalls/utimensat.cpp
@@ -41,7 +41,7 @@ ErrorOr<FlatPtr> Process::sys$utimensat(Userspace<Syscall::SC_utimensat_params c
OwnPtr<KString> path;
LockRefPtr<OpenFileDescription> description;
- LockRefPtr<Custody> base;
+ RefPtr<Custody> base;
auto path_or_error = get_syscall_path_argument(params.path);
if (path_or_error.is_error()) {