diff options
author | Andreas Kling <kling@serenityos.org> | 2022-08-21 01:04:35 +0200 |
---|---|---|
committer | Andreas Kling <kling@serenityos.org> | 2022-08-21 12:25:14 +0200 |
commit | 728c3fbd14252bd746f97dbb5441063992074b6b (patch) | |
tree | 539c32ede90702ab3ef35b984addad93769db340 | |
parent | 5331d243c690c70e431e2f8d260eacab19946c2b (diff) | |
download | serenity-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.cpp | 9 | ||||
-rw-r--r-- | Kernel/FileSystem/Custody.h | 12 | ||||
-rw-r--r-- | Kernel/FileSystem/Inode.cpp | 2 | ||||
-rw-r--r-- | Kernel/FileSystem/Inode.h | 2 | ||||
-rw-r--r-- | Kernel/FileSystem/Mount.cpp | 32 | ||||
-rw-r--r-- | Kernel/FileSystem/Mount.h | 7 | ||||
-rw-r--r-- | Kernel/FileSystem/OpenFileDescription.cpp | 24 | ||||
-rw-r--r-- | Kernel/FileSystem/OpenFileDescription.h | 7 | ||||
-rw-r--r-- | Kernel/FileSystem/VirtualFileSystem.cpp | 71 | ||||
-rw-r--r-- | Kernel/FileSystem/VirtualFileSystem.h | 14 | ||||
-rw-r--r-- | Kernel/Process.cpp | 22 | ||||
-rw-r--r-- | Kernel/Process.h | 14 | ||||
-rw-r--r-- | Kernel/ProcessSpecificExposed.cpp | 2 | ||||
-rw-r--r-- | Kernel/Syscalls/chdir.cpp | 5 | ||||
-rw-r--r-- | Kernel/Syscalls/chmod.cpp | 2 | ||||
-rw-r--r-- | Kernel/Syscalls/chown.cpp | 3 | ||||
-rw-r--r-- | Kernel/Syscalls/execve.cpp | 2 | ||||
-rw-r--r-- | Kernel/Syscalls/fork.cpp | 2 | ||||
-rw-r--r-- | Kernel/Syscalls/open.cpp | 3 | ||||
-rw-r--r-- | Kernel/Syscalls/stat.cpp | 3 | ||||
-rw-r--r-- | Kernel/Syscalls/unlink.cpp | 2 | ||||
-rw-r--r-- | Kernel/Syscalls/unveil.cpp | 3 | ||||
-rw-r--r-- | Kernel/Syscalls/utimensat.cpp | 2 |
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()) { |