0ct0pu5/ladybird
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 2

Kernel: Fix TOCTOU in fstatvfs

Browse source 
In particular, fstatvfs used to assume that a file that was earlier
opened using some path will forever be at that path. This is wrong, and
in the meantime new mounts and new filesystems could take up the
filename or directories, leading to a completely inaccurate result.
This commit improves the situation:
- All filesystem information is now always accurate.
- The mount flags *might* be erroneously zero, if the custody for the
  open file is not available. I don't know when that might happen, but
  it is definitely not the typical case.
This commit is contained in:
Ben Wiederhake 2021-11-07 00:09:48 +01:00 • committed by Andreas Kling
parent 26a48f3516
commit 631447da57
Notes: sideshowbarker 2024-07-18 01:18:55 +09:00
2 changed files with 16 additions and 33 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
Kernel/Process.h
View file

@ -535,7 +535,7 @@ private:
ErrorOr<void> do_exec(NonnullRefPtr<OpenFileDescription> main_program_description, NonnullOwnPtrVector<KString> arguments, NonnullOwnPtrVector<KString> environment, RefPtr<OpenFileDescription> interpreter_description, Thread*& new_main_thread, u32& prev_flags, const ElfW(Ehdr) & main_program_header);
ErrorOr<FlatPtr> do_write(OpenFileDescription&, const UserOrKernelBuffer&, size_t);
ErrorOr<FlatPtr> do_statvfs(StringView path, statvfs* buf);
ErrorOr<FlatPtr> do_statvfs(FileSystem const& path, Custody const*, statvfs* buf);
ErrorOr<RefPtr<OpenFileDescription>> find_elf_interpreter_for_executable(StringView path, ElfW(Ehdr) const& main_executable_header, size_t main_executable_header_size, size_t file_size);

47
Kernel/Syscalls/statvfs.cpp
View file

@ -10,12 +10,8 @@
namespace Kernel {
ErrorOr<FlatPtr> Process::do_statvfs(StringView path, statvfs* buf)
ErrorOr<FlatPtr> Process::do_statvfs(FileSystem const& fs, Custody const* custody, statvfs* buf)
{
auto custody = TRY(VirtualFileSystem::the().resolve_path(path, current_directory(), nullptr, 0));
auto& inode = custody->inode();
auto& fs = inode.fs();
statvfs kernelbuf = {};
kernelbuf.f_bsize = static_cast<u64>(fs.block_size());
@ -34,29 +30,8 @@ ErrorOr<FlatPtr> Process::do_statvfs(StringView path, statvfs* buf)
kernelbuf.f_namemax = 255;
Custody* current_custody = custody;
while (current_custody) {
VirtualFileSystem::the().for_each_mount([&kernelbuf, &current_custody](auto& mount) {
if (&current_custody->inode() == &mount.guest()) {
int mountflags = mount.flags();
int flags = 0;
if (mountflags & MS_RDONLY)
flags = flags | ST_RDONLY;
if (mountflags & MS_NOSUID)
flags = flags | ST_NOSUID;
kernelbuf.f_flag = flags;
current_custody = nullptr;
return IterationDecision::Break;
}
return IterationDecision::Continue;
});
if (current_custody) {
current_custody = current_custody->parent();
}
}
if (custody)
kernelbuf.f_flag = custody->mount_flags();
TRY(copy_to_user(buf, &kernelbuf));
return 0;
@ -69,7 +44,12 @@ ErrorOr<FlatPtr> Process::sys$statvfs(Userspace<const Syscall::SC_statvfs_params
auto params = TRY(copy_typed_from_user(user_params));
auto path = TRY(get_syscall_path_argument(params.path));
return do_statvfs(path->view(), params.buf);
auto custody = TRY(VirtualFileSystem::the().resolve_path(path->view(), current_directory(), nullptr, 0));
auto& inode = custody->inode();
auto const& fs = inode.fs();
return do_statvfs(fs, custody, params.buf);
}
ErrorOr<FlatPtr> Process::sys$fstatvfs(int fd, statvfs* buf)
@ -78,9 +58,12 @@ ErrorOr<FlatPtr> Process::sys$fstatvfs(int fd, statvfs* buf)
REQUIRE_PROMISE(stdio);
auto description = TRY(fds().open_file_description(fd));
auto absolute_path = TRY(description->original_absolute_path());
// FIXME: TOCTOU bug! The file connected to the fd may or may not have been moved, and the name possibly taken by a different file.
return do_statvfs(absolute_path->view(), buf);
auto inode = description->inode();
if (inode == nullptr)
return ENOENT;
// FIXME: The custody that we pass in might be outdated. However, this only affects the mount flags.
return do_statvfs(inode->fs(), description->custody(), buf);
}
}