Merge pull request #9732 from simonbronk/fix/libkernel-vector-io-argument-order

fix(libkernel): check vector I/O lengths and descriptors before offsets

core/libs/prx/libkernel/File/include/DescriptorControl.hpp

@@ -33,6 +33,7 @@ private:
 };
 bool IsVnode(int fd);
 bool IsCharacterDevice(int fd);
+bool GrantsAccess(int fd, bool write);
 
 }
 

core/libs/prx/libkernel/File/src/DescriptorControl.cpp

@@ -647,6 +647,11 @@ bool IsCharacterDevice(int fd) {
 #endif
 }
 
+bool GrantsAccess(int fd, bool write) {
+    const int access = AccessMode(fd, write ? "pwrite" : "pread");
+    return access == ReadWrite || access == (write ? WriteOnly : ReadOnly);
+}
+
 bool IsVnode(int fd) {
     if (fd < 0 || File::IsRandomDevice(fd)) return false;
     if (File::IsArchivedFile(fd)) return true;

core/libs/prx/libkernel/File/src/Stdio.cpp

@@ -462,10 +462,17 @@ static bool IsPipeDescriptor(int descriptor) {
 #endif
 }
 
-static int PositionedArgumentError(int descriptor, bool nullBuffer, std::int64_t offset) {
+static bool IsSeekableDescriptor(int descriptor) {
+    if (GuestSockets::IsOpen(descriptor)) return false;
+    return File::IsVnode(descriptor) || File::IsCharacterDevice(descriptor) || File::IsRandomDevice(descriptor);
+}
+
+static int PositionedArgumentError(int descriptor, bool write, bool nullBuffer, std::int64_t offset) {
+    if (IsKqueueDescriptor(descriptor)) return GUEST_ESPIPE;
     if (!nullBuffer && offset >= 0) return 0;
     if (!GuestSockets::IsOpen(descriptor) && !DescriptorIsOpen_nid_no_patch(descriptor)) return GUEST_EBADF;
-    if (GuestSockets::IsOpen(descriptor) || IsPipeDescriptor(descriptor)) return GUEST_ESPIPE;
+    if (!IsSeekableDescriptor(descriptor)) return GUEST_ESPIPE;
+    if (!File::GrantsAccess(descriptor, write)) return GUEST_EBADF;
     return offset < 0 ? GUEST_EINVAL : GUEST_EFAULT;
 }
 
@@ -1023,7 +1030,7 @@ int APS5_VABI _open_nid_postfix(const char* path, int flags, ...) {
 
 int64_t APS5_VABI pread_nid_postfix(int d, void* buf, size_t nbytes, int64_t offset) {
     if (ExceedsTransferLimit(nbytes)) return PosixFailure(GUEST_EINVAL);
-    if (const int error = PositionedArgumentError(d, buf == nullptr && nbytes != 0, offset)) return PosixFailure(error);
+    if (const int error = PositionedArgumentError(d, false, buf == nullptr && nbytes != 0, offset)) return PosixFailure(error);
     if (FailPosixPositionedGuestSocket(d)) return -1;
     const GuestArena::HostWrite destination(buf, nbytes);
     if (!destination.Open()) return PosixFailure(GUEST_EFAULT);
@@ -1036,7 +1043,7 @@ int64_t APS5_VABI pread_nid_postfix(int d, void* buf, size_t nbytes, int64_t off
 
 int64_t APS5_VABI pwrite_nid_disambig1_nid_postfix(int d, const void* buf, size_t nbytes, int64_t offset) {
     if (ExceedsTransferLimit(nbytes)) return PosixFailure(GUEST_EINVAL);
-    if (const int error = PositionedArgumentError(d, buf == nullptr && nbytes != 0, offset)) return PosixFailure(error);
+    if (const int error = PositionedArgumentError(d, true, buf == nullptr && nbytes != 0, offset)) return PosixFailure(error);
     if (FailPosixPositionedGuestSocket(d)) return -1;
     const File::WriteSync sync(d);
     GuestArena::GuestArenaPrepareHostRead_nid_no_patch(buf, nbytes);
@@ -1333,6 +1340,11 @@ static int CopyIovecs(const KernelIovec* iov, int iovcnt, std::vector<KernelIove
 #else
     throw std::runtime_error("iovec: checked guest copies are not implemented on this host");
 #endif
+    std::size_t total = 0;
+    for (const auto& vector : vectors) {
+        if (vector.length > static_cast<std::size_t>(std::numeric_limits<std::int64_t>::max()) - total) return SceErrorFromErrno(GUEST_EINVAL);
+        total += vector.length;
+    }
     return 0;
 }
 
@@ -1359,7 +1371,7 @@ static void PrepareWriteIovecs(const KernelIovec* iov, int iovcnt) {
 #ifdef _WIN32
 
 int64_t APS5_VABI sceKernelPread(int d, void* buf, size_t nbytes, int64_t offset) {
-    if (const int error = PositionedArgumentError(d, buf == nullptr && nbytes != 0, offset)) return SceErrorFromErrno(error);
+    if (const int error = PositionedArgumentError(d, false, buf == nullptr && nbytes != 0, offset)) return SceErrorFromErrno(error);
     if (const auto error = PositionedGuestSocketError(d); error != 0) return error;
     const GuestArena::HostWrite destination(buf, nbytes);
     if (!destination.Open()) return SceErrorFromErrno(GUEST_EFAULT);
@@ -1371,7 +1383,7 @@ int64_t APS5_VABI sceKernelPread(int d, void* buf, size_t nbytes, int64_t offset
 }
 
 int64_t APS5_VABI sceKernelPwrite(int d, const void* buf, size_t nbytes, int64_t offset) {
-    if (const int error = PositionedArgumentError(d, buf == nullptr && nbytes != 0, offset)) return SceErrorFromErrno(error);
+    if (const int error = PositionedArgumentError(d, true, buf == nullptr && nbytes != 0, offset)) return SceErrorFromErrno(error);
     if (const auto error = PositionedGuestSocketError(d); error != 0) return error;
     const File::WriteSync sync(d);
     GuestArena::GuestArenaPrepareHostRead_nid_no_patch(buf, nbytes);
@@ -1384,11 +1396,12 @@ static std::int64_t TransferIovecs(int d, const KernelIovec* guestIov, int iovcn
     std::vector<KernelIovec> vectors;
     if (const int error = CopyIovecs(guestIov, iovcnt, vectors)) return error;
     const auto* iov = vectors.data();
-    if (offset != nullptr && *offset < 0) return SceErrorFromErrno(GUEST_EINVAL);
+    if (offset != nullptr) {
+        if (const int error = PositionedArgumentError(d, write, false, *offset)) return SceErrorFromErrno(error);
+    }
     std::size_t total = 0;
     for (int i = 0; i < iovcnt; ++i) {
         if (iov[i].base == nullptr && iov[i].length != 0) return SceErrorFromErrno(GUEST_EFAULT);
-        if (iov[i].length > static_cast<std::size_t>(std::numeric_limits<std::int64_t>::max()) - total) return SceErrorFromErrno(GUEST_EINVAL);
         total += iov[i].length;
     }
     if (offset != nullptr && total > static_cast<std::size_t>(std::numeric_limits<std::int64_t>::max() - *offset)) return SceErrorFromErrno(GUEST_EINVAL);
@@ -1461,7 +1474,7 @@ int64_t APS5_VABI sceKernelPwritev(int d, const KernelIovec* iov, int iovcnt, in
 #else
 
 int64_t APS5_VABI sceKernelPread(int d, void* buf, size_t nbytes, int64_t offset) {
-    if (const int error = PositionedArgumentError(d, buf == nullptr && nbytes != 0, offset)) return SceErrorFromErrno(error);
+    if (const int error = PositionedArgumentError(d, false, buf == nullptr && nbytes != 0, offset)) return SceErrorFromErrno(error);
     if (const auto error = PositionedGuestSocketError(d); error != 0) return error;
     const GuestArena::HostWrite destination(buf, nbytes);
     if (!destination.Open()) return SceErrorFromErrno(GUEST_EFAULT);
@@ -1472,7 +1485,7 @@ int64_t APS5_VABI sceKernelPread(int d, void* buf, size_t nbytes, int64_t offset
 }
 
 int64_t APS5_VABI sceKernelPwrite(int d, const void* buf, size_t nbytes, int64_t offset) {
-    if (const int error = PositionedArgumentError(d, buf == nullptr && nbytes != 0, offset)) return SceErrorFromErrno(error);
+    if (const int error = PositionedArgumentError(d, true, buf == nullptr && nbytes != 0, offset)) return SceErrorFromErrno(error);
     if (const auto error = PositionedGuestSocketError(d); error != 0) return error;
     const File::WriteSync sync(d);
     GuestArena::GuestArenaPrepareHostRead_nid_no_patch(buf, nbytes);
@@ -1516,7 +1529,7 @@ int64_t APS5_VABI sceKernelPreadv(int d, const KernelIovec* guestIov, int iovcnt
     std::vector<KernelIovec> vectors;
     if (const int error = CopyIovecs(guestIov, iovcnt, vectors)) return error;
     const auto* iov = vectors.data();
-    if (offset < 0) return SceErrorFromErrno(GUEST_EINVAL);
+    if (const int error = PositionedArgumentError(d, false, false, offset)) return SceErrorFromErrno(error);
     if (const auto error = PositionedGuestSocketError(d); error != 0) return error;
     std::deque<GuestArena::HostWrite> destinations;
     if (!OpenIovecs(iov, iovcnt, destinations)) return SceErrorFromErrno(GUEST_EFAULT);
@@ -1530,7 +1543,7 @@ int64_t APS5_VABI sceKernelPwritev(int d, const KernelIovec* guestIov, int iovcn
     std::vector<KernelIovec> vectors;
     if (const int error = CopyIovecs(guestIov, iovcnt, vectors)) return error;
     const auto* iov = vectors.data();
-    if (offset < 0) return SceErrorFromErrno(GUEST_EINVAL);
+    if (const int error = PositionedArgumentError(d, true, false, offset)) return SceErrorFromErrno(error);
     if (const auto error = PositionedGuestSocketError(d); error != 0) return error;
     const File::WriteSync sync(d);
     PrepareWriteIovecs(iov, iovcnt);

core/libs/tests/GuestFileVector.cpp

@@ -14,6 +14,7 @@
 #include <filesystem>
 #include <fstream>
 #include <iterator>
+#include <limits>
 #include <string>
 #include <thread>
 #include <vector>
@@ -279,12 +280,22 @@ int main() {
     const int readOnly = sceKernelOpen(path.string().c_str(), SCE_KERNEL_O_RDONLY, 0);
     Require(readOnly >= 0);
     Require(sceKernelPwrite(readOnly, "X", 1, 0) == ErrorEbadf);
+    Require(sceKernelPwrite(readOnly, "X", 1, -1) == ErrorEbadf);
+    Require(pwrite_nid_disambig1_nid_postfix(readOnly, nullptr, 1, 0) == -1 && *__error_nid_postfix() == 9);
+    Require(sceKernelPwritev(readOnly, writes, 2, -1) == ErrorEbadf);
+    Require(pwritev_nid_postfix(readOnly, writes, 2, -1) == -1 && *__error_nid_postfix() == 9);
+    Require(sceKernelPreadv(readOnly, reads, 2, -1) == ErrorEinval);
     Require(sceKernelFtruncate(readOnly, 0) == ErrorEinval);
     Require(sceKernelClose(readOnly) == 0);
     const int writeOnly = sceKernelOpen(path.string().c_str(), SCE_KERNEL_O_WRONLY, 0);
     Require(writeOnly >= 0);
     Require(sceKernelPread(writeOnly, &byte, 1, 0) == ErrorEbadf);
     Require(pread_nid_postfix(writeOnly, &byte, 1, 0) == -1 && *__error_nid_postfix() == 9);
+    Require(sceKernelPread(writeOnly, &byte, 1, -1) == ErrorEbadf);
+    Require(pread_nid_postfix(writeOnly, nullptr, 1, 0) == -1 && *__error_nid_postfix() == 9);
+    Require(sceKernelPreadv(writeOnly, reads, 2, -1) == ErrorEbadf);
+    Require(preadv_nid_postfix(writeOnly, reads, 2, -1) == -1 && *__error_nid_postfix() == 9);
+    Require(sceKernelPwritev(writeOnly, writes, 2, -1) == ErrorEinval);
     Require(sceKernelClose(writeOnly) == 0);
     Require(Contents(path) == "01ABCDE7ABCDE");
     Require(sceKernelFtruncate(file, 5) == 0);
@@ -298,6 +309,17 @@ int main() {
     Require(sceKernelPread(file, &byte, 1, 0) == ErrorEbadf);
     Require(sceKernelPwrite(file, &byte, 1, 0) == ErrorEbadf);
     Require(sceKernelFtruncate(file, 0) == ErrorEbadf);
+    Require(sceKernelPreadv(file, reads, 2, -1) == ErrorEbadf);
+    Require(sceKernelPwritev(file, writes, 2, -1) == ErrorEbadf);
+    Require(preadv_nid_postfix(file, reads, 2, -1) == -1 && *__error_nid_postfix() == 9);
+    Require(pwritev_nid_postfix(file, writes, 2, -1) == -1 && *__error_nid_postfix() == 9);
+    KernelIovec oversized[2] = {{first, static_cast<std::size_t>(std::numeric_limits<std::int64_t>::max())}, {second, 1}};
+    Require(sceKernelReadv(file, oversized, 2) == ErrorEinval);
+    Require(sceKernelWritev(file, oversized, 2) == ErrorEinval);
+    Require(sceKernelPreadv(file, oversized, 2, 0) == ErrorEinval);
+    Require(sceKernelPwritev(file, oversized, 2, 0) == ErrorEinval);
+    Require(readv_nid_postfix(file, oversized, 2) == -1 && *__error_nid_postfix() == 22);
+    Require(pwritev_nid_postfix(file, oversized, 2, 0) == -1 && *__error_nid_postfix() == 22);
 
     int ends[2] = {};
     Require(MakePipe(ends) == 0);
@@ -317,6 +339,10 @@ int main() {
     Require(sceKernelReadv(ends[0], reads, 2) == 3 && std::memcmp(first, "uvw", 3) == 0);
     Require(sceKernelPreadv(ends[0], reads, 2, 0) == ErrorEspipe);
     Require(sceKernelPwritev(ends[1], message, 1, 0) == ErrorEspipe);
+    Require(sceKernelPreadv(ends[0], reads, 2, -1) == ErrorEspipe);
+    Require(sceKernelPwritev(ends[1], message, 1, -1) == ErrorEspipe);
+    Require(preadv_nid_postfix(ends[0], reads, 2, -1) == -1 && *__error_nid_postfix() == 29);
+    Require(pwritev_nid_postfix(ends[1], message, 1, -1) == -1 && *__error_nid_postfix() == 29);
 #ifndef _WIN32
     Require(::fcntl(ends[0], F_SETFL, ::fcntl(ends[0], F_GETFL) | O_NONBLOCK) == 0);
     Require(sceKernelReadv(ends[0], reads, 2) == ErrorEagain);
@@ -324,6 +350,18 @@ int main() {
 #endif
     Require(ClosePipe(ends[0]) == 0 && ClosePipe(ends[1]) == 0);
 
+    const int queue = kqueue_nid_postfix();
+    Require(queue >= 0);
+    Require(sceKernelPreadv(queue, reads, 2, -1) == ErrorEspipe);
+    Require(sceKernelPwritev(queue, writes, 2, -1) == ErrorEspipe);
+    Require(preadv_nid_postfix(queue, reads, 2, -1) == -1 && *__error_nid_postfix() == 29);
+    Require(pwritev_nid_postfix(queue, writes, 2, -1) == -1 && *__error_nid_postfix() == 29);
+    Require(sceKernelPreadv(queue, reads, 2, 0) == ErrorEspipe);
+    Require(sceKernelPwritev(queue, writes, 2, 0) == ErrorEspipe);
+    Require(sceKernelPread(queue, &byte, 1, 0) == ErrorEspipe);
+    Require(pwrite_nid_disambig1_nid_postfix(queue, &byte, 1, -1) == -1 && *__error_nid_postfix() == 29);
+    Require(sceKernelClose(queue) == 0);
+
     const int random = sceKernelOpen("/dev/urandom", 0, 0);
     Require(random >= 0);
     unsigned char noise[48]{};

docs/dev/debt/libkernel.md

@@ -160,7 +160,7 @@
 - [Linux directory entry types](../../../core/libs/prx/libkernel/File/src/Stdio.cpp) (libkernel) - `sceKernelGetdirentries` reports a host symlink inside the app0 tree as the type of its target (directory or regular file), since a console app0 has no links and links there come from how the title was unpacked; a link elsewhere, or a dangling one, stays a link. A directory opened through an app0 link resolves to its target's host path, so links nested inside that target keep the link type.
 - [ftruncate, truncate, sceKernelFtruncate](../../../core/libs/prx/libkernel/File/src/Stdio.cpp) (libkernel) extend a file on Windows with `SetFileInformationByHandle` (`FileEndOfFileInfo`), which reserves the clusters but writes no zero bytes, and report `EFBIG` for a length the volume cannot hold (`ERROR_INVALID_PARAMETER` on a disk file; seen with a length of `INT64_MAX`, the size from which NTFS refuses it was not measured) and `ENOSPC` when the free space is smaller. FreeBSD 11.4 makes the extension a hole on a file system that has them ([`ffs_truncate`](https://github.com/freebsd/freebsd-src/blob/release/11.4.0/sys/ufs/ffs/ffs_inode.c) writes only the last block), so a length above the free space succeeds there. A write behind the end of the file still makes NTFS write zero bytes over the gap, as the file is not sparse. None of this was measured on a console.
 - [Windows directory entry types](../../../core/libs/prx/libkernel/File/src/DirectoryDescriptor.cpp) (libkernel) - types other than regular files, directories and symbolic links, and reparse points unsupported by `lstat`, throw. If the host cannot stat an entry, its type is `DT_UNKNOWN`. Console directory-entry types have not been measured; the host `lstat` modes map to `DT_REG`, `DT_DIR` and `DT_LNK` following [FreeBSD 11.4 dirent.h](https://github.com/freebsd/freebsd-src/blob/release/11.4.0/sys/sys/dirent.h). A host link inside canonical app0 is reported as its target's type (directory or regular file); a link elsewhere, or a dangling one, stays a link. A directory opened through an app0 link resolves to its target's host path, so links nested inside a target outside app0 keep the link type.
-- [readv, writev, preadv, pwritev and their sceKernel forms](../../../core/libs/prx/libkernel/File/src/Stdio.cpp) (libkernel) - unreadable iovec-table errors follow FreeBSD 11.4's [`copyinuio`](https://github.com/freebsd/freebsd-src/blob/release/11.4.0/sys/kern/subr_uio.c#L401-L434), not console measurements. The table is copied before inspecting its entries; payload-buffer validation is unchanged. A Linux `process_vm_readv` failure other than `EFAULT` throws, including when the host denies that API.
+- [readv, writev, preadv, pwritev and their sceKernel forms](../../../core/libs/prx/libkernel/File/src/Stdio.cpp) (libkernel) - unreadable iovec-table errors follow FreeBSD 11.4's [`copyinuio`](https://github.com/freebsd/freebsd-src/blob/release/11.4.0/sys/kern/subr_uio.c#L401-L434), not console measurements. The table is copied before inspecting its entries; payload-buffer validation is unchanged. A total length above `SSIZE_MAX` fails with `EINVAL` before the descriptor is looked up, and the positioned forms, `pread` and `pwrite` included, report `EBADF` for a descriptor not open in that direction, then `ESPIPE` for a pipe, socket or kqueue, before `EINVAL` or `EFAULT`, as [`kern_preadv`](https://github.com/freebsd/freebsd-src/blob/release/11.4.0/sys/kern/sys_generic.c) does. FreeBSD passes a negative offset on a character device to the driver; here it still returns `EINVAL`. A Linux `process_vm_readv` failure other than `EFAULT` throws, including when the host denies that API.
 - [/dev/random and /dev/urandom](../../../core/libs/prx/libkernel/File/src/RandomDevice.cpp) (libkernel) opened for reading return a descriptor backed by the host null device whose reads are filled with host OS entropy (`getentropy`, `rand_s` on Windows); both paths behave the same, never block, and positioned reads ignore the offset (FreeBSD `random(4)`). Only the exact paths are recognised, `fstat`, `lseek` and `mmap` see the null device, and opening them for writing throws
 - [pthread_attr_getinheritsched, pthread_attr_setinheritsched, pthread_attr_getscope, pthread_attr_setscope, pthread_attr_getstackaddr](../../../core/libs/prx/libkernel/Pthread/Posix/Attr.cpp) (libkernel) follow FreeBSD 11 libthr ([thr_attr.c](https://github.com/freebsd/freebsd-src/blob/5ed7eb0d97ba4436218e810f61bd059acba984c2/lib/libthr/thread/thr_attr.c), [thr_create.c](https://github.com/freebsd/freebsd-src/blob/5ed7eb0d97ba4436218e810f61bd059acba984c2/lib/libthr/thread/thr_create.c)), not measured on the console: the scope defaults to `PTHREAD_SCOPE_SYSTEM` (2), only it and `PTHREAD_SCOPE_PROCESS` (0) are accepted and other values return `EINVAL`, and an inherit value other than `PTHREAD_INHERIT_SCHED` (4) or `PTHREAD_EXPLICIT_SCHED` (0) returns `ENOTSUP`. `pthread_attr_getstackaddr` returns the configured stack address pointer (null by default). A null output of `pthread_attr_getinheritsched` throws, since libthr writes through it unchecked. A thread keeps the scope it was created with, taken from the creating thread under `PTHREAD_INHERIT_SCHED`, and `pthread_attr_get_np` and `scePthreadAttrGet` report it, but every guest thread is a host thread at system scope, so process scope changes nothing
 - [Shared memory](../../../core/libs/prx/libkernel/File/src/SharedMemory.cpp) (libkernel) on Linux implements anonymous and named objects, bounded descriptor writes and seeks, and `shm_unlink` with effective read/write permission checks from FreeBSD 11. Named paths up to 1023 bytes use `/anyps5-shm-` plus SHA-1; collisions are not detected, and the namespace is shared across processes in the host mount namespace. Named creation applies the guest umask without changing the host mask. Linux procfs and `/dev/shm` are required. Operations use an advisory directory lock; external host operations bypass it, unrelated objects serialize, and descriptor operations require two spare host descriptors and compare device/inode with the initial descriptor identity on both the pinned-object and ordinary-file fallback paths. [Guest mappings](../../../core/libs/prx/libkernel/DirectMemory/PosixMemory.cpp) support shared and private mappings of these descriptors through the allocation tracker and pin a duplicate descriptor during validation and mapping; mapping requires one spare host descriptor; file-backed mappings use the write-watch scan fallback because aliases and descriptor writes bypass per-mapping tracking. On Windows, anonymous objects are delete-on-close temporary files and `mmap` maps them as [regular files](../../../core/libs/prx/libkernel/DirectMemory/PosixMemory.cpp); named objects are described in the next entry. Invalid non-null name pointers, console behaviour and title compatibility remain unverified.