From ea5825f2c9e2cc641daa5584b6129c1f7ef3fa9e Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Sat, 16 Jan 2021 15:48:56 +0100 Subject: [PATCH] Kernel+LibC: Make sys$getcwd truncate the result silently This gives us the superpower of knowing the ideal buffer length if it fails. See also https://github.com/SerenityOS/serenity/discussions/4357 --- Kernel/Process.h | 2 +- Kernel/Syscalls/chdir.cpp | 15 +++---- Userland/Libraries/LibC/unistd.cpp | 65 ++++++++++++++++++++++++++++-- 3 files changed, 70 insertions(+), 12 deletions(-) diff --git a/Kernel/Process.h b/Kernel/Process.h index 946aee0594..3d8bfba55b 100644 --- a/Kernel/Process.h +++ b/Kernel/Process.h @@ -263,7 +263,7 @@ public: int sys$select(const Syscall::SC_select_params*); int sys$poll(Userspace); ssize_t sys$get_dir_entries(int fd, void*, ssize_t); - int sys$getcwd(Userspace, ssize_t); + int sys$getcwd(Userspace, size_t); int sys$chdir(Userspace, size_t); int sys$fchdir(int fd); int sys$sleep(unsigned seconds); diff --git a/Kernel/Syscalls/chdir.cpp b/Kernel/Syscalls/chdir.cpp index 46cf3a2459..23555e2cff 100644 --- a/Kernel/Syscalls/chdir.cpp +++ b/Kernel/Syscalls/chdir.cpp @@ -61,17 +61,18 @@ int Process::sys$fchdir(int fd) return 0; } -int Process::sys$getcwd(Userspace buffer, ssize_t size) +int Process::sys$getcwd(Userspace buffer, size_t size) { REQUIRE_PROMISE(rpath); - if (size < 0) - return -EINVAL; + auto path = current_directory().absolute_path(); - if ((size_t)size < path.length() + 1) - return -ERANGE; - if (!copy_to_user(buffer, path.characters(), path.length() + 1)) + + size_t ideal_size = path.length() + 1; + auto size_to_copy = min(ideal_size, size); + if (!copy_to_user(buffer, path.characters(), size_to_copy)) return -EFAULT; - return 0; + // Note: we return the whole size here, not the copied size. + return ideal_size; } } diff --git a/Userland/Libraries/LibC/unistd.cpp b/Userland/Libraries/LibC/unistd.cpp index 718ff6961c..404a944406 100644 --- a/Userland/Libraries/LibC/unistd.cpp +++ b/Userland/Libraries/LibC/unistd.cpp @@ -322,22 +322,79 @@ int fchdir(int fd) char* getcwd(char* buffer, size_t size) { + if (buffer && size == 0) { + // POSIX requires that we set errno to EINVAL here, but in our syscall it makes sense to + // allow "probing" the Kernel with a zero-sized buffer, and it does not return -EINVAL. + // So we have to inject EINVAL here. + errno = EINVAL; + return nullptr; + } + bool self_allocated = false; if (!buffer) { - size = size ? size : PATH_MAX; + size = size ? size : 64; buffer = (char*)malloc(size); self_allocated = true; } + int rc = syscall(SC_getcwd, buffer, size); - if (rc < 0 && self_allocated) { - free(buffer); + if (rc < 0) { + if (self_allocated) + free(buffer); + errno = -rc; + return nullptr; } - __RETURN_WITH_ERRNO(rc, buffer, nullptr); + + size_t actual_size = static_cast(rc); + if (actual_size <= size) { + return buffer; + } + + // If we get here, the current directory path was silently truncated. + + if (!self_allocated) { + // In this case, POSIX causes information loss: the caller cannot learn about the ideal + // buffer size. This is the reason we went with silently truncation instead. + errno = ERANGE; + return nullptr; + } + + // Try again. + free(buffer); + size = actual_size; + buffer = (char*)malloc(size); + rc = syscall(SC_getcwd, buffer, size); + if (rc < 0) { + // Can only happen if we lose a race. Let's pretend we lost the race in the first place. + free(buffer); + errno = -rc; + return nullptr; + } + + actual_size = static_cast(rc); + if (actual_size < size) { + // If we're here, then cwd has become longer while we were looking at it. (Race with another thread?) + // There's not much we can do, unless we want to loop endlessly + // in this case. Let's leave it up to the caller whether to loop. + free(buffer); + errno = EAGAIN; + return nullptr; + } + + return buffer; } char* getwd(char* buf) { + if (buf == nullptr) { + errno = EINVAL; + return nullptr; + } auto* p = getcwd(buf, PATH_MAX); + if (errno == ERANGE) { + // POSIX quirk + errno = ENAMETOOLONG; + } return p; }