From d0d13e2bf5541298567d8c57e7bf240855ab29e1 Mon Sep 17 00:00:00 2001 From: Sergey Bugaev Date: Sun, 19 Jan 2020 01:15:52 +0300 Subject: [PATCH] Kernel: Move setting file flags and r/w mode to VFS::open() Previously, VFS::open() would only use the passed flags for permission checking purposes, and Process::sys$open() would set them on the created FileDescription explicitly. Now, they should be set by VFS::open() on any files being opened, including files that the kernel opens internally. This also lets us get rid of the explicit check for whether or not the returned FileDescription was a preopen fd, and in fact, fixes a bug where a read-only preopen fd without any other flags would be considered freshly opened (due to O_RDONLY being indistinguishable from 0) and granted a new set of flags. --- Kernel/FileSystem/File.cpp | 1 + Kernel/FileSystem/VirtualFileSystem.cpp | 12 ++++++++---- Kernel/Process.cpp | 8 -------- Kernel/TTY/PTYMultiplexer.cpp | 1 + 4 files changed, 10 insertions(+), 12 deletions(-) diff --git a/Kernel/FileSystem/File.cpp b/Kernel/FileSystem/File.cpp index cb73de0e34..3d78dd41d6 100644 --- a/Kernel/FileSystem/File.cpp +++ b/Kernel/FileSystem/File.cpp @@ -39,6 +39,7 @@ KResultOr> File::open(int options) { auto description = FileDescription::create(*this); description->set_rw_mode(options); + description->set_file_flags(options); return description; } diff --git a/Kernel/FileSystem/VirtualFileSystem.cpp b/Kernel/FileSystem/VirtualFileSystem.cpp index 44805dd5f6..05880480f1 100644 --- a/Kernel/FileSystem/VirtualFileSystem.cpp +++ b/Kernel/FileSystem/VirtualFileSystem.cpp @@ -277,7 +277,10 @@ KResultOr> VFS::open(StringView path, int options inode.truncate(0); inode.set_mtime(kgettimeofday().tv_sec); } - return FileDescription::create(custody); + auto description = FileDescription::create(custody); + description->set_rw_mode(options); + description->set_file_flags(options); + return description; } KResult VFS::mknod(StringView path, mode_t mode, dev_t dev, Custody& base) @@ -309,8 +312,6 @@ KResult VFS::mknod(StringView path, mode_t mode, dev_t dev, Custody& base) KResultOr> VFS::create(StringView path, int options, mode_t mode, Custody& parent_custody, Optional owner) { - (void)options; - if (!is_socket(mode) && !is_fifo(mode) && !is_block_device(mode) && !is_character_device(mode)) { // Turn it into a regular file. (This feels rather hackish.) mode |= 0100000; @@ -332,7 +333,10 @@ KResultOr> VFS::create(StringView path, int optio return KResult(error); auto new_custody = Custody::create(&parent_custody, p.basename(), *new_file, parent_custody.mount_flags()); - return FileDescription::create(*new_custody); + auto description = FileDescription::create(*new_custody); + description->set_rw_mode(options); + description->set_file_flags(options); + return description; } KResult VFS::mkdir(StringView path, mode_t mode, Custody& base) diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index 8f1aff6106..9f9e7e5199 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -1971,14 +1971,6 @@ int Process::sys$open(const Syscall::SC_open_params* user_params) if (result.is_error()) return result.error(); auto description = result.value(); - if (description->file_flags()) { - // We already have file flags set on this description, so - // it must be a preopen description (probably, /proc/pid/fd). - // So don't reset its flags and r/w mode. - } else { - description->set_rw_mode(options); - description->set_file_flags(options); - } u32 fd_flags = (options & O_CLOEXEC) ? FD_CLOEXEC : 0; m_fds[fd].set(move(description), fd_flags); return fd; diff --git a/Kernel/TTY/PTYMultiplexer.cpp b/Kernel/TTY/PTYMultiplexer.cpp index a9cfd01529..bdbadaa4d8 100644 --- a/Kernel/TTY/PTYMultiplexer.cpp +++ b/Kernel/TTY/PTYMultiplexer.cpp @@ -66,6 +66,7 @@ KResultOr> PTYMultiplexer::open(int options) #endif auto description = FileDescription::create(move(master)); description->set_rw_mode(options); + description->set_file_flags(options); return description; }