From b6d0636656cbffaf5ab175996f682f13dbe4fcc6 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Tue, 16 Aug 2022 20:35:32 +0200 Subject: [PATCH] Kernel: Don't leak file descriptors in sys$pipe() If the final copy_to_user() call fails when writing the file descriptors to the output array, we have to make sure the file descriptors don't remain in the process file descriptor table. Otherwise they are basically leaked, as userspace is not aware of them. This matches the behavior of our sys$socketpair() implementation. --- Kernel/Process.h | 2 +- Kernel/Syscalls/pipe.cpp | 15 +++++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/Kernel/Process.h b/Kernel/Process.h index 0cc9418d75..f9e04a7fc1 100644 --- a/Kernel/Process.h +++ b/Kernel/Process.h @@ -355,7 +355,7 @@ public: ErrorOr sys$sigtimedwait(Userspace, Userspace, Userspace); ErrorOr sys$getgroups(size_t, Userspace); ErrorOr sys$setgroups(size_t, Userspace); - ErrorOr sys$pipe(int pipefd[2], int flags); + ErrorOr sys$pipe(Userspace, int flags); ErrorOr sys$killpg(pid_t pgrp, int sig); ErrorOr sys$seteuid(UserID); ErrorOr sys$setegid(GroupID); diff --git a/Kernel/Syscalls/pipe.cpp b/Kernel/Syscalls/pipe.cpp index 365fb8c4a1..6db456e731 100644 --- a/Kernel/Syscalls/pipe.cpp +++ b/Kernel/Syscalls/pipe.cpp @@ -9,7 +9,7 @@ namespace Kernel { -ErrorOr Process::sys$pipe(int pipefd[2], int flags) +ErrorOr Process::sys$pipe(Userspace pipefd, int flags) { VERIFY_NO_PROCESS_BIG_LOCK(this) TRY(require_promise(Pledge::stdio)); @@ -43,11 +43,18 @@ ErrorOr Process::sys$pipe(int pipefd[2], int flags) TRY(m_fds.with_exclusive([&](auto& fds) -> ErrorOr { fds[reader_fd_allocation.fd].set(move(reader_description), fd_flags); fds[writer_fd_allocation.fd].set(move(writer_description), fd_flags); + + int fds_for_userspace[2] = { + reader_fd_allocation.fd, + writer_fd_allocation.fd, + }; + if (copy_to_user(pipefd, fds_for_userspace, sizeof(fds_for_userspace)).is_error()) { + fds[reader_fd_allocation.fd] = {}; + fds[writer_fd_allocation.fd] = {}; + return EFAULT; + } return {}; })); - - TRY(copy_to_user(&pipefd[0], &reader_fd_allocation.fd)); - TRY(copy_to_user(&pipefd[1], &writer_fd_allocation.fd)); return 0; }