From e886337a67484516b4393a6ddcf7f8c3bc7bdc23 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sun, 28 Apr 2019 15:02:55 +0200 Subject: [PATCH] Kernel: Make ProcessTracer inherit from File. --- Kernel/Devices/BXVGADevice.cpp | 2 +- Kernel/Devices/BXVGADevice.h | 2 +- Kernel/Devices/BlockDevice.h | 2 +- Kernel/Devices/Device.cpp | 14 +-- Kernel/Devices/Device.h | 31 +----- Kernel/File.h | 2 + Kernel/FileSystem/FileDescriptor.cpp | 147 +++++++++------------------ Kernel/FileSystem/FileDescriptor.h | 32 ++---- Kernel/Makefile | 3 +- Kernel/Process.cpp | 13 ++- Kernel/ProcessTracer.cpp | 22 ++-- Kernel/ProcessTracer.h | 17 ++-- 12 files changed, 105 insertions(+), 182 deletions(-) diff --git a/Kernel/Devices/BXVGADevice.cpp b/Kernel/Devices/BXVGADevice.cpp index b014a22304..eacf06f4f2 100644 --- a/Kernel/Devices/BXVGADevice.cpp +++ b/Kernel/Devices/BXVGADevice.cpp @@ -84,7 +84,7 @@ dword BXVGADevice::find_framebuffer_address() return framebuffer_address; } -Region* BXVGADevice::mmap(Process& process, LinearAddress preferred_laddr, size_t offset, size_t size) +KResultOr BXVGADevice::mmap(Process& process, LinearAddress preferred_laddr, size_t offset, size_t size) { ASSERT(offset == 0); ASSERT(size == framebuffer_size_in_bytes()); diff --git a/Kernel/Devices/BXVGADevice.h b/Kernel/Devices/BXVGADevice.h index f7c80c63c2..4db38fc577 100644 --- a/Kernel/Devices/BXVGADevice.h +++ b/Kernel/Devices/BXVGADevice.h @@ -18,7 +18,7 @@ public: void set_y_offset(int); virtual int ioctl(Process&, unsigned request, unsigned arg) override; - virtual Region* mmap(Process&, LinearAddress preferred_laddr, size_t offset, size_t) override; + virtual KResultOr mmap(Process&, LinearAddress preferred_laddr, size_t offset, size_t) override; size_t framebuffer_size_in_bytes() const { return m_framebuffer_size.area() * sizeof(dword) * 2; } Size framebuffer_size() const { return m_framebuffer_size; } diff --git a/Kernel/Devices/BlockDevice.h b/Kernel/Devices/BlockDevice.h index 2d9edfddc0..c96d4d3185 100644 --- a/Kernel/Devices/BlockDevice.h +++ b/Kernel/Devices/BlockDevice.h @@ -7,7 +7,7 @@ class BlockDevice : public Device { public: virtual ~BlockDevice() override; - virtual Region* mmap(Process&, LinearAddress preferred_laddr, size_t offset, size_t size) = 0; + virtual bool is_seekable() const override { return true; } protected: BlockDevice(unsigned major, unsigned minor) : Device(major, minor) { } diff --git a/Kernel/Devices/Device.cpp b/Kernel/Devices/Device.cpp index 0163c94736..8319fc08c4 100644 --- a/Kernel/Devices/Device.cpp +++ b/Kernel/Devices/Device.cpp @@ -13,17 +13,7 @@ Device::~Device() VFS::the().unregister_device(*this); } -KResultOr> Device::open(int options) +String Device::absolute_path() const { - UNUSED_PARAM(options); - return FileDescriptor::create(this); -} - -void Device::close() -{ -} - -int Device::ioctl(Process&, unsigned, unsigned) -{ - return -ENOTTY; + return String::format("device:%u,%u (%s)", m_major, m_minor, class_name()); } diff --git a/Kernel/Devices/Device.h b/Kernel/Devices/Device.h index 155c5adf85..14ca312234 100644 --- a/Kernel/Devices/Device.h +++ b/Kernel/Devices/Device.h @@ -33,42 +33,21 @@ // - Subclasses should take care to validate incoming addresses before dereferencing. // -#include -#include -#include +#include -class Process; - -class Device : public Retainable { +class Device : public File { public: - virtual ~Device(); - - InodeMetadata metadata() const { return { }; } - - virtual KResultOr> open(int options); - virtual void close(); - - virtual bool can_read(Process&) const = 0; - virtual bool can_write(Process&) const = 0; - - virtual ssize_t read(Process&, byte*, ssize_t) = 0; - virtual ssize_t write(Process&, const byte*, ssize_t) = 0; + virtual ~Device() override; unsigned major() const { return m_major; } unsigned minor() const { return m_minor; } - virtual bool is_tty() const { return false; } - virtual bool is_master_pty() const { return false; } - - virtual int ioctl(Process&, unsigned request, unsigned arg); - - virtual const char* class_name() const = 0; + virtual String absolute_path() const override; uid_t uid() const { return m_uid; } uid_t gid() const { return m_gid; } - virtual bool is_block_device() const { return false; } - virtual bool is_character_device() const { return false; } + virtual bool is_device() const override { return true; } protected: Device(unsigned major, unsigned minor); diff --git a/Kernel/File.h b/Kernel/File.h index 51235cf5e0..e3e67d9d01 100644 --- a/Kernel/File.h +++ b/Kernel/File.h @@ -1,3 +1,5 @@ +#pragma once + #include #include #include diff --git a/Kernel/FileSystem/FileDescriptor.cpp b/Kernel/FileSystem/FileDescriptor.cpp index b037e1ee03..9746b9d40f 100644 --- a/Kernel/FileSystem/FileDescriptor.cpp +++ b/Kernel/FileSystem/FileDescriptor.cpp @@ -12,21 +12,15 @@ #include #include #include -#include Retained FileDescriptor::create(RetainPtr&& inode) { return adopt(*new FileDescriptor(move(inode))); } -Retained FileDescriptor::create(RetainPtr&& device) +Retained FileDescriptor::create(RetainPtr&& file) { - return adopt(*new FileDescriptor(move(device))); -} - -Retained FileDescriptor::create(RetainPtr&& tracer) -{ - return adopt(*new FileDescriptor(move(tracer))); + return adopt(*new FileDescriptor(move(file))); } Retained FileDescriptor::create(RetainPtr&& shared_memory) @@ -54,13 +48,8 @@ FileDescriptor::FileDescriptor(RetainPtr&& inode) { } -FileDescriptor::FileDescriptor(RetainPtr&& device) - : m_device(move(device)) -{ -} - -FileDescriptor::FileDescriptor(RetainPtr&& tracer) - : m_tracer(move(tracer)) +FileDescriptor::FileDescriptor(RetainPtr&& file) + : m_file(move(file)) { } @@ -81,9 +70,9 @@ FileDescriptor::~FileDescriptor() m_socket->detach_fd(m_socket_role); m_socket = nullptr; } - if (m_device) { - m_device->close(); - m_device = nullptr; + if (m_file) { + m_file->close(); + m_file = nullptr; } if (m_fifo) { m_fifo->close(fifo_direction()); @@ -113,8 +102,8 @@ Retained FileDescriptor::clone() ? FileDescriptor::create_pipe_reader(*m_fifo) : FileDescriptor::create_pipe_writer(*m_fifo); } else { - if (m_device) { - descriptor = FileDescriptor::create(m_device.copy_ref()); + if (m_file) { + descriptor = FileDescriptor::create(m_file.copy_ref()); descriptor->m_inode = m_inode.copy_ref(); } else if (m_socket) { descriptor = FileDescriptor::create(m_socket.copy_ref(), m_socket_role); @@ -140,7 +129,7 @@ bool addition_would_overflow(off_t a, off_t b) KResult FileDescriptor::fstat(stat& buffer) { ASSERT(!is_fifo()); - if (!m_inode && !m_device) + if (!m_inode && !m_file) return KResult(-EBADF); auto metadata = this->metadata(); @@ -173,7 +162,7 @@ KResult FileDescriptor::fchmod(mode_t mode) off_t FileDescriptor::seek(off_t offset, int whence) { ASSERT(!is_fifo()); - if (!m_inode && !m_device) + if (!m_inode && !m_file) return -EBADF; // FIXME: The file type should be cached on the vnode. @@ -210,15 +199,15 @@ off_t FileDescriptor::seek(off_t offset, int whence) ssize_t FileDescriptor::read(Process& process, byte* buffer, ssize_t count) { - if (m_tracer) - return m_tracer->read(buffer, count); if (is_fifo()) { ASSERT(fifo_direction() == FIFO::Reader); return m_fifo->read(buffer, count); } - if (m_device) { - // FIXME: What should happen to m_currentOffset? - return m_device->read(process, buffer, count); + if (m_file) { + int nread = m_file->read(process, buffer, count); + if (!m_file->is_seekable()) + m_current_offset += nread; + return nread; } if (m_socket) return m_socket->read(m_socket_role, buffer, count); @@ -230,17 +219,15 @@ ssize_t FileDescriptor::read(Process& process, byte* buffer, ssize_t count) ssize_t FileDescriptor::write(Process& process, const byte* data, ssize_t size) { - if (m_tracer) { - // FIXME: Figure out what the right error code would be. - return -EIO; - } if (is_fifo()) { ASSERT(fifo_direction() == FIFO::Writer); return m_fifo->write(data, size); } - if (m_device) { - // FIXME: What should happen to m_currentOffset? - return m_device->write(process, data, size); + if (m_file) { + int nwritten = m_file->write(process, data, size); + if (m_file->is_seekable()) + m_current_offset += nwritten; + return nwritten; } if (m_socket) return m_socket->write(m_socket_role, data, size); @@ -252,14 +239,12 @@ ssize_t FileDescriptor::write(Process& process, const byte* data, ssize_t size) bool FileDescriptor::can_write(Process& process) { - if (m_tracer) - return true; if (is_fifo()) { ASSERT(fifo_direction() == FIFO::Writer); return m_fifo->can_write(); } - if (m_device) - return m_device->can_write(process); + if (m_file) + return m_file->can_write(process); if (m_socket) return m_socket->can_write(m_socket_role); return true; @@ -267,14 +252,12 @@ bool FileDescriptor::can_write(Process& process) bool FileDescriptor::can_read(Process& process) { - if (m_tracer) - return m_tracer->can_read(); if (is_fifo()) { ASSERT(fifo_direction() == FIFO::Reader); return m_fifo->can_read(); } - if (m_device) - return m_device->can_read(process); + if (m_file) + return m_file->can_read(process); if (m_socket) return m_socket->can_read(m_socket_role); return true; @@ -282,11 +265,13 @@ bool FileDescriptor::can_read(Process& process) ByteBuffer FileDescriptor::read_entire_file(Process& process) { + // HACK ALERT: (This entire function) + ASSERT(!is_fifo()); - if (m_device) { + if (m_file) { auto buffer = ByteBuffer::create_uninitialized(1024); - ssize_t nread = m_device->read(process, buffer.pointer(), buffer.size()); + ssize_t nread = m_file->read(process, buffer.pointer(), buffer.size()); ASSERT(nread >= 0); buffer.trim(nread); return buffer; @@ -330,44 +315,47 @@ ssize_t FileDescriptor::get_dir_entries(byte* buffer, ssize_t size) return stream.offset(); } +bool FileDescriptor::is_device() const +{ + return m_file && m_file->is_device(); +} + bool FileDescriptor::is_tty() const { - return m_device && m_device->is_tty(); + return m_file && m_file->is_tty(); } const TTY* FileDescriptor::tty() const { if (!is_tty()) return nullptr; - return static_cast(m_device.ptr()); + return static_cast(m_file.ptr()); } TTY* FileDescriptor::tty() { if (!is_tty()) return nullptr; - return static_cast(m_device.ptr()); + return static_cast(m_file.ptr()); } bool FileDescriptor::is_master_pty() const { - if (m_device) - return m_device->is_master_pty(); - return false; + return m_file && m_file->is_master_pty(); } const MasterPTY* FileDescriptor::master_pty() const { if (!is_master_pty()) return nullptr; - return static_cast(m_device.ptr()); + return static_cast(m_file.ptr()); } MasterPTY* FileDescriptor::master_pty() { if (!is_master_pty()) return nullptr; - return static_cast(m_device.ptr()); + return static_cast(m_file.ptr()); } int FileDescriptor::close() @@ -389,22 +377,19 @@ const char* to_string(SocketRole role) } } -bool FileDescriptor::is_file() const +bool FileDescriptor::is_fsfile() const { return !is_tty() && !is_fifo() && !is_device() && !is_socket() && !is_shared_memory(); } KResultOr FileDescriptor::absolute_path() { - Stopwatch sw("absolute_path"); - if (m_tracer) - return String::format("tracer:%d", m_tracer->pid()); if (is_tty()) return tty()->tty_name(); if (is_fifo()) return String::format("fifo:%u", m_fifo.ptr()); - if (is_device()) - return String::format("device:%u,%u (%s)", m_device->major(), m_device->minor(), m_device->class_name()); + if (m_file) + return m_file->absolute_path(); if (is_socket()) return String::format("socket:%x (role: %s)", m_socket.ptr(), to_string(m_socket_role)); ASSERT(m_inode); @@ -426,32 +411,20 @@ InodeMetadata FileDescriptor::metadata() const return { }; } -bool FileDescriptor::supports_mmap() const +KResultOr FileDescriptor::mmap(Process& process, LinearAddress laddr, size_t offset, size_t size, int prot) { - if (m_tracer) - return false; - if (m_inode) - return true; - if (m_shared_memory) - return true; - if (m_device) - return m_device->is_block_device(); - return false; -} - -Region* FileDescriptor::mmap(Process& process, LinearAddress laddr, size_t offset, size_t size, int prot) -{ - ASSERT(supports_mmap()); - - if (is_block_device()) - return static_cast(*m_device).mmap(process, laddr, offset, size); + if (m_file) + return m_file->mmap(process, laddr, offset, size); if (is_shared_memory()) { if (!shared_memory()->vmo()) - return nullptr; + return KResult(-ENODEV); return process.allocate_region_with_vmo(laddr, size, *shared_memory()->vmo(), offset, shared_memory()->name(), true, true); } + if (!is_fsfile()) + return KResult(-ENODEV); + ASSERT(m_inode); // FIXME: If PROT_EXEC, check that the underlying file system isn't mounted noexec. String region_name; @@ -469,26 +442,6 @@ Region* FileDescriptor::mmap(Process& process, LinearAddress laddr, size_t offse return region; } -bool FileDescriptor::is_block_device() const -{ - return m_device && m_device->is_block_device(); -} - -bool FileDescriptor::is_character_device() const -{ - return m_device && m_device->is_character_device(); -} - -CharacterDevice* FileDescriptor::character_device() -{ - return is_character_device() ? static_cast(device()) : nullptr; -} - -const CharacterDevice* FileDescriptor::character_device() const -{ - return is_character_device() ? static_cast(device()) : nullptr; -} - KResult FileDescriptor::truncate(off_t length) { if (is_file()) { diff --git a/Kernel/FileSystem/FileDescriptor.h b/Kernel/FileSystem/FileDescriptor.h index e0f8e71049..8a4a8fd41c 100644 --- a/Kernel/FileSystem/FileDescriptor.h +++ b/Kernel/FileSystem/FileDescriptor.h @@ -11,21 +11,19 @@ #include #include +class File; class TTY; class MasterPTY; class Process; -class ProcessTracer; class Region; class CharacterDevice; class FileDescriptor : public Retainable { public: - static Retained create(RetainPtr&&, SocketRole = SocketRole::None); static Retained create(RetainPtr&&); - static Retained create(RetainPtr&&); + static Retained create(RetainPtr&&); static Retained create(RetainPtr&&); - static Retained create(RetainPtr&&); static Retained create_pipe_writer(FIFO&); static Retained create_pipe_reader(FIFO&); ~FileDescriptor(); @@ -52,14 +50,12 @@ public: bool is_directory() const; - bool is_device() const { return m_device.ptr(); } - Device* device() { return m_device.ptr(); } - const Device* device() const { return m_device.ptr(); } + // FIXME: These should go away once everything is a File. + bool is_file() const { return m_file.ptr(); } + File* file() { return m_file.ptr(); } + const File* file() const { return m_file.ptr(); } - bool is_block_device() const; - bool is_character_device() const; - CharacterDevice* character_device(); - const CharacterDevice* character_device() const; + bool is_device() const; bool is_tty() const; const TTY* tty() const; @@ -73,8 +69,7 @@ public: Inode* inode() { return m_inode.ptr(); } const Inode* inode() const { return m_inode.ptr(); } - bool supports_mmap() const; - Region* mmap(Process&, LinearAddress, size_t offset, size_t, int prot); + KResultOr mmap(Process&, LinearAddress, size_t offset, size_t, int prot); bool is_blocking() const { return m_is_blocking; } void set_blocking(bool b) { m_is_blocking = b; } @@ -89,7 +84,7 @@ public: bool is_fifo() const { return m_fifo; } FIFO::Direction fifo_direction() { return m_fifo_direction; } - bool is_file() const; + bool is_fsfile() const; bool is_shared_memory() const { return m_shared_memory; } SharedMemory* shared_memory() { return m_shared_memory.ptr(); } const SharedMemory* shared_memory() const { return m_shared_memory.ptr(); } @@ -103,20 +98,16 @@ public: KResult truncate(off_t); - ProcessTracer* tracer() { return m_tracer.ptr(); } - const ProcessTracer* tracer() const { return m_tracer.ptr(); } - private: friend class VFS; FileDescriptor(RetainPtr&&, SocketRole); explicit FileDescriptor(RetainPtr&&); - explicit FileDescriptor(RetainPtr&&); - explicit FileDescriptor(RetainPtr&&); + explicit FileDescriptor(RetainPtr&&); explicit FileDescriptor(RetainPtr&&); FileDescriptor(FIFO&, FIFO::Direction); RetainPtr m_inode; - RetainPtr m_device; + RetainPtr m_file; off_t m_current_offset { 0 }; @@ -132,7 +123,6 @@ private: FIFO::Direction m_fifo_direction { FIFO::Neither }; RetainPtr m_shared_memory; - RetainPtr m_tracer; bool m_closed { false }; }; diff --git a/Kernel/Makefile b/Kernel/Makefile index 85deede46f..265b143cb0 100644 --- a/Kernel/Makefile +++ b/Kernel/Makefile @@ -48,7 +48,8 @@ KERNEL_OBJS = \ Net/LoopbackAdapter.o \ Net/Routing.o \ Net/NetworkTask.o \ - ProcessTracer.o + ProcessTracer.o \ + File.o VFS_OBJS = \ FileSystem/ProcFS.o \ diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index e6771eb69c..c9e11927e9 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -175,11 +175,10 @@ void* Process::sys$mmap(const Syscall::SC_mmap_params* params) auto* descriptor = file_descriptor(fd); if (!descriptor) return (void*)-EBADF; - if (!descriptor->supports_mmap()) - return (void*)-ENODEV; - auto* region = descriptor->mmap(*this, LinearAddress((dword)addr), offset, size, prot); - if (!region) - return (void*)-ENOMEM; + auto region_or_error = descriptor->mmap(*this, LinearAddress((dword)addr), offset, size, prot); + if (region_or_error.is_error()) + return (void*)(int)region_or_error.error(); + auto region = region_or_error.value(); if (flags & MAP_SHARED) region->set_shared(true); return region->laddr().as_ptr(); @@ -1539,9 +1538,9 @@ int Process::sys$ioctl(int fd, unsigned request, unsigned arg) auto* descriptor = file_descriptor(fd); if (!descriptor) return -EBADF; - if (!descriptor->is_device()) + if (!descriptor->is_file()) return -ENOTTY; - return descriptor->device()->ioctl(*this, request, arg); + return descriptor->file()->ioctl(*this, request, arg); } int Process::sys$getdtablesize() diff --git a/Kernel/ProcessTracer.cpp b/Kernel/ProcessTracer.cpp index b15763d632..30889a121d 100644 --- a/Kernel/ProcessTracer.cpp +++ b/Kernel/ProcessTracer.cpp @@ -18,14 +18,18 @@ void ProcessTracer::did_syscall(dword function, dword arg1, dword arg2, dword ar m_calls.enqueue(data); } -int ProcessTracer::read(byte* buffer, int buffer_size) +int ProcessTracer::read(Process&, byte* buffer, int buffer_size) { - if (!m_calls.is_empty()) { - auto data = m_calls.dequeue(); - // FIXME: This should not be an assertion. - ASSERT(buffer_size == sizeof(data)); - memcpy(buffer, &data, sizeof(data)); - return sizeof(data); - } - return 0; + if (m_calls.is_empty()) + return 0; + auto data = m_calls.dequeue(); + // FIXME: This should not be an assertion. + ASSERT(buffer_size == sizeof(data)); + memcpy(buffer, &data, sizeof(data)); + return sizeof(data); +} + +String ProcessTracer::absolute_path() const +{ + return String::format("tracer:%d", m_pid); } diff --git a/Kernel/ProcessTracer.h b/Kernel/ProcessTracer.h index d178fb123d..355d5512d1 100644 --- a/Kernel/ProcessTracer.h +++ b/Kernel/ProcessTracer.h @@ -1,25 +1,30 @@ #pragma once -#include -#include +#include #include #include -class ProcessTracer : public Retainable { +class ProcessTracer : public File { public: static Retained create(pid_t pid) { return adopt(*new ProcessTracer(pid)); } - ~ProcessTracer(); + virtual ~ProcessTracer() override; bool is_dead() const { return m_dead; } void set_dead() { m_dead = true; } - bool can_read() const { return !m_calls.is_empty() || m_dead; } - int read(byte*, int); + virtual bool can_read(Process&) const override { return !m_calls.is_empty() || m_dead; } + virtual int read(Process&, byte*, int) override; + + virtual bool can_write(Process&) const override { return true; } + virtual int write(Process&, const byte*, int) override { return -EIO; } + + virtual String absolute_path() const override; void did_syscall(dword function, dword arg1, dword arg2, dword arg3, dword result); pid_t pid() const { return m_pid; } private: + virtual const char* class_name() const override { return "ProcessTracer"; } explicit ProcessTracer(pid_t); struct CallData {