From 0a1865ebc6a0df78464c63064e1e3e61384441bc Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Fri, 3 Jan 2020 03:29:59 +0100 Subject: [PATCH] Kernel: read() and write() should fail with EBADF for wrong mode fd's It was previously possible to write to read-only file descriptors, and read from write-only file descriptors. All FileDescription objects now start out non-readable + non-writable, and whoever is creating them has to "manually" enable reading/writing by calling set_readable() and/or set_writable() on them. --- Kernel/FileSystem/FileDescription.h | 22 ++++++++++++++++++++++ Kernel/Process.cpp | 17 +++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/Kernel/FileSystem/FileDescription.h b/Kernel/FileSystem/FileDescription.h index 29f144aee4..df3ab85429 100644 --- a/Kernel/FileSystem/FileDescription.h +++ b/Kernel/FileSystem/FileDescription.h @@ -25,6 +25,26 @@ public: static NonnullRefPtr create(File&); ~FileDescription(); + bool is_readable() const { return m_readable; } + bool is_writable() const { return m_writable; } + + void set_readable(bool b) { m_readable = b; } + void set_writable(bool b) { m_writable = b; } + + void set_rw_mode(int options) + { + if (options & O_WRONLY) { + set_readable(false); + set_writable(true); + } else if (options & O_RDWR) { + set_readable(true); + set_writable(true); + } else { + set_readable(true); + set_writable(false); + } + } + int close(); off_t seek(off_t, int whence); @@ -111,6 +131,8 @@ private: u32 m_file_flags { 0 }; + bool m_readable { false }; + bool m_writable { false }; bool m_is_blocking { true }; bool m_is_directory { false }; bool m_should_append { false }; diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index e66f93ea1b..e36d023900 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -1285,6 +1285,9 @@ ssize_t Process::sys$writev(int fd, const struct iovec* iov, int iov_count) if (!description) return -EBADF; + if (!description->is_writable()) + return -EBADF; + int nwritten = 0; for (int i = 0; i < iov_count; ++i) { int rc = do_write(*description, (const u8*)iov[i].iov_base, iov[i].iov_len); @@ -1357,6 +1360,8 @@ ssize_t Process::sys$write(int fd, const u8* data, ssize_t size) auto* description = file_description(fd); if (!description) return -EBADF; + if (!description->is_writable()) + return -EBADF; return do_write(*description, data, size); } @@ -1375,6 +1380,8 @@ ssize_t Process::sys$read(int fd, u8* buffer, ssize_t size) auto* description = file_description(fd); if (!description) return -EBADF; + if (!description->is_readable()) + return -EBADF; if (description->is_directory()) return -EISDIR; if (description->is_blocking()) { @@ -1589,6 +1596,7 @@ int Process::sys$open(const Syscall::SC_open_params* params) if (result.is_error()) return result.error(); auto description = result.value(); + 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); @@ -1627,6 +1635,7 @@ int Process::sys$openat(const Syscall::SC_openat_params* params) if (result.is_error()) return result.error(); auto description = result.value(); + 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); @@ -1660,10 +1669,12 @@ int Process::sys$pipe(int pipefd[2], int flags) int reader_fd = alloc_fd(); m_fds[reader_fd].set(fifo->open_direction(FIFO::Direction::Reader), fd_flags); + m_fds[reader_fd].description->set_readable(true); pipefd[0] = reader_fd; int writer_fd = alloc_fd(); m_fds[writer_fd].set(fifo->open_direction(FIFO::Direction::Writer), fd_flags); + m_fds[writer_fd].description->set_writable(true); pipefd[1] = writer_fd; return 0; @@ -2684,6 +2695,8 @@ int Process::sys$socket(int domain, int type, int protocol) if (result.is_error()) return result.error(); auto description = FileDescription::create(*result.value()); + description->set_readable(true); + description->set_writable(true); unsigned flags = 0; if (type & SOCK_CLOEXEC) flags |= FD_CLOEXEC; @@ -2747,6 +2760,8 @@ int Process::sys$accept(int accepting_socket_fd, sockaddr* address, socklen_t* a bool success = accepted_socket->get_peer_address(address, address_size); ASSERT(success); auto accepted_socket_description = FileDescription::create(*accepted_socket); + accepted_socket_description->set_readable(true); + accepted_socket_description->set_writable(true); // NOTE: The accepted socket inherits fd flags from the accepting socket. // I'm not sure if this matches other systems but it makes sense to me. accepted_socket_description->set_blocking(accepting_socket_description->is_blocking()); @@ -3376,6 +3391,7 @@ int Process::sys$watch_file(const char* path, int path_length) return fd; m_fds[fd].set(FileDescription::create(*InodeWatcher::create(inode))); + m_fds[fd].description->set_readable(true); return fd; } @@ -3391,6 +3407,7 @@ int Process::sys$systrace(pid_t pid) if (fd < 0) return fd; auto description = FileDescription::create(peer->ensure_tracer()); + description->set_readable(true); m_fds[fd].set(move(description), 0); return fd; }