From 39a5c5208810aeff1c540f49983282ff5954f2c1 Mon Sep 17 00:00:00 2001 From: Bananymous Date: Fri, 8 Sep 2023 11:46:53 +0300 Subject: [PATCH] Kernel: Fix directory permissions We did not care about X bit in directories and instead used only the R bit for search/read. --- kernel/include/kernel/OpenFileDescriptorSet.h | 2 + kernel/include/kernel/Process.h | 2 + kernel/kernel/FS/Inode.cpp | 2 +- kernel/kernel/FS/VirtualFileSystem.cpp | 4 +- kernel/kernel/OpenFileDescriptorSet.cpp | 42 +++++++++++++++++-- kernel/kernel/Process.cpp | 16 +++++++ kernel/kernel/Syscall.cpp | 6 +++ libc/dirent.cpp | 2 +- libc/include/sys/syscall.h | 2 + libc/sys/stat.cpp | 29 ++----------- 10 files changed, 73 insertions(+), 34 deletions(-) diff --git a/kernel/include/kernel/OpenFileDescriptorSet.h b/kernel/include/kernel/OpenFileDescriptorSet.h index c4551be4dd..17057d2f81 100644 --- a/kernel/include/kernel/OpenFileDescriptorSet.h +++ b/kernel/include/kernel/OpenFileDescriptorSet.h @@ -34,6 +34,8 @@ namespace Kernel BAN::ErrorOr tell(int) const; BAN::ErrorOr fstat(int fd, struct stat*) const; + BAN::ErrorOr fstatat(int fd, BAN::StringView path, struct stat* buf, int flag); + BAN::ErrorOr stat(BAN::StringView absolute_path, struct stat* buf, int flag); BAN::ErrorOr close(int); void close_all(); diff --git a/kernel/include/kernel/Process.h b/kernel/include/kernel/Process.h index a77c97594d..97448a1050 100644 --- a/kernel/include/kernel/Process.h +++ b/kernel/include/kernel/Process.h @@ -106,6 +106,8 @@ namespace Kernel BAN::ErrorOr sys_tell(int fd); BAN::ErrorOr sys_fstat(int fd, struct stat*); + BAN::ErrorOr sys_fstatat(int fd, const char* path, struct stat* buf, int flag); + BAN::ErrorOr sys_stat(const char* path, struct stat* buf, int flag); BAN::ErrorOr mount(BAN::StringView source, BAN::StringView target); diff --git a/kernel/kernel/FS/Inode.cpp b/kernel/kernel/FS/Inode.cpp index cfd24bbae9..efc9dcee21 100644 --- a/kernel/kernel/FS/Inode.cpp +++ b/kernel/kernel/FS/Inode.cpp @@ -38,7 +38,7 @@ namespace Kernel } } - if (flags & O_EXEC) + if (flags & (O_EXEC | O_SEARCH)) { if (mode().mode & S_IXOTH) { } diff --git a/kernel/kernel/FS/VirtualFileSystem.cpp b/kernel/kernel/FS/VirtualFileSystem.cpp index 0c519c87d9..32c978d82b 100644 --- a/kernel/kernel/FS/VirtualFileSystem.cpp +++ b/kernel/kernel/FS/VirtualFileSystem.cpp @@ -133,7 +133,7 @@ namespace Kernel } else { - if (!inode->can_access(credentials, O_RDONLY)) + if (!inode->can_access(credentials, O_SEARCH)) return BAN::Error::from_errno(EACCES); inode = TRY(inode->directory_find_inode(path_part)); @@ -152,7 +152,7 @@ namespace Kernel auto target = TRY(inode->link_target()); if (target.empty()) return BAN::Error::from_errno(ENOENT); - + if (target.front() == '/') { inode = root_inode(); diff --git a/kernel/kernel/OpenFileDescriptorSet.cpp b/kernel/kernel/OpenFileDescriptorSet.cpp index bf3c19c741..6c79938ce3 100644 --- a/kernel/kernel/OpenFileDescriptorSet.cpp +++ b/kernel/kernel/OpenFileDescriptorSet.cpp @@ -179,11 +179,8 @@ namespace Kernel return m_open_files[fd]->offset; } - BAN::ErrorOr OpenFileDescriptorSet::fstat(int fd, struct stat* out) const + static void read_stat_from_inode(BAN::RefPtr inode, struct stat* out) { - TRY(validate_fd(fd)); - - auto inode = m_open_files[fd]->inode; out->st_dev = inode->dev(); out->st_ino = inode->ino(); out->st_mode = inode->mode().mode; @@ -197,7 +194,42 @@ namespace Kernel out->st_ctim = inode->ctime(); out->st_blksize = inode->blksize(); out->st_blocks = inode->blocks(); + } + BAN::ErrorOr OpenFileDescriptorSet::fstat(int fd, struct stat* out) const + { + TRY(validate_fd(fd)); + read_stat_from_inode(m_open_files[fd]->inode, out); + return {}; + } + + BAN::ErrorOr OpenFileDescriptorSet::fstatat(int fd, BAN::StringView path, struct stat* out, int flag) + { + if (flag & ~AT_SYMLINK_NOFOLLOW) + return BAN::Error::from_errno(EINVAL); + if (flag == AT_SYMLINK_NOFOLLOW) + flag = O_NOFOLLOW; + + BAN::String absolute_path; + TRY(absolute_path.append(TRY(path_of(fd)))); + TRY(absolute_path.push_back('/')); + TRY(absolute_path.append(path)); + + // FIXME: handle O_SEARCH in fd + auto file = TRY(VirtualFileSystem::get().file_from_absolute_path(m_credentials, absolute_path, flag)); + read_stat_from_inode(file.inode, out); + + return {}; + } + + BAN::ErrorOr OpenFileDescriptorSet::stat(BAN::StringView absolute_path, struct stat* out, int flag) + { + if (flag & ~AT_SYMLINK_NOFOLLOW) + return BAN::Error::from_errno(EINVAL); + if (flag == AT_SYMLINK_NOFOLLOW) + flag = O_NOFOLLOW; + auto file = TRY(VirtualFileSystem::get().file_from_absolute_path(m_credentials, absolute_path, flag)); + read_stat_from_inode(file.inode, out); return {}; } @@ -256,6 +288,8 @@ namespace Kernel { TRY(validate_fd(fd)); auto& open_file = m_open_files[fd]; + if (!(open_file->flags & O_RDONLY)) + return BAN::Error::from_errno(EACCES); TRY(open_file->inode->directory_read_next_entries(open_file->offset, list, list_size)); open_file->offset++; return {}; diff --git a/kernel/kernel/Process.cpp b/kernel/kernel/Process.cpp index 99d4bbebe1..ed5fa75d9b 100644 --- a/kernel/kernel/Process.cpp +++ b/kernel/kernel/Process.cpp @@ -630,6 +630,8 @@ namespace Kernel { LockGuard _(m_lock); + // FIXME: handle O_SEARCH in fd + BAN::String absolute_path; TRY(absolute_path.append(TRY(m_open_file_descriptors.path_of(fd)))); TRY(absolute_path.push_back('/')); @@ -737,6 +739,20 @@ namespace Kernel return 0; } + BAN::ErrorOr Process::sys_fstatat(int fd, const char* path, struct stat* buf, int flag) + { + LockGuard _(m_lock); + TRY(m_open_file_descriptors.fstatat(fd, path, buf, flag)); + return 0; + } + + BAN::ErrorOr Process::sys_stat(const char* path, struct stat* buf, int flag) + { + LockGuard _(m_lock); + TRY(m_open_file_descriptors.stat(TRY(absolute_path_of(path)), buf, flag)); + return 0; + } + BAN::ErrorOr Process::sys_read_dir_entries(int fd, DirectoryEntryList* list, size_t list_size) { LockGuard _(m_lock); diff --git a/kernel/kernel/Syscall.cpp b/kernel/kernel/Syscall.cpp index a6e147301f..b60cd3236a 100644 --- a/kernel/kernel/Syscall.cpp +++ b/kernel/kernel/Syscall.cpp @@ -185,6 +185,12 @@ namespace Kernel case SYS_NANOSLEEP: ret = Process::current().sys_nanosleep((const timespec*)arg1, (timespec*)arg2); break; + case SYS_FSTATAT: + ret = Process::current().sys_fstatat((int)arg1, (const char*)arg2, (struct stat*)arg3, (int)arg4); + break; + case SYS_STAT: + ret = Process::current().sys_stat((const char*)arg1, (struct stat*)arg2, (int)arg3); + break; default: dwarnln("Unknown syscall {}", syscall); break; diff --git a/libc/dirent.cpp b/libc/dirent.cpp index 16f9f6ede1..b8c94e4442 100644 --- a/libc/dirent.cpp +++ b/libc/dirent.cpp @@ -58,7 +58,7 @@ DIR* fdopendir(int fd) DIR* opendir(const char* dirname) { - int fd = open(dirname, O_SEARCH); + int fd = open(dirname, O_RDONLY); if (fd == -1) return nullptr; return fdopendir(fd); diff --git a/libc/include/sys/syscall.h b/libc/include/sys/syscall.h index a3f0ba53b0..9d16a1c23c 100644 --- a/libc/include/sys/syscall.h +++ b/libc/include/sys/syscall.h @@ -52,6 +52,8 @@ __BEGIN_DECLS #define SYS_SET_PGID 45 #define SYS_FCNTL 46 #define SYS_NANOSLEEP 47 +#define SYS_FSTATAT 48 +#define SYS_STAT 49 // stat/lstat __END_DECLS diff --git a/libc/sys/stat.cpp b/libc/sys/stat.cpp index 3f977f2a6f..457e65ab67 100644 --- a/libc/sys/stat.cpp +++ b/libc/sys/stat.cpp @@ -11,38 +11,15 @@ int fstat(int fildes, struct stat* buf) int fstatat(int fd, const char* __restrict path, struct stat* __restrict buf, int flag) { - if (flag == AT_SYMLINK_NOFOLLOW) - flag = O_NOFOLLOW; - else if (flag) - { - errno = EINVAL; - return -1; - } - - int target = openat(fd, path, O_SEARCH | flag); - if (target == -1) - return -1; - int ret = fstat(target, buf); - close(target); - return ret; + return syscall(SYS_FSTATAT, fd, path, buf, flag); } int lstat(const char* __restrict path, struct stat* __restrict buf) { - int fd = open(path, O_SEARCH | O_NOFOLLOW); - if (fd == -1) - return -1; - int ret = fstat(fd, buf); - close(fd); - return ret; + return syscall(SYS_STAT, path, buf, AT_SYMLINK_NOFOLLOW); } int stat(const char* __restrict path, struct stat* __restrict buf) { - int fd = open(path, O_SEARCH); - if (fd == -1) - return -1; - int ret = fstat(fd, buf); - close(fd); - return ret; + return syscall(SYS_STAT, path, buf, 0); }