diff --git a/Kernel/FileSystem/VirtualFileSystem.cpp b/Kernel/FileSystem/VirtualFileSystem.cpp index 74eabfb8f1..46a9c6209c 100644 --- a/Kernel/FileSystem/VirtualFileSystem.cpp +++ b/Kernel/FileSystem/VirtualFileSystem.cpp @@ -748,10 +748,10 @@ UnveilNode const& VirtualFileSystem::find_matching_unveiled_path(StringView path { auto& current_process = Process::current(); VERIFY(current_process.veil_state() != VeilState::None); - auto& unveil_root = current_process.unveiled_paths(); - - auto path_parts = KLexicalPath::parts(path); - return unveil_root.traverse_until_last_accessible_node(path_parts.begin(), path_parts.end()); + return current_process.unveil_data().with([&](auto const& unveil_data) -> UnveilNode const& { + auto path_parts = KLexicalPath::parts(path); + return unveil_data.paths.traverse_until_last_accessible_node(path_parts.begin(), path_parts.end()); + }); } ErrorOr VirtualFileSystem::validate_path_against_process_veil(Custody const& custody, int options) diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index 30ffbe7421..aede604637 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -224,13 +224,13 @@ ErrorOr> Process::try_create(RefPtr& first_thread return process; } -Process::Process(NonnullOwnPtr name, UserID uid, GroupID gid, ProcessID ppid, bool is_kernel_process, NonnullRefPtr current_directory, RefPtr executable, TTY* tty, UnveilNode unveil_tree) +Process::Process(NonnullOwnPtr name, UserID uid, GroupID gid, ProcessID ppid, bool is_kernel_process, RefPtr current_directory, RefPtr executable, TTY* tty, UnveilNode unveil_tree) : m_name(move(name)) , m_is_kernel_process(is_kernel_process) , m_executable(move(executable)) , m_current_directory(move(current_directory)) , m_tty(tty) - , m_unveiled_paths(move(unveil_tree)) + , m_unveil_data(move(unveil_tree)) , m_wait_blocker_set(*this) { // Ensure that we protect the process data when exiting the constructor. diff --git a/Kernel/Process.h b/Kernel/Process.h index 8c59d0d11b..b90fb36225 100644 --- a/Kernel/Process.h +++ b/Kernel/Process.h @@ -466,13 +466,21 @@ public: VeilState veil_state() const { - return m_veil_state; - } - const UnveilNode& unveiled_paths() const - { - return m_unveiled_paths; + return m_unveil_data.with([&](auto const& unveil_data) { return unveil_data.state; }); } + struct UnveilData { + explicit UnveilData(UnveilNode&& p) + : paths(move(p)) + { + } + VeilState state { VeilState::None }; + UnveilNode paths; + }; + + auto& unveil_data() { return m_unveil_data; } + auto const& unveil_data() const { return m_unveil_data; } + bool wait_for_tracer_at_next_execve() const { return m_wait_for_tracer_at_next_execve; @@ -805,8 +813,7 @@ private: RefPtr m_alarm_timer; - VeilState m_veil_state { VeilState::None }; - UnveilNode m_unveiled_paths; + SpinlockProtected m_unveil_data; OwnPtr m_perf_event_buffer; diff --git a/Kernel/ProcessSpecificExposed.cpp b/Kernel/ProcessSpecificExposed.cpp index a8b1e0ecbb..b64ebdf0a8 100644 --- a/Kernel/ProcessSpecificExposed.cpp +++ b/Kernel/ProcessSpecificExposed.cpp @@ -145,25 +145,28 @@ ErrorOr Process::procfs_get_pledge_stats(KBufferBuilder& builder) const ErrorOr Process::procfs_get_unveil_stats(KBufferBuilder& builder) const { auto array = TRY(JsonArraySerializer<>::try_create(builder)); - TRY(unveiled_paths().for_each_node_in_tree_order([&](auto const& unveiled_path) -> ErrorOr { - if (!unveiled_path.was_explicitly_unveiled()) + TRY(m_unveil_data.with([&](auto& unveil_data) -> ErrorOr { + TRY(unveil_data.paths.for_each_node_in_tree_order([&](auto const& unveiled_path) -> ErrorOr { + if (!unveiled_path.was_explicitly_unveiled()) + return IterationDecision::Continue; + auto obj = TRY(array.add_object()); + TRY(obj.add("path", unveiled_path.path())); + StringBuilder permissions_builder; + if (unveiled_path.permissions() & UnveilAccess::Read) + permissions_builder.append('r'); + if (unveiled_path.permissions() & UnveilAccess::Write) + permissions_builder.append('w'); + if (unveiled_path.permissions() & UnveilAccess::Execute) + permissions_builder.append('x'); + if (unveiled_path.permissions() & UnveilAccess::CreateOrRemove) + permissions_builder.append('c'); + if (unveiled_path.permissions() & UnveilAccess::Browse) + permissions_builder.append('b'); + TRY(obj.add("permissions", permissions_builder.string_view())); + TRY(obj.finish()); return IterationDecision::Continue; - auto obj = TRY(array.add_object()); - TRY(obj.add("path", unveiled_path.path())); - StringBuilder permissions_builder; - if (unveiled_path.permissions() & UnveilAccess::Read) - permissions_builder.append('r'); - if (unveiled_path.permissions() & UnveilAccess::Write) - permissions_builder.append('w'); - if (unveiled_path.permissions() & UnveilAccess::Execute) - permissions_builder.append('x'); - if (unveiled_path.permissions() & UnveilAccess::CreateOrRemove) - permissions_builder.append('c'); - if (unveiled_path.permissions() & UnveilAccess::Browse) - permissions_builder.append('b'); - TRY(obj.add("permissions", permissions_builder.string_view())); - TRY(obj.finish()); - return IterationDecision::Continue; + })); + return {}; })); TRY(array.finish()); return {}; diff --git a/Kernel/Syscalls/execve.cpp b/Kernel/Syscalls/execve.cpp index 0dbc731168..376be0fb73 100644 --- a/Kernel/Syscalls/execve.cpp +++ b/Kernel/Syscalls/execve.cpp @@ -523,9 +523,12 @@ ErrorOr Process::do_exec(NonnullRefPtr main_program_d m_arguments = move(arguments); m_environment = move(environment); - m_veil_state = VeilState::None; - m_unveiled_paths.clear(); - m_unveiled_paths.set_metadata({ TRY(KString::try_create("/"sv)), UnveilAccess::None, false }); + TRY(m_unveil_data.with([&](auto& unveil_data) -> ErrorOr { + unveil_data.state = VeilState::None; + unveil_data.paths.clear(); + unveil_data.paths.set_metadata({ TRY(KString::try_create("/"sv)), UnveilAccess::None, false }); + return {}; + })); for (auto& property : m_coredump_properties) property = {}; diff --git a/Kernel/Syscalls/fork.cpp b/Kernel/Syscalls/fork.cpp index 2872b94737..505cbcc1fc 100644 --- a/Kernel/Syscalls/fork.cpp +++ b/Kernel/Syscalls/fork.cpp @@ -20,8 +20,13 @@ ErrorOr Process::sys$fork(RegisterState& regs) RefPtr child_first_thread; 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)); - child->m_veil_state = m_veil_state; - child->m_unveiled_paths = TRY(m_unveiled_paths.deep_copy()); + TRY(m_unveil_data.with([&](auto& parent_unveil_data) -> ErrorOr { + return child->m_unveil_data.with([&](auto& child_unveil_data) -> ErrorOr { + child_unveil_data.state = parent_unveil_data.state; + child_unveil_data.paths = TRY(parent_unveil_data.paths.deep_copy()); + return {}; + }); + })); TRY(child->m_fds.with_exclusive([&](auto& child_fds) { return m_fds.with_exclusive([&](auto& parent_fds) { diff --git a/Kernel/Syscalls/unveil.cpp b/Kernel/Syscalls/unveil.cpp index 38cd21c23b..b968653c40 100644 --- a/Kernel/Syscalls/unveil.cpp +++ b/Kernel/Syscalls/unveil.cpp @@ -30,11 +30,11 @@ ErrorOr Process::sys$unveil(Userspace auto params = TRY(copy_typed_from_user(user_params)); if (!params.path.characters && !params.permissions.characters) { - m_veil_state = VeilState::Locked; + m_unveil_data.with([&](auto& unveil_data) { unveil_data.state = VeilState::Locked; }); return 0; } - if (m_veil_state == VeilState::Locked) + if (veil_state() == VeilState::Locked) return EPERM; if (!params.path.characters || !params.permissions.characters) @@ -94,39 +94,41 @@ ErrorOr Process::sys$unveil(Userspace auto path_parts = KLexicalPath::parts(new_unveiled_path->view()); auto it = path_parts.begin(); - auto& matching_node = m_unveiled_paths.traverse_until_last_accessible_node(it, path_parts.end()); - if (it.is_end()) { - // If the path has already been explicitly unveiled, do not allow elevating its permissions. - if (matching_node.was_explicitly_unveiled()) { - if (new_permissions & ~matching_node.permissions()) - return EPERM; + return m_unveil_data.with([&](auto& unveil_data) -> ErrorOr { + auto& matching_node = unveil_data.paths.traverse_until_last_accessible_node(it, path_parts.end()); + if (it.is_end()) { + // If the path has already been explicitly unveiled, do not allow elevating its permissions. + if (matching_node.was_explicitly_unveiled()) { + if (new_permissions & ~matching_node.permissions()) + return EPERM; + } + + // It is possible that nodes that are "grandchildren" of the matching node have already been unveiled. + // This means that there may be intermediate nodes between this one and the unveiled "grandchildren" + // that inherited the current node's previous permissions. Those nodes now need their permissions + // updated to match the current node. + if (matching_node.permissions() != new_permissions) + update_intermediate_node_permissions(matching_node, (UnveilAccess)new_permissions); + + matching_node.metadata_value().explicitly_unveiled = true; + matching_node.metadata_value().permissions = (UnveilAccess)new_permissions; + unveil_data.state = VeilState::Dropped; + return 0; } - // It is possible that nodes that are "grandchildren" of the matching node have already been unveiled. - // This means that there may be intermediate nodes between this one and the unveiled "grandchildren" - // that inherited the current node's previous permissions. Those nodes now need their permissions - // updated to match the current node. - if (matching_node.permissions() != new_permissions) - update_intermediate_node_permissions(matching_node, (UnveilAccess)new_permissions); + TRY(matching_node.insert( + it, + path_parts.end(), + { new_unveiled_path.release_nonnull(), (UnveilAccess)new_permissions, true }, + [](auto& parent, auto& it) -> ErrorOr> { + auto path = TRY(KString::formatted("{}/{}", parent.path(), *it)); + return UnveilMetadata(move(path), parent.permissions(), false); + })); - matching_node.metadata_value().explicitly_unveiled = true; - matching_node.metadata_value().permissions = (UnveilAccess)new_permissions; - m_veil_state = VeilState::Dropped; + VERIFY(unveil_data.state != VeilState::Locked); + unveil_data.state = VeilState::Dropped; return 0; - } - - TRY(matching_node.insert( - it, - path_parts.end(), - { new_unveiled_path.release_nonnull(), (UnveilAccess)new_permissions, true }, - [](auto& parent, auto& it) -> ErrorOr> { - auto path = TRY(KString::formatted("{}/{}", parent.path(), *it)); - return UnveilMetadata(move(path), parent.permissions(), false); - })); - - VERIFY(m_veil_state != VeilState::Locked); - m_veil_state = VeilState::Dropped; - return 0; + }); } }