From 6466c3d750da0ddc46498c4e90f8ff8b0972ca65 Mon Sep 17 00:00:00 2001 From: Sergey Bugaev Date: Sun, 19 Jan 2020 01:04:48 +0300 Subject: [PATCH] Kernel: Pass correct permission flags when opening files Right now, permission flags passed to VFS::open() are effectively ignored, but that is going to change. * O_RDONLY is 0, but it's still nicer to pass it explicitly * POSIX says that binding a Unix socket to a symlink shall fail with EADDRINUSE --- Kernel/KSyms.cpp | 2 +- Kernel/Net/LocalSocket.cpp | 4 +-- Kernel/Process.cpp | 2 +- Tests/Kernel/bind-local-socket-to-symlink.cpp | 31 +++++++++++++++++++ 4 files changed, 35 insertions(+), 4 deletions(-) create mode 100644 Tests/Kernel/bind-local-socket-to-symlink.cpp diff --git a/Kernel/KSyms.cpp b/Kernel/KSyms.cpp index 9cdb4e9e00..cf9be648b6 100644 --- a/Kernel/KSyms.cpp +++ b/Kernel/KSyms.cpp @@ -182,7 +182,7 @@ void dump_backtrace() void load_ksyms() { - auto result = VFS::the().open("/res/kernel.map", 0, 0, VFS::the().root_custody()); + auto result = VFS::the().open("/res/kernel.map", O_RDONLY, 0, VFS::the().root_custody()); ASSERT(!result.is_error()); auto description = result.value(); auto buffer = description->read_entire_file(); diff --git a/Kernel/Net/LocalSocket.cpp b/Kernel/Net/LocalSocket.cpp index c49f363985..48c6fbfb31 100644 --- a/Kernel/Net/LocalSocket.cpp +++ b/Kernel/Net/LocalSocket.cpp @@ -111,7 +111,7 @@ KResult LocalSocket::bind(const sockaddr* user_address, socklen_t address_size) mode_t mode = S_IFSOCK | (m_prebind_mode & 04777); UidAndGid owner { m_prebind_uid, m_prebind_gid }; - auto result = VFS::the().open(path, O_CREAT | O_EXCL, mode, current->process().current_directory(), owner); + auto result = VFS::the().open(path, O_RDWR | O_CREAT | O_EXCL | O_NOFOLLOW_NOERROR, mode, current->process().current_directory(), owner); if (result.is_error()) { if (result.error() == -EEXIST) return KResult(-EADDRINUSE); @@ -145,7 +145,7 @@ KResult LocalSocket::connect(FileDescription& description, const sockaddr* addre kprintf("%s(%u) LocalSocket{%p} connect(%s)\n", current->process().name().characters(), current->pid(), this, safe_address); #endif - auto description_or_error = VFS::the().open(safe_address, 0, 0, current->process().current_directory()); + auto description_or_error = VFS::the().open(safe_address, O_RDWR, 0, current->process().current_directory()); if (description_or_error.is_error()) return KResult(-ECONNREFUSED); diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index f1cda814f3..ca8c1caceb 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -4222,7 +4222,7 @@ int Process::sys$module_load(const char* user_path, size_t path_length) auto path = get_syscall_path_argument(user_path, path_length); if (path.is_error()) return path.error(); - auto description_or_error = VFS::the().open(path.value(), 0, 0, current_directory()); + auto description_or_error = VFS::the().open(path.value(), O_RDONLY, 0, current_directory()); if (description_or_error.is_error()) return description_or_error.error(); auto& description = description_or_error.value(); diff --git a/Tests/Kernel/bind-local-socket-to-symlink.cpp b/Tests/Kernel/bind-local-socket-to-symlink.cpp new file mode 100644 index 0000000000..424f02633d --- /dev/null +++ b/Tests/Kernel/bind-local-socket-to-symlink.cpp @@ -0,0 +1,31 @@ +#include +#include +#include +#include + +int main(int, char**) +{ + constexpr const char* path = "/tmp/foo"; + int rc = symlink("bar", path); + if (rc < 0) { + perror("symlink"); + return 1; + } + + int fd = socket(AF_UNIX, SOCK_STREAM, 0); + if (fd < 0) { + perror("socket"); + return 1; + } + + struct sockaddr_un addr; + memset(&addr, 0, sizeof(addr)); + addr.sun_family = AF_UNIX; + strncpy(addr.sun_path, path, sizeof(addr.sun_path) - 1); + + rc = bind(fd, (struct sockaddr*)(&addr), sizeof(addr)); + if (rc < 0 && errno == EADDRINUSE) + return 0; + + return 1; +}