diff --git a/Kernel/Devices/Device.cpp b/Kernel/Devices/Device.cpp index f4301f2bce..7c9faa960c 100644 --- a/Kernel/Devices/Device.cpp +++ b/Kernel/Devices/Device.cpp @@ -140,18 +140,9 @@ Device::~Device() VERIFY(m_state == State::BeingRemoved); } -String Device::absolute_path() const +KResultOr> Device::pseudo_path(const OpenFileDescription&) const { - // FIXME: I assume we can't really provide a well known path in the kernel - // because this is a violation of abstraction layers between userland and the - // kernel, but maybe the whole name of "absolute_path" is just wrong as this - // is really not an "absolute_path". - return String::formatted("device:{},{}", major(), minor()); -} - -String Device::absolute_path(const OpenFileDescription&) const -{ - return absolute_path(); + return KString::try_create(String::formatted("device:{},{}", major(), minor())); } void Device::process_next_queued_request(Badge, const AsyncDeviceRequest& completed_request) diff --git a/Kernel/Devices/Device.h b/Kernel/Devices/Device.h index 67af4f85e1..6675242127 100644 --- a/Kernel/Devices/Device.h +++ b/Kernel/Devices/Device.h @@ -40,8 +40,7 @@ public: unsigned major() const { return m_major; } unsigned minor() const { return m_minor; } - virtual String absolute_path(const OpenFileDescription&) const override; - virtual String absolute_path() const; + virtual KResultOr> pseudo_path(const OpenFileDescription&) const override; UserID uid() const { return m_uid; } GroupID gid() const { return m_gid; } diff --git a/Kernel/FileSystem/AnonymousFile.cpp b/Kernel/FileSystem/AnonymousFile.cpp index 0c6ba54945..81022dda8f 100644 --- a/Kernel/FileSystem/AnonymousFile.cpp +++ b/Kernel/FileSystem/AnonymousFile.cpp @@ -30,4 +30,9 @@ KResultOr AnonymousFile::mmap(Process& process, OpenFileDescrip return process.address_space().allocate_region_with_vmobject(range, m_vmobject, offset, {}, prot, shared); } +KResultOr> AnonymousFile::pseudo_path(const OpenFileDescription&) const +{ + return KString::try_create(":anonymous-file:"sv); +} + } diff --git a/Kernel/FileSystem/AnonymousFile.h b/Kernel/FileSystem/AnonymousFile.h index af91c36d21..49a21e7fc7 100644 --- a/Kernel/FileSystem/AnonymousFile.h +++ b/Kernel/FileSystem/AnonymousFile.h @@ -24,7 +24,7 @@ public: private: virtual StringView class_name() const override { return "AnonymousFile"sv; } - virtual String absolute_path(const OpenFileDescription&) const override { return ":anonymous-file:"; } + virtual KResultOr> pseudo_path(const OpenFileDescription&) const; virtual bool can_read(const OpenFileDescription&, size_t) const override { return false; } virtual bool can_write(const OpenFileDescription&, size_t) const override { return false; } virtual KResultOr read(OpenFileDescription&, u64, UserOrKernelBuffer&, size_t) override { return ENOTSUP; } diff --git a/Kernel/FileSystem/FIFO.cpp b/Kernel/FileSystem/FIFO.cpp index 002c220247..2d1e585043 100644 --- a/Kernel/FileSystem/FIFO.cpp +++ b/Kernel/FileSystem/FIFO.cpp @@ -132,9 +132,9 @@ KResultOr FIFO::write(OpenFileDescription& fd, u64, const UserOrKernelBu return m_buffer->write(buffer, size); } -String FIFO::absolute_path(const OpenFileDescription&) const +KResultOr> FIFO::pseudo_path(const OpenFileDescription&) const { - return String::formatted("fifo:{}", m_fifo_id); + return KString::try_create(String::formatted("fifo:{}", m_fifo_id)); } KResult FIFO::stat(::stat& st) const diff --git a/Kernel/FileSystem/FIFO.h b/Kernel/FileSystem/FIFO.h index ab3326dd59..6352360863 100644 --- a/Kernel/FileSystem/FIFO.h +++ b/Kernel/FileSystem/FIFO.h @@ -45,7 +45,7 @@ private: virtual KResult stat(::stat&) const override; virtual bool can_read(const OpenFileDescription&, size_t) const override; virtual bool can_write(const OpenFileDescription&, size_t) const override; - virtual String absolute_path(const OpenFileDescription&) const override; + virtual KResultOr> pseudo_path(const OpenFileDescription&) const override; virtual StringView class_name() const override { return "FIFO"sv; } virtual bool is_fifo() const override { return true; } diff --git a/Kernel/FileSystem/File.h b/Kernel/FileSystem/File.h index 303549fdc3..c42ac1c51a 100644 --- a/Kernel/FileSystem/File.h +++ b/Kernel/FileSystem/File.h @@ -93,7 +93,8 @@ public: virtual KResultOr mmap(Process&, OpenFileDescription&, Memory::VirtualRange const&, u64 offset, int prot, bool shared); virtual KResult stat(::stat&) const { return EBADF; } - virtual String absolute_path(const OpenFileDescription&) const = 0; + // Although this might be better described "name" or "description", these terms already have other meanings. + virtual KResultOr> pseudo_path(const OpenFileDescription&) const = 0; virtual KResult truncate(u64) { return EINVAL; } virtual KResult sync() { return EINVAL; } diff --git a/Kernel/FileSystem/InodeFile.cpp b/Kernel/FileSystem/InodeFile.cpp index f037504f03..009de243c7 100644 --- a/Kernel/FileSystem/InodeFile.cpp +++ b/Kernel/FileSystem/InodeFile.cpp @@ -89,15 +89,14 @@ KResultOr InodeFile::mmap(Process& process, OpenFileDescription vmobject = TRY(Memory::SharedInodeVMObject::try_create_with_inode(inode())); else vmobject = TRY(Memory::PrivateInodeVMObject::try_create_with_inode(inode())); - auto path = TRY(description.try_serialize_absolute_path()); + auto path = TRY(description.pseudo_path()); return process.address_space().allocate_region_with_vmobject(range, vmobject.release_nonnull(), offset, path->view(), prot, shared); } -String InodeFile::absolute_path(const OpenFileDescription& description) const +KResultOr> InodeFile::pseudo_path(const OpenFileDescription&) const { + // If it has an inode, then it has a path, and therefore the caller should have been able to get a custody at some point. VERIFY_NOT_REACHED(); - VERIFY(description.custody()); - return description.absolute_path(); } KResult InodeFile::truncate(u64 size) diff --git a/Kernel/FileSystem/InodeFile.h b/Kernel/FileSystem/InodeFile.h index a51e90f1d6..ec220bbf99 100644 --- a/Kernel/FileSystem/InodeFile.h +++ b/Kernel/FileSystem/InodeFile.h @@ -36,7 +36,7 @@ public: virtual KResultOr mmap(Process&, OpenFileDescription&, Memory::VirtualRange const&, u64 offset, int prot, bool shared) override; virtual KResult stat(::stat& buffer) const override { return inode().metadata().stat(buffer); } - virtual String absolute_path(const OpenFileDescription&) const override; + virtual KResultOr> pseudo_path(const OpenFileDescription&) const override; virtual KResult truncate(u64) override; virtual KResult sync() override; diff --git a/Kernel/FileSystem/InodeWatcher.cpp b/Kernel/FileSystem/InodeWatcher.cpp index 33624ad440..bd750c5365 100644 --- a/Kernel/FileSystem/InodeWatcher.cpp +++ b/Kernel/FileSystem/InodeWatcher.cpp @@ -81,9 +81,9 @@ KResult InodeWatcher::close() return KSuccess; } -String InodeWatcher::absolute_path(const OpenFileDescription&) const +KResultOr> InodeWatcher::pseudo_path(const OpenFileDescription&) const { - return String::formatted("InodeWatcher:({})", m_wd_to_watches.size()); + return KString::try_create(String::formatted("InodeWatcher:({})", m_wd_to_watches.size())); } void InodeWatcher::notify_inode_event(Badge, InodeIdentifier inode_id, InodeWatcherEvent::Type event_type, String const& name) diff --git a/Kernel/FileSystem/InodeWatcher.h b/Kernel/FileSystem/InodeWatcher.h index 383506bc91..99840da740 100644 --- a/Kernel/FileSystem/InodeWatcher.h +++ b/Kernel/FileSystem/InodeWatcher.h @@ -50,7 +50,7 @@ public: virtual KResultOr write(OpenFileDescription&, u64, const UserOrKernelBuffer&, size_t) override { return EIO; } virtual KResult close() override; - virtual String absolute_path(const OpenFileDescription&) const override; + virtual KResultOr> pseudo_path(const OpenFileDescription&) const override; virtual StringView class_name() const override { return "InodeWatcher"sv; }; virtual bool is_inode_watcher() const override { return true; } diff --git a/Kernel/FileSystem/OpenFileDescription.cpp b/Kernel/FileSystem/OpenFileDescription.cpp index eb90c3450b..327d0d37e0 100644 --- a/Kernel/FileSystem/OpenFileDescription.cpp +++ b/Kernel/FileSystem/OpenFileDescription.cpp @@ -349,19 +349,18 @@ KResult OpenFileDescription::close() return m_file->close(); } -KResultOr> OpenFileDescription::try_serialize_absolute_path() +KResultOr> OpenFileDescription::original_absolute_path() const +{ + if (!m_custody) + return ENOENT; + return m_custody->try_serialize_absolute_path(); +} + +KResultOr> OpenFileDescription::pseudo_path() const { if (m_custody) return m_custody->try_serialize_absolute_path(); - // FIXME: Don't go through a String here! - return KString::try_create(m_file->absolute_path(*this)); -} - -String OpenFileDescription::absolute_path() const -{ - if (m_custody) - return m_custody->absolute_path(); - return m_file->absolute_path(*this); + return m_file->pseudo_path(*this); } InodeMetadata OpenFileDescription::metadata() const diff --git a/Kernel/FileSystem/OpenFileDescription.h b/Kernel/FileSystem/OpenFileDescription.h index 67413d4e74..72ec153439 100644 --- a/Kernel/FileSystem/OpenFileDescription.h +++ b/Kernel/FileSystem/OpenFileDescription.h @@ -64,8 +64,8 @@ public: KResultOr> read_entire_file(); - KResultOr> try_serialize_absolute_path(); - String absolute_path() const; + KResultOr> original_absolute_path() const; + KResultOr> pseudo_path() const; bool is_direct() const { return m_direct; } diff --git a/Kernel/FileSystem/VirtualFileSystem.cpp b/Kernel/FileSystem/VirtualFileSystem.cpp index 61546fecc7..b7c8d90826 100644 --- a/Kernel/FileSystem/VirtualFileSystem.cpp +++ b/Kernel/FileSystem/VirtualFileSystem.cpp @@ -124,7 +124,8 @@ KResult VirtualFileSystem::mount_root(FileSystem& fs) } m_root_inode = root_inode; - dmesgln("VirtualFileSystem: mounted root from {} ({})", fs.class_name(), static_cast(fs).file_description().absolute_path()); + auto pseudo_path = TRY(static_cast(fs).file_description().pseudo_path()); + dmesgln("VirtualFileSystem: mounted root from {} ({})", fs.class_name(), pseudo_path); m_mounts.with_exclusive([&](auto& mounts) { mounts.append(move(mount)); diff --git a/Kernel/GlobalProcessExposed.cpp b/Kernel/GlobalProcessExposed.cpp index 8e05187f88..8d158a0b69 100644 --- a/Kernel/GlobalProcessExposed.cpp +++ b/Kernel/GlobalProcessExposed.cpp @@ -365,15 +365,23 @@ private: fs_object.add("readonly", fs.is_readonly()); fs_object.add("mount_flags", mount.flags()); - if (fs.is_file_backed()) - fs_object.add("source", static_cast(fs).file_description().absolute_path()); - else + if (fs.is_file_backed()) { + auto pseudo_path_or_error = static_cast(fs).file_description().pseudo_path(); + if (pseudo_path_or_error.is_error()) { + // We're probably out of memory and should not attempt to continue. + result = pseudo_path_or_error.error(); + return IterationDecision::Break; + } + fs_object.add("source", pseudo_path_or_error.value()->characters()); + } else { fs_object.add("source", "none"); + } return IterationDecision::Continue; }); - array.finish(); - return KSuccess; + if (result == KSuccess) + array.finish(); + return result; } }; diff --git a/Kernel/Net/IPv4Socket.cpp b/Kernel/Net/IPv4Socket.cpp index 326c2452e4..9eaebce437 100644 --- a/Kernel/Net/IPv4Socket.cpp +++ b/Kernel/Net/IPv4Socket.cpp @@ -456,10 +456,10 @@ bool IPv4Socket::did_receive(const IPv4Address& source_address, u16 source_port, return true; } -String IPv4Socket::absolute_path(const OpenFileDescription&) const +KResultOr> IPv4Socket::pseudo_path(const OpenFileDescription&) const { if (m_role == Role::None) - return "socket"; + return KString::try_create("socket"sv); StringBuilder builder; builder.append("socket:"); @@ -485,7 +485,7 @@ String IPv4Socket::absolute_path(const OpenFileDescription&) const VERIFY_NOT_REACHED(); } - return builder.to_string(); + return KString::try_create(builder.to_string()); } KResult IPv4Socket::setsockopt(int level, int option, Userspace user_value, socklen_t user_value_size) diff --git a/Kernel/Net/IPv4Socket.h b/Kernel/Net/IPv4Socket.h index 2cd90350e9..bf122114be 100644 --- a/Kernel/Net/IPv4Socket.h +++ b/Kernel/Net/IPv4Socket.h @@ -61,7 +61,7 @@ public: IPv4SocketTuple tuple() const { return IPv4SocketTuple(m_local_address, m_local_port, m_peer_address, m_peer_port); } - String absolute_path(const OpenFileDescription& description) const override; + KResultOr> pseudo_path(const OpenFileDescription& description) const override; u8 type_of_service() const { return m_type_of_service; } u8 ttl() const { return m_ttl; } diff --git a/Kernel/Net/LocalSocket.cpp b/Kernel/Net/LocalSocket.cpp index fd6895237c..47e428c953 100644 --- a/Kernel/Net/LocalSocket.cpp +++ b/Kernel/Net/LocalSocket.cpp @@ -355,7 +355,7 @@ StringView LocalSocket::socket_path() const return m_path->view(); } -String LocalSocket::absolute_path(const OpenFileDescription& description) const +KResultOr> LocalSocket::pseudo_path(const OpenFileDescription& description) const { StringBuilder builder; builder.append("socket:"); @@ -378,7 +378,7 @@ String LocalSocket::absolute_path(const OpenFileDescription& description) const break; } - return builder.to_string(); + return KString::try_create(builder.to_string()); } KResult LocalSocket::getsockopt(OpenFileDescription& description, int level, int option, Userspace value, Userspace value_size) diff --git a/Kernel/Net/LocalSocket.h b/Kernel/Net/LocalSocket.h index 49279ee6e8..97705f1018 100644 --- a/Kernel/Net/LocalSocket.h +++ b/Kernel/Net/LocalSocket.h @@ -32,7 +32,7 @@ public: static void for_each(Function); StringView socket_path() const; - String absolute_path(const OpenFileDescription& description) const override; + KResultOr> pseudo_path(const OpenFileDescription& description) const override; // ^Socket virtual KResult bind(Userspace, socklen_t) override; diff --git a/Kernel/Net/Socket.h b/Kernel/Net/Socket.h index ef67cd6bd8..d623bdb5fd 100644 --- a/Kernel/Net/Socket.h +++ b/Kernel/Net/Socket.h @@ -105,7 +105,7 @@ public: virtual KResultOr read(OpenFileDescription&, u64, UserOrKernelBuffer&, size_t) override final; virtual KResultOr write(OpenFileDescription&, u64, const UserOrKernelBuffer&, size_t) override final; virtual KResult stat(::stat&) const override; - virtual String absolute_path(const OpenFileDescription&) const override = 0; + virtual KResultOr> pseudo_path(const OpenFileDescription&) const override = 0; bool has_receive_timeout() const { return m_receive_timeout != Time::zero(); } const Time& receive_timeout() const { return m_receive_timeout; } diff --git a/Kernel/ProcessSpecificExposed.cpp b/Kernel/ProcessSpecificExposed.cpp index be8014f8e4..06504327b4 100644 --- a/Kernel/ProcessSpecificExposed.cpp +++ b/Kernel/ProcessSpecificExposed.cpp @@ -80,9 +80,10 @@ KResultOr> Process::lookup_stacks_directory(const ProcFS& p KResultOr Process::procfs_get_file_description_link(unsigned fd, KBufferBuilder& builder) const { auto file_description = TRY(m_fds.open_file_description(fd)); - auto data = file_description->absolute_path(); - TRY(builder.append(data)); - return data.length(); + // Note: These links are not guaranteed to point to actual VFS paths, just like in other kernels. + auto data = TRY(file_description->pseudo_path()); + TRY(builder.append(data->view())); + return data->length(); } KResult Process::traverse_file_descriptions_directory(unsigned fsid, Function callback) const @@ -187,7 +188,9 @@ KResult Process::procfs_get_fds_stats(KBufferBuilder& builder) const RefPtr description = file_description_metadata.description(); auto description_object = array.add_object(); description_object.add("fd", count); - description_object.add("absolute_path", description->absolute_path()); + // TODO: Better OOM handling. + auto pseudo_path_or_error = description->pseudo_path(); + description_object.add("absolute_path", pseudo_path_or_error.is_error() ? "???"sv : pseudo_path_or_error.value()->view()); description_object.add("seekable", description->file().is_seekable()); description_object.add("class", description->file().class_name()); description_object.add("offset", description->offset()); diff --git a/Kernel/Syscalls/execve.cpp b/Kernel/Syscalls/execve.cpp index dd94d6fb20..32c81c896d 100644 --- a/Kernel/Syscalls/execve.cpp +++ b/Kernel/Syscalls/execve.cpp @@ -275,7 +275,7 @@ static KResultOr load_elf_object(NonnullOwnPtr size_t master_tls_alignment = 0; FlatPtr load_base_address = 0; - auto elf_name = TRY(object_description.try_serialize_absolute_path()); + auto elf_name = TRY(object_description.pseudo_path()); VERIFY(!Processor::in_critical()); Memory::MemoryManager::enter_address_space(*new_space); @@ -438,7 +438,9 @@ KResult Process::do_exec(NonnullRefPtr main_program_descrip { VERIFY(is_user_process()); VERIFY(!Processor::in_critical()); - auto path = TRY(main_program_description->try_serialize_absolute_path()); + // Although we *could* handle a pseudo_path here, trying to execute something that doesn't have + // a custody (e.g. BlockDevice or RandomDevice) is pretty suspicious anyway. + auto path = TRY(main_program_description->original_absolute_path()); dbgln_if(EXEC_DEBUG, "do_exec: {}", path); diff --git a/Kernel/Syscalls/mount.cpp b/Kernel/Syscalls/mount.cpp index b0a3709bf3..343aae3d72 100644 --- a/Kernel/Syscalls/mount.cpp +++ b/Kernel/Syscalls/mount.cpp @@ -70,7 +70,8 @@ KResultOr Process::sys$mount(Userspace return ENODEV; } - dbgln("mount: attempting to mount {} on {}", description->absolute_path(), target); + auto source_pseudo_path = TRY(description->pseudo_path()); + dbgln("mount: attempting to mount {} on {}", source_pseudo_path, target); fs = TRY(Ext2FS::try_create(*description)); } else if (fs_type == "9p"sv || fs_type == "Plan9FS"sv) { @@ -96,7 +97,8 @@ KResultOr Process::sys$mount(Userspace dbgln("mount: this is not a seekable file"); return ENODEV; } - dbgln("mount: attempting to mount {} on {}", description->absolute_path(), target); + auto source_pseudo_path = TRY(description->pseudo_path()); + dbgln("mount: attempting to mount {} on {}", source_pseudo_path, target); fs = TRY(ISO9660FS::try_create(*description)); } else { return ENODEV; diff --git a/Kernel/Syscalls/statvfs.cpp b/Kernel/Syscalls/statvfs.cpp index 8764f98a9b..6cfbdc6602 100644 --- a/Kernel/Syscalls/statvfs.cpp +++ b/Kernel/Syscalls/statvfs.cpp @@ -77,7 +77,9 @@ KResultOr Process::sys$fstatvfs(int fd, statvfs* buf) REQUIRE_PROMISE(stdio); auto description = TRY(fds().open_file_description(fd)); - return do_statvfs(description->absolute_path(), buf); + 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); } } diff --git a/Kernel/TTY/MasterPTY.cpp b/Kernel/TTY/MasterPTY.cpp index d669ea60b7..ba5e2db5c2 100644 --- a/Kernel/TTY/MasterPTY.cpp +++ b/Kernel/TTY/MasterPTY.cpp @@ -127,9 +127,10 @@ KResult MasterPTY::ioctl(OpenFileDescription& description, unsigned request, Use return EINVAL; } -String MasterPTY::absolute_path(const OpenFileDescription&) const +KResultOr> MasterPTY::pseudo_path(const OpenFileDescription&) const { - return String::formatted("ptm:{}", m_pts_name); + // FIXME: Replace this and others of this pattern by KString::formatted() + return KString::try_create(String::formatted("ptm:{}", m_pts_name)); } } diff --git a/Kernel/TTY/MasterPTY.h b/Kernel/TTY/MasterPTY.h index 7f6c5732c7..38a9cbc747 100644 --- a/Kernel/TTY/MasterPTY.h +++ b/Kernel/TTY/MasterPTY.h @@ -26,7 +26,7 @@ public: void notify_slave_closed(Badge); bool is_closed() const { return m_closed; } - virtual String absolute_path(const OpenFileDescription&) const override; + virtual KResultOr> pseudo_path(const OpenFileDescription&) const override; private: explicit MasterPTY(unsigned index, NonnullOwnPtr buffer); diff --git a/Kernel/TTY/TTY.cpp b/Kernel/TTY/TTY.cpp index 0d3e6aec38..7e4c2f26c8 100644 --- a/Kernel/TTY/TTY.cpp +++ b/Kernel/TTY/TTY.cpp @@ -576,6 +576,11 @@ KResult TTY::ioctl(OpenFileDescription&, unsigned request, Userspace arg) return EINVAL; } +KResultOr> TTY::pseudo_path(const OpenFileDescription&) const +{ + return KString::try_create(tty_name()); +} + void TTY::set_size(unsigned short columns, unsigned short rows) { m_rows = rows; diff --git a/Kernel/TTY/TTY.h b/Kernel/TTY/TTY.h index 87b5bc5f40..3d2cc760a2 100644 --- a/Kernel/TTY/TTY.h +++ b/Kernel/TTY/TTY.h @@ -26,7 +26,7 @@ public: virtual bool can_read(const OpenFileDescription&, size_t) const override; virtual bool can_write(const OpenFileDescription&, size_t) const override; virtual KResult ioctl(OpenFileDescription&, unsigned request, Userspace arg) override final; - virtual String absolute_path(const OpenFileDescription&) const override { return tty_name(); } + virtual KResultOr> pseudo_path(const OpenFileDescription&) const override; virtual String const& tty_name() const = 0;