Skip to content

Commit

Permalink
Kernel: Allow unveiling subfolders regardless of parent's permissions
Browse files Browse the repository at this point in the history
This fixes a bug where unveiling a subdirectory of an already unveiled
path would sometimes be allowed and sometimes not (depending on what
other unveil calls have been made).

Now, it is always allowed to unveil a subdirectory of an already
unveiled directory, even if it has higher permissions.

This removes the need for the permissions_inherited_from_root flag in
UnveilMetadata, so it has been removed.
  • Loading branch information
MaxWipfli authored and awesomekling committed Jun 8, 2021
1 parent 9d41dd2 commit e8a3170
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 10 deletions.
2 changes: 1 addition & 1 deletion Kernel/Process.h
Original file line number Diff line number Diff line change
Expand Up @@ -629,7 +629,7 @@ class Process
RefPtr<Timer> m_alarm_timer;

VeilState m_veil_state { VeilState::None };
UnveilNode m_unveiled_paths { "/", { .full_path = "/", .unveil_inherited_from_root = true } };
UnveilNode m_unveiled_paths { "/", { .full_path = "/" } };

OwnPtr<PerformanceEventBuffer> m_perf_event_buffer;

Expand Down
14 changes: 7 additions & 7 deletions Kernel/Syscalls/unveil.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/*
* Copyright (c) 2018-2020, Andreas Kling <[email protected]>
* Copyright (c) 2021, Max Wipfli <[email protected]>
*
* SPDX-License-Identifier: BSD-2-Clause
*/
Expand Down Expand Up @@ -90,14 +91,12 @@ KResultOr<int> Process::sys$unveil(Userspace<const Syscall::SC_unveil_params*> u
auto it = lexical_path.parts().begin();
auto& matching_node = m_unveiled_paths.traverse_until_last_accessible_node(it, lexical_path.parts().end());
if (it.is_end()) {
auto old_permissions = matching_node.permissions();
// Allow "elevating" the permissions when the permissions are inherited from root (/),
// as that would be the first time this path is unveiled.
if (old_permissions != UnveilAccess::None || !matching_node.permissions_inherited_from_root()) {
if (new_permissions & ~old_permissions)
// 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;
}
matching_node.set_metadata({ matching_node.path(), (UnveilAccess)new_permissions, true, false });
matching_node.set_metadata({ matching_node.path(), (UnveilAccess)new_permissions, true });
return 0;
}

Expand All @@ -107,8 +106,9 @@ KResultOr<int> Process::sys$unveil(Userspace<const Syscall::SC_unveil_params*> u
{ new_unveiled_path, (UnveilAccess)new_permissions, true },
[](auto& parent, auto& it) -> Optional<UnveilMetadata> {
auto path = LexicalPath::join(parent.path(), *it).string();
return UnveilMetadata { path, parent.permissions(), false, parent.permissions_inherited_from_root() };
return UnveilMetadata { path, parent.permissions(), false };
});

VERIFY(m_veil_state != VeilState::Locked);
m_veil_state = VeilState::Dropped;
return 0;
Expand Down
2 changes: 0 additions & 2 deletions Kernel/UnveilNode.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,11 @@ struct UnveilMetadata {
String full_path;
UnveilAccess permissions { None };
bool explicitly_unveiled { false };
bool unveil_inherited_from_root { false }; // true if permissions are inherited from the tree root (/).
};

struct UnveilNode final : public Trie<String, UnveilMetadata, Traits<String>, UnveilNode> {
using Trie<String, UnveilMetadata, Traits<String>, UnveilNode>::Trie;

bool permissions_inherited_from_root() const { return this->metadata_value().unveil_inherited_from_root; }
bool was_explicitly_unveiled() const { return this->metadata_value().explicitly_unveiled; }
UnveilAccess permissions() const { return this->metadata_value().permissions; }
const String& path() const { return this->metadata_value().full_path; }
Expand Down

0 comments on commit e8a3170

Please sign in to comment.