From b066586355581f34ad9981bb6974066f8eb40446 Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Sun, 8 Mar 2020 13:14:35 +0100 Subject: [PATCH] Kernel: Fix race in waitid This is similar to 28e1da344d1de4fb80ce9e9c8da9127fa8606dc7 and 4dd4dd2f3c067eca446d9513e814ae9aaa648882. The crux is that wait verifies that the outvalue (siginfo* infop) is writable *before* waiting, and writes to it *after* waiting. In the meantime, a concurrent thread can make the output region unwritable, e.g. by deallocating it. --- Kernel/Process.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index e1feeb6f52..b392793473 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -2427,6 +2427,12 @@ pid_t Process::sys$waitid(const Syscall::SC_waitid_params* user_params) auto siginfo_or_error = do_waitid(static_cast(params.idtype), params.id, params.options); if (siginfo_or_error.is_error()) return siginfo_or_error.error(); + // While we waited, the process lock was dropped. This gave other threads + // the opportunity to mess with the memory. For example, it could free the + // region, and map it to a region to which it has no write permissions. + // Therefore, we need to re-validate the pointer. + if (!validate_write_typed(params.infop)) + return -EFAULT; copy_to_user(params.infop, &siginfo_or_error.value()); return 0;