From 2c5a378ccc916cc8ffb9eb58bbd6558bdfe7f7a7 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sat, 2 Mar 2019 01:50:34 +0100 Subject: [PATCH] Kernel+Userland: Add symlink() syscall and add "-s" flag to /bin/ln. It's now possible to create symbolic links! :^) This exposed an issue in Ext2FS where we'd write uninitialized data past the end of an inode's content. Fix this by zeroing out the tail end of the last block in a file. --- Kernel/Ext2FileSystem.cpp | 38 +++++++++++++++++++++++++++--------- Kernel/Process.cpp | 9 +++++++++ Kernel/Process.h | 1 + Kernel/Syscall.cpp | 2 ++ Kernel/Syscall.h | 1 + Kernel/VirtualFileSystem.cpp | 25 ++++++++++++++++++++++++ Kernel/VirtualFileSystem.h | 1 + LibC/unistd.cpp | 6 ++++++ LibC/unistd.h | 1 + Userland/ln.cpp | 34 +++++++++++++++++++++++++++++--- 10 files changed, 106 insertions(+), 12 deletions(-) diff --git a/Kernel/Ext2FileSystem.cpp b/Kernel/Ext2FileSystem.cpp index 8b813de1f39..a5cfcd2123f 100644 --- a/Kernel/Ext2FileSystem.cpp +++ b/Kernel/Ext2FileSystem.cpp @@ -13,6 +13,8 @@ //#define EXT2_DEBUG +static const ssize_t max_inline_symlink_length = 60; + Retained Ext2FS::create(Retained&& device) { return adopt(*new Ext2FS(move(device))); @@ -437,10 +439,9 @@ ssize_t Ext2FSInode::read_bytes(off_t offset, ssize_t count, byte* buffer, FileD // Symbolic links shorter than 60 characters are store inline inside the i_block array. // This avoids wasting an entire block on short links. (Most links are short.) - static const unsigned max_inline_symlink_length = 60; if (is_symlink() && size() < max_inline_symlink_length) { ssize_t nread = min((off_t)size() - offset, static_cast(count)); - memcpy(buffer, m_raw_inode.i_block + offset, (size_t)nread); + memcpy(buffer, ((byte*)m_raw_inode.i_block) + offset, (size_t)nread); return nread; } @@ -495,13 +496,24 @@ ssize_t Ext2FSInode::read_bytes(off_t offset, ssize_t count, byte* buffer, FileD ssize_t Ext2FSInode::write_bytes(off_t offset, ssize_t count, const byte* data, FileDescriptor*) { + ASSERT(offset >= 0); + ASSERT(count >= 0); + Locker inode_locker(m_lock); Locker fs_locker(fs().m_lock); - // FIXME: Support writing to symlink inodes. - ASSERT(!is_symlink()); - - ASSERT(offset >= 0); + if (is_symlink()) { + if ((offset + count) < max_inline_symlink_length) { +#ifdef EXT2_DEBUG + dbgprintf("Ext2FSInode: write_bytes poking into i_block array for inline symlink '%s' (%u bytes)\n", String((const char*)data, count).characters(), count); +#endif + memcpy(((byte*)m_raw_inode.i_block) + offset, data, (size_t)count); + if ((offset + count) > m_raw_inode.i_size) + m_raw_inode.i_size = offset + count; + set_metadata_dirty(true); + return count; + } + } const ssize_t block_size = fs().block_size(); size_t old_size = size(); @@ -528,6 +540,8 @@ ssize_t Ext2FSInode::write_bytes(off_t offset, ssize_t count, const byte* data, dword offset_into_first_block = offset % block_size; + dword last_logical_block_index_in_file = size() / block_size; + ssize_t nwritten = 0; size_t remaining_count = min((off_t)count, (off_t)new_size - offset); const byte* in = data; @@ -542,7 +556,7 @@ ssize_t Ext2FSInode::write_bytes(off_t offset, ssize_t count, const byte* data, size_t num_bytes_to_copy = min((size_t)block_size - offset_into_block, remaining_count); ByteBuffer block; - if (offset_into_block != 0) { + if (offset_into_block != 0 || num_bytes_to_copy != block_size) { block = fs().read_block(block_list[bi]); if (!block) { kprintf("Ext2FSInode::write_bytes: read_block(%u) failed (lbi: %u)\n", block_list[bi], bi); @@ -552,8 +566,14 @@ ssize_t Ext2FSInode::write_bytes(off_t offset, ssize_t count, const byte* data, block = buffer_block; memcpy(block.pointer() + offset_into_block, in, num_bytes_to_copy); - if (offset_into_block == 0 && !num_bytes_to_copy) - memset(block.pointer() + num_bytes_to_copy, 0, block_size - num_bytes_to_copy); + if (bi == last_logical_block_index_in_file && num_bytes_to_copy < block_size) { + size_t padding_start = new_size % block_size; + size_t padding_bytes = block_size - padding_start; +#ifdef EXT2_DEBUG + dbgprintf("Ext2FSInode::write_bytes padding last block of file with zero x %u (new_size=%u, offset_into_block=%u, num_bytes_to_copy=%u)\n", padding_bytes, new_size, offset_into_block, num_bytes_to_copy); +#endif + memset(block.pointer() + padding_start, 0, padding_bytes); + } #ifdef EXT2_DEBUG dbgprintf("Ext2FSInode::write_bytes: writing block %u (offset_into_block: %u)\n", block_list[bi], offset_into_block); #endif diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index 7d66b557248..39b8140bce0 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -2144,6 +2144,15 @@ int Process::sys$unlink(const char* pathname) return VFS::the().unlink(String(pathname), cwd_inode()); } +int Process::sys$symlink(const char* target, const char* linkpath) +{ + if (!validate_read_str(target)) + return -EFAULT; + if (!validate_read_str(linkpath)) + return -EFAULT; + return VFS::the().symlink(String(target), String(linkpath), cwd_inode()); +} + int Process::sys$rmdir(const char* pathname) { if (!validate_read_str(pathname)) diff --git a/Kernel/Process.h b/Kernel/Process.h index 7a1085be2a1..ab8439e4aa6 100644 --- a/Kernel/Process.h +++ b/Kernel/Process.h @@ -216,6 +216,7 @@ public: int sys$utime(const char* pathname, const struct utimbuf*); int sys$link(const char* old_path, const char* new_path); int sys$unlink(const char* pathname); + int sys$symlink(const char* target, const char* linkpath); int sys$rmdir(const char* pathname); int sys$read_tsc(dword* lsw, dword* msw); int sys$chmod(const char* pathname, mode_t); diff --git a/Kernel/Syscall.cpp b/Kernel/Syscall.cpp index ea983ea87e5..2c339ea1769 100644 --- a/Kernel/Syscall.cpp +++ b/Kernel/Syscall.cpp @@ -193,6 +193,8 @@ static dword handle(RegisterDump& regs, dword function, dword arg1, dword arg2, return current->sys$link((const char*)arg1, (const char*)arg2); case Syscall::SC_unlink: return current->sys$unlink((const char*)arg1); + case Syscall::SC_symlink: + return current->sys$symlink((const char*)arg1, (const char*)arg2); case Syscall::SC_read_tsc: return current->sys$read_tsc((dword*)arg1, (dword*)arg2); case Syscall::SC_rmdir: diff --git a/Kernel/Syscall.h b/Kernel/Syscall.h index 020463322b9..bdf5dc7fcbf 100644 --- a/Kernel/Syscall.h +++ b/Kernel/Syscall.h @@ -84,6 +84,7 @@ __ENUMERATE_SYSCALL(link) \ __ENUMERATE_SYSCALL(chown) \ __ENUMERATE_SYSCALL(fchmod) \ + __ENUMERATE_SYSCALL(symlink) \ namespace Syscall { diff --git a/Kernel/VirtualFileSystem.cpp b/Kernel/VirtualFileSystem.cpp index 06fe7e7e3ea..4734a24cb80 100644 --- a/Kernel/VirtualFileSystem.cpp +++ b/Kernel/VirtualFileSystem.cpp @@ -431,6 +431,31 @@ KResult VFS::unlink(const String& path, Inode& base) return parent_inode->remove_child(FileSystemPath(path).basename()); } +KResult VFS::symlink(const String& target, const String& linkpath, Inode& base) +{ + RetainPtr parent_inode; + auto existing_file_or_error = resolve_path_to_inode(linkpath, base, &parent_inode); + if (!existing_file_or_error.is_error()) + return KResult(-EEXIST); + if (!parent_inode) + return KResult(-ENOENT); + if (existing_file_or_error.error() != -ENOENT) + return existing_file_or_error.error(); + if (!parent_inode->metadata().may_write(*current)) + return KResult(-EACCES); + + FileSystemPath p(linkpath); + dbgprintf("VFS::symlink: '%s' (-> '%s') in %u:%u\n", p.basename().characters(), target.characters(), parent_inode->fsid(), parent_inode->index()); + int error; + auto new_file = parent_inode->fs().create_inode(parent_inode->identifier(), p.basename(), 0120644, 0, error); + if (!new_file) + return KResult(error); + ssize_t nwritten = new_file->write_bytes(0, target.length(), (const byte*)target.characters(), nullptr); + if (nwritten < 0) + return KResult(nwritten); + return KSuccess; +} + KResult VFS::rmdir(const String& path, Inode& base) { RetainPtr parent_inode; diff --git a/Kernel/VirtualFileSystem.h b/Kernel/VirtualFileSystem.h index e054493679b..9ea065ede1c 100644 --- a/Kernel/VirtualFileSystem.h +++ b/Kernel/VirtualFileSystem.h @@ -68,6 +68,7 @@ public: KResult mkdir(const String& path, mode_t mode, Inode& base); KResult link(const String& old_path, const String& new_path, Inode& base); KResult unlink(const String& path, Inode& base); + KResult symlink(const String& target, const String& linkpath, Inode& base); KResult rmdir(const String& path, Inode& base); KResult chmod(const String& path, mode_t, Inode& base); KResult chmod(Inode&, mode_t); diff --git a/LibC/unistd.cpp b/LibC/unistd.cpp index 51d734b3b22..6a749e3aa13 100644 --- a/LibC/unistd.cpp +++ b/LibC/unistd.cpp @@ -247,6 +247,12 @@ int unlink(const char* pathname) __RETURN_WITH_ERRNO(rc, rc, -1); } +int symlink(const char* target, const char* linkpath) +{ + int rc = syscall(SC_symlink, target, linkpath); + __RETURN_WITH_ERRNO(rc, rc, -1); +} + int rmdir(const char* pathname) { int rc = syscall(SC_rmdir, pathname); diff --git a/LibC/unistd.h b/LibC/unistd.h index 930866ade5d..232f11bafca 100644 --- a/LibC/unistd.h +++ b/LibC/unistd.h @@ -62,6 +62,7 @@ int ttyname_r(int fd, char* buffer, size_t); off_t lseek(int fd, off_t, int whence); int link(const char* oldpath, const char* newpath); int unlink(const char* pathname); +int symlink(const char* target, const char* linkpath); int rmdir(const char* pathname); int getdtablesize(); int dup(int old_fd); diff --git a/Userland/ln.cpp b/Userland/ln.cpp index d591be2aff3..b5c460783b0 100644 --- a/Userland/ln.cpp +++ b/Userland/ln.cpp @@ -1,13 +1,41 @@ #include #include #include +#include +#include + +[[noreturn]] static void print_usage_and_exit() +{ + printf("usage: ln [-s] \n"); + exit(0); +} int main(int argc, char** argv) { - if (argc != 3) { - fprintf(stderr, "usage: ln \n"); - return 1; + bool flag_symlink = false; + int opt; + while ((opt = getopt(argc, argv, "s")) != -1) { + switch (opt) { + case 's': + flag_symlink = true; + break; + default: + print_usage_and_exit(); + } } + + if ((optind + 1) >= argc) + print_usage_and_exit(); + + if (flag_symlink) { + int rc = symlink(argv[optind], argv[optind + 1]); + if (rc < 0) { + perror("symlink"); + return 1; + } + return 0; + } + int rc = link(argv[1], argv[2]); if (rc < 0) { perror("link");