system/serenity
system/serenity
1
0
Fork 0
mirror of https://github.com/SerenityOS/serenity synced 2024-10-15 04:13:11 +00:00
Code Issues Packages Projects Releases Wiki Activity

Kernel/VFS: Ensure working with mount entry per a custody is safe

Browse source 
Previously we could get a raw pointer to a Mount object which might be
invalid when actually dereferencing it.
To ensure this could not happen, we should just use a callback that will
be used immediately after finding the appropriate Mount entry, while
holding the mount table lock.
This commit is contained in:
Liav A 2023-08-04 22:57:25 +03:00 committed by Jelle Raaijmakers
parent d216f780a4
commit 5efb91ec06
2 changed files with 18 additions and 16 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

31
Kernel/FileSystem/VirtualFileSystem.cpp
View file

@ -245,11 +245,9 @@ ErrorOr<void> VirtualFileSystem::remount(Custody& mount_point, int new_flags)
{ {
dbgln("VirtualFileSystem: Remounting inode {}", mount_point.inode().identifier()); dbgln("VirtualFileSystem: Remounting inode {}", mount_point.inode().identifier());
auto* mount = find_mount_for_host_custody(mount_point); TRY(apply_to_mount_for_host_custody(mount_point, [new_flags](auto& mount) {
if (!mount) mount.set_flags(new_flags);
return ENODEV; }));
mount->set_flags(new_flags);
return {}; return {};
} }
@ -372,25 +370,28 @@ ErrorOr<void> VirtualFileSystem::mount_root(FileSystem& fs)
return {}; return {};
} }
auto VirtualFileSystem::find_mount_for_host_custody(Custody const& current_custody) -> Mount* ErrorOr<void> VirtualFileSystem::apply_to_mount_for_host_custody(Custody const& current_custody, Function<void(Mount&)> callback)
{ {
return m_mounts.with([&](auto& mounts) -> Mount* { return m_mounts.with([&](auto& mounts) -> ErrorOr<void> {
// NOTE: We either search for the root mount or for a mount that has a parent custody! // NOTE: We either search for the root mount or for a mount that has a parent custody!
if (!current_custody.parent()) { if (!current_custody.parent()) {
for (auto& mount : mounts) { for (auto& mount : mounts) {
if (!mount.host_custody()) if (!mount.host_custody()) {
return &mount; callback(mount);
return {};
}
} }
// NOTE: There must be a root mount entry, so fail if we don't find it. // NOTE: There must be a root mount entry, so fail if we don't find it.
VERIFY_NOT_REACHED(); VERIFY_NOT_REACHED();
} else { } else {
for (auto& mount : mounts) { for (auto& mount : mounts) {
if (mount.host_custody() && check_matching_absolute_path_hierarchy(*mount.host_custody(), current_custody)) { if (mount.host_custody() && check_matching_absolute_path_hierarchy(*mount.host_custody(), current_custody)) {
return &mount; callback(mount);
return {};
} }
} }
} }
return nullptr; return Error::from_errno(ENODEV);
}); });
} }
@ -1225,9 +1226,11 @@ ErrorOr<NonnullRefPtr<Custody>> VirtualFileSystem::resolve_path_without_veil(Cre
// See if there's something mounted on the child; in that case // See if there's something mounted on the child; in that case
// we would need to return the guest inode, not the host inode. // we would need to return the guest inode, not the host inode.
if (auto mount = find_mount_for_host_custody(current_custody)) { auto found_mount_or_error = apply_to_mount_for_host_custody(current_custody, [&child_inode, &mount_flags_for_child](auto& mount) {
child_inode = mount->guest(); child_inode = mount.guest();
mount_flags_for_child = mount->flags(); mount_flags_for_child = mount.flags();
});
if (!found_mount_or_error.is_error()) {
custody = TRY(Custody::try_create(&parent, part, *child_inode, mount_flags_for_child)); custody = TRY(Custody::try_create(&parent, part, *child_inode, mount_flags_for_child));
} else { } else {
custody = current_custody; custody = current_custody;

3
Kernel/FileSystem/VirtualFileSystem.h
View file

@ -114,8 +114,7 @@ private:
static bool check_matching_absolute_path_hierarchy(Custody const& first_custody, Custody const& second_custody); static bool check_matching_absolute_path_hierarchy(Custody const& first_custody, Custody const& second_custody);
bool mount_point_exists_at_custody(Custody& mount_point); bool mount_point_exists_at_custody(Custody& mount_point);
// FIXME: This function is totally unsafe as someone could unmount the returned Mount underneath us. ErrorOr<void> apply_to_mount_for_host_custody(Custody const& current_custody, Function<void(Mount&)>);
Mount* find_mount_for_host_custody(Custody const& current_custody);
RefPtr<Inode> m_root_inode; RefPtr<Inode> m_root_inode;