From 8e56b86bdd555b2d587177ba9df4a03d98868a82 Mon Sep 17 00:00:00 2001 From: Augustin Cavalier Date: Tue, 25 Jun 2024 21:33:20 -0400 Subject: [PATCH] kernel: Use BStackOrHeapArray in fd & port vector I/O syscalls. The socket syscall needs to still use the heap. Also add a comment around get_iovecs_from_user() indicating what callers must do. --- headers/private/kernel/util/iovec_support.h | 19 +++++++------------ src/system/kernel/fs/fd.cpp | 9 +++++++-- src/system/kernel/fs/socket.cpp | 7 +++++-- src/system/kernel/port.cpp | 11 +++++++---- 4 files changed, 26 insertions(+), 20 deletions(-) diff --git a/headers/private/kernel/util/iovec_support.h b/headers/private/kernel/util/iovec_support.h index 22b46e8859..31bed53881 100644 --- a/headers/private/kernel/util/iovec_support.h +++ b/headers/private/kernel/util/iovec_support.h @@ -41,36 +41,31 @@ generic_memcpy(generic_addr_t dest, bool destPhysical, generic_addr_t src, bool #ifdef IS_USER_ADDRESS +/*! + * Copies an array of `iovec`s from userland. + * Callers must verify vecCount <= IOV_MAX and supply their own vecs buffer. + */ static inline status_t -get_iovecs_from_user(const iovec* userVecs, size_t vecCount, iovec*& vecs, +get_iovecs_from_user(const iovec* userVecs, size_t vecCount, iovec* vecs, bool permitNull = false) { - // prevent integer overflow - if (vecCount > IOV_MAX || vecCount == 0) + if (vecCount == 0) return B_BAD_VALUE; if (!IS_USER_ADDRESS(userVecs)) return B_BAD_ADDRESS; - vecs = (iovec*)malloc(sizeof(iovec) * vecCount); - if (vecs == NULL) - return B_NO_MEMORY; - - if (user_memcpy(vecs, userVecs, sizeof(iovec) * vecCount) != B_OK) { - free(vecs); + if (user_memcpy(vecs, userVecs, sizeof(iovec) * vecCount) != B_OK) return B_BAD_ADDRESS; - } size_t total = 0; for (size_t i = 0; i < vecCount; i++) { if (permitNull && vecs[i].iov_base == NULL) continue; if (!is_user_address_range(vecs[i].iov_base, vecs[i].iov_len)) { - free(vecs); return B_BAD_ADDRESS; } if (vecs[i].iov_len > SSIZE_MAX || total > (SSIZE_MAX - vecs[i].iov_len)) { - free(vecs); return B_BAD_VALUE; } total += vecs[i].iov_len; diff --git a/src/system/kernel/fs/fd.cpp b/src/system/kernel/fs/fd.cpp index f05c6ef528..53788598d1 100644 --- a/src/system/kernel/fs/fd.cpp +++ b/src/system/kernel/fs/fd.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include @@ -759,12 +760,16 @@ common_user_vector_io(int fd, off_t pos, const iovec* userVecs, size_t count, { if (pos < -1) return B_BAD_VALUE; + if (count > IOV_MAX) + return B_BAD_VALUE; + + BStackOrHeapArray vecs(count); + if (!vecs.IsValid()) + return B_NO_MEMORY; - iovec* vecs; status_t error = get_iovecs_from_user(userVecs, count, vecs, true); if (error != B_OK) return error; - MemoryDeleter _(vecs); FileDescriptorPutter descriptor(get_fd(get_current_io_context(false), fd)); if (!descriptor.IsSet()) diff --git a/src/system/kernel/fs/socket.cpp b/src/system/kernel/fs/socket.cpp index 065e48be67..7f8b3484bc 100644 --- a/src/system/kernel/fs/socket.cpp +++ b/src/system/kernel/fs/socket.cpp @@ -164,11 +164,14 @@ prepare_userland_msghdr(const msghdr* userMessage, msghdr& message, if (message.msg_iovlen < 0 || message.msg_iovlen > IOV_MAX) return EMSGSIZE; if (userVecs != NULL && message.msg_iovlen > 0) { - iovec* vecs; + iovec* vecs = (iovec*)malloc(sizeof(iovec) * message.msg_iovlen); + if (vecs == NULL) + return B_NO_MEMORY; + vecsDeleter.SetTo(vecs); + status_t error = get_iovecs_from_user(message.msg_iov, message.msg_iovlen, vecs); if (error != B_OK) return error; - vecsDeleter.SetTo(vecs); message.msg_iov = vecs; } else { message.msg_iov = NULL; diff --git a/src/system/kernel/port.cpp b/src/system/kernel/port.cpp index cacc36a330..edbcb38634 100644 --- a/src/system/kernel/port.cpp +++ b/src/system/kernel/port.cpp @@ -22,6 +22,7 @@ #include #include +#include #include #include @@ -1920,10 +1921,13 @@ _user_writev_port_etc(port_id port, int32 messageCode, const iovec *userVecs, if (userVecs == NULL && bufferSize != 0) return B_BAD_VALUE; - if (userVecs != NULL && !IS_USER_ADDRESS(userVecs)) - return B_BAD_ADDRESS; + if (vecCount > IOV_MAX) + return B_BAD_VALUE; + + BStackOrHeapArray vecs(vecCount); + if (!vecs.IsValid()) + return B_NO_MEMORY; - iovec *vecs = NULL; if (userVecs != NULL && vecCount != 0) { status_t status = get_iovecs_from_user(userVecs, vecCount, vecs); if (status != B_OK) @@ -1934,7 +1938,6 @@ _user_writev_port_etc(port_id port, int32 messageCode, const iovec *userVecs, bufferSize, flags | PORT_FLAG_USE_USER_MEMCPY | B_CAN_INTERRUPT, timeout); - free(vecs); return syscall_restart_handle_timeout_post(status, timeout); }