diff options
Diffstat (limited to 'linux-sandbox-fix-fstatat-crash.patch')
-rw-r--r-- | linux-sandbox-fix-fstatat-crash.patch | 348 |
1 files changed, 0 insertions, 348 deletions
diff --git a/linux-sandbox-fix-fstatat-crash.patch b/linux-sandbox-fix-fstatat-crash.patch deleted file mode 100644 index 899e48e69b64..000000000000 --- a/linux-sandbox-fix-fstatat-crash.patch +++ /dev/null @@ -1,348 +0,0 @@ -From 60d5e803ef2a4874d29799b638754152285e0ed9 Mon Sep 17 00:00:00 2001 -From: Matthew Denton <mpdenton@chromium.org> -Date: Wed, 21 Jul 2021 12:55:11 +0000 -Subject: [PATCH] Linux sandbox: fix fstatat() crash - -This is a reland of https://crrev.com/c/2801873. - -Glibc has started rewriting fstat(fd, stat_buf) to -fstatat(fd, "", stat_buf, AT_EMPTY_PATH). This works because when -AT_EMPTY_PATH is specified, and the second argument is an empty string, -then fstatat just performs an fstat on fd like normal. - -Unfortunately, fstatat() also allows stat-ing arbitrary pathnames like -with fstatat(AT_FDCWD, "/i/am/a/file", stat_buf, 0); -The baseline policy needs to prevent this usage of fstatat() since it -doesn't allow access to arbitrary pathnames. - -Sadly, if the second argument is not an empty string, AT_EMPTY_PATH is -simply ignored by current kernels. - -This means fstatat() is completely unsandboxable with seccomp, since -we *need* to verify that the second argument is the empty string, but -we can't dereference pointers in seccomp (due to limitations of BPF, -and the difficulty of addressing these limitations due to TOCTOU -issues). - -So, this CL Traps (raises a SIGSYS via seccomp) on any fstatat syscall. -The signal handler, which runs in the sandboxed process, checks for -AT_EMPTY_PATH and the empty string, and then rewrites any applicable -fstatat() back into the old-style fstat(). - -Bug: 1164975 -Change-Id: I3df6c04c0d781eb1f181d707ccaaead779337291 -Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3042179 -Reviewed-by: Robert Sesek <rsesek@chromium.org> -Commit-Queue: Matthew Denton <mpdenton@chromium.org> -Cr-Commit-Position: refs/heads/master@{#903873} ---- - .../seccomp-bpf-helpers/baseline_policy.cc | 8 ++++++ - .../baseline_policy_unittest.cc | 17 ++++++++++++- - .../seccomp-bpf-helpers/sigsys_handlers.cc | 25 +++++++++++++++++++ - .../seccomp-bpf-helpers/sigsys_handlers.h | 14 +++++++++++ - .../linux/syscall_broker/broker_process.cc | 21 ++++++++++------ - .../syscall_broker/broker_process_unittest.cc | 18 ++++++------- - sandbox/linux/system_headers/linux_stat.h | 4 +++ - 7 files changed, 89 insertions(+), 18 deletions(-) - -diff --git a/sandbox/linux/seccomp-bpf-helpers/baseline_policy.cc b/sandbox/linux/seccomp-bpf-helpers/baseline_policy.cc -index f2a60bb4d7..9df0d2dbd3 100644 ---- a/sandbox/linux/seccomp-bpf-helpers/baseline_policy.cc -+++ b/sandbox/linux/seccomp-bpf-helpers/baseline_policy.cc -@@ -20,6 +20,7 @@ - #include "sandbox/linux/seccomp-bpf-helpers/syscall_sets.h" - #include "sandbox/linux/seccomp-bpf/sandbox_bpf.h" - #include "sandbox/linux/services/syscall_wrappers.h" -+#include "sandbox/linux/system_headers/linux_stat.h" - #include "sandbox/linux/system_headers/linux_syscalls.h" - - #if !defined(SO_PEEK_OFF) -@@ -304,6 +305,13 @@ ResultExpr EvaluateSyscallImpl(int fs_denied_errno, - return Allow(); - } - -+ // The fstatat syscalls are file system syscalls, which will be denied below -+ // with fs_denied_errno. However some allowed fstat syscalls are rewritten by -+ // libc implementations to fstatat syscalls, and we need to rewrite them back. -+ if (sysno == __NR_fstatat_default) { -+ return RewriteFstatatSIGSYS(fs_denied_errno); -+ } -+ - if (SyscallSets::IsFileSystem(sysno) || - SyscallSets::IsCurrentDirectory(sysno)) { - return Error(fs_denied_errno); -diff --git a/sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc b/sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc -index 68c29b564b..57d307e09d 100644 ---- a/sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc -+++ b/sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc -@@ -51,7 +51,8 @@ namespace sandbox { - - namespace { - --// This also tests that read(), write() and fstat() are allowed. -+// This also tests that read(), write(), fstat(), and fstatat(.., "", .., -+// AT_EMPTY_PATH) are allowed. - void TestPipeOrSocketPair(base::ScopedFD read_end, base::ScopedFD write_end) { - BPF_ASSERT_LE(0, read_end.get()); - BPF_ASSERT_LE(0, write_end.get()); -@@ -60,6 +61,20 @@ void TestPipeOrSocketPair(base::ScopedFD read_end, base::ScopedFD write_end) { - BPF_ASSERT_EQ(0, sys_ret); - BPF_ASSERT(S_ISFIFO(stat_buf.st_mode) || S_ISSOCK(stat_buf.st_mode)); - -+ sys_ret = fstatat(read_end.get(), "", &stat_buf, AT_EMPTY_PATH); -+ BPF_ASSERT_EQ(0, sys_ret); -+ BPF_ASSERT(S_ISFIFO(stat_buf.st_mode) || S_ISSOCK(stat_buf.st_mode)); -+ -+ // Make sure fstatat with anything other than an empty string is denied. -+ sys_ret = fstatat(read_end.get(), "/", &stat_buf, AT_EMPTY_PATH); -+ BPF_ASSERT_EQ(sys_ret, -1); -+ BPF_ASSERT_EQ(EPERM, errno); -+ -+ // Make sure fstatat without AT_EMPTY_PATH is denied. -+ sys_ret = fstatat(read_end.get(), "", &stat_buf, 0); -+ BPF_ASSERT_EQ(sys_ret, -1); -+ BPF_ASSERT_EQ(EPERM, errno); -+ - const ssize_t kTestTransferSize = 4; - static const char kTestString[kTestTransferSize] = {'T', 'E', 'S', 'T'}; - ssize_t transfered = 0; -diff --git a/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.cc b/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.cc -index 64edbd68bd..71068a0452 100644 ---- a/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.cc -+++ b/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.cc -@@ -6,6 +6,7 @@ - - #include "sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.h" - -+#include <fcntl.h> - #include <stddef.h> - #include <stdint.h> - #include <string.h> -@@ -22,6 +23,7 @@ - #include "sandbox/linux/seccomp-bpf/syscall.h" - #include "sandbox/linux/services/syscall_wrappers.h" - #include "sandbox/linux/system_headers/linux_seccomp.h" -+#include "sandbox/linux/system_headers/linux_stat.h" - #include "sandbox/linux/system_headers/linux_syscalls.h" - - #if defined(__mips__) -@@ -355,6 +357,24 @@ intptr_t SIGSYSSchedHandler(const struct arch_seccomp_data& args, - return -ENOSYS; - } - -+intptr_t SIGSYSFstatatHandler(const struct arch_seccomp_data& args, -+ void* fs_denied_errno) { -+ if (args.nr == __NR_fstatat_default) { -+ if (*reinterpret_cast<const char*>(args.args[1]) == '\0' && -+ args.args[3] == static_cast<uint64_t>(AT_EMPTY_PATH)) { -+ return syscall(__NR_fstat_default, static_cast<int>(args.args[0]), -+ reinterpret_cast<default_stat_struct*>(args.args[2])); -+ } -+ return -reinterpret_cast<intptr_t>(fs_denied_errno); -+ } -+ -+ CrashSIGSYS_Handler(args, fs_denied_errno); -+ -+ // Should never be reached. -+ RAW_CHECK(false); -+ return -ENOSYS; -+} -+ - bpf_dsl::ResultExpr CrashSIGSYS() { - return bpf_dsl::Trap(CrashSIGSYS_Handler, NULL); - } -@@ -387,6 +407,11 @@ bpf_dsl::ResultExpr RewriteSchedSIGSYS() { - return bpf_dsl::Trap(SIGSYSSchedHandler, NULL); - } - -+bpf_dsl::ResultExpr RewriteFstatatSIGSYS(int fs_denied_errno) { -+ return bpf_dsl::Trap(SIGSYSFstatatHandler, -+ reinterpret_cast<void*>(fs_denied_errno)); -+} -+ - void AllocateCrashKeys() { - #if !defined(OS_NACL_NONSFI) - if (seccomp_crash_key) -diff --git a/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.h b/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.h -index 7a958b93b2..8cd735ce15 100644 ---- a/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.h -+++ b/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.h -@@ -62,6 +62,19 @@ SANDBOX_EXPORT intptr_t SIGSYSPtraceFailure(const arch_seccomp_data& args, - // sched_setparam(), sched_setscheduler() - SANDBOX_EXPORT intptr_t SIGSYSSchedHandler(const arch_seccomp_data& args, - void* aux); -+// If the fstatat() syscall is functionally equivalent to an fstat() syscall, -+// then rewrite the syscall to the equivalent fstat() syscall which can be -+// adequately sandboxed. -+// If the fstatat() is not functionally equivalent to an fstat() syscall, we -+// fail with -fs_denied_errno. -+// If the syscall is not an fstatat() at all, crash in the same way as -+// CrashSIGSYS_Handler. -+// This is necessary because glibc and musl have started rewriting fstat(fd, -+// stat_buf) as fstatat(fd, "", stat_buf, AT_EMPTY_PATH). We rewrite the latter -+// back to the former, which is actually sandboxable. -+SANDBOX_EXPORT intptr_t -+SIGSYSFstatatHandler(const struct arch_seccomp_data& args, -+ void* fs_denied_errno); - - // Variants of the above functions for use with bpf_dsl. - SANDBOX_EXPORT bpf_dsl::ResultExpr CrashSIGSYS(); -@@ -72,6 +85,7 @@ SANDBOX_EXPORT bpf_dsl::ResultExpr CrashSIGSYSKill(); - SANDBOX_EXPORT bpf_dsl::ResultExpr CrashSIGSYSFutex(); - SANDBOX_EXPORT bpf_dsl::ResultExpr CrashSIGSYSPtrace(); - SANDBOX_EXPORT bpf_dsl::ResultExpr RewriteSchedSIGSYS(); -+SANDBOX_EXPORT bpf_dsl::ResultExpr RewriteFstatatSIGSYS(int fs_denied_errno); - - // Allocates a crash key so that Seccomp information can be recorded. - void AllocateCrashKeys(); -diff --git a/sandbox/linux/syscall_broker/broker_process.cc b/sandbox/linux/syscall_broker/broker_process.cc -index c2176eb785..e9dad37485 100644 ---- a/sandbox/linux/syscall_broker/broker_process.cc -+++ b/sandbox/linux/syscall_broker/broker_process.cc -@@ -113,44 +113,49 @@ bool BrokerProcess::IsSyscallAllowed(int sysno) const { - } - - bool BrokerProcess::IsSyscallBrokerable(int sysno, bool fast_check) const { -+ // The syscalls unavailable on aarch64 are all blocked by Android's default -+ // seccomp policy, even on non-aarch64 architectures. I.e., the syscalls XX() -+ // with a corresponding XXat() versions are typically unavailable in aarch64 -+ // and are default disabled in Android. So, we should refuse to broker them -+ // to be consistent with the platform's restrictions. - switch (sysno) { --#if !defined(__aarch64__) -+#if !defined(__aarch64__) && !defined(OS_ANDROID) - case __NR_access: - #endif - case __NR_faccessat: - return !fast_check || allowed_command_set_.test(COMMAND_ACCESS); - --#if !defined(__aarch64__) -+#if !defined(__aarch64__) && !defined(OS_ANDROID) - case __NR_mkdir: - #endif - case __NR_mkdirat: - return !fast_check || allowed_command_set_.test(COMMAND_MKDIR); - --#if !defined(__aarch64__) -+#if !defined(__aarch64__) && !defined(OS_ANDROID) - case __NR_open: - #endif - case __NR_openat: - return !fast_check || allowed_command_set_.test(COMMAND_OPEN); - --#if !defined(__aarch64__) -+#if !defined(__aarch64__) && !defined(OS_ANDROID) - case __NR_readlink: - #endif - case __NR_readlinkat: - return !fast_check || allowed_command_set_.test(COMMAND_READLINK); - --#if !defined(__aarch64__) -+#if !defined(__aarch64__) && !defined(OS_ANDROID) - case __NR_rename: - #endif - case __NR_renameat: - case __NR_renameat2: - return !fast_check || allowed_command_set_.test(COMMAND_RENAME); - --#if !defined(__aarch64__) -+#if !defined(__aarch64__) && !defined(OS_ANDROID) - case __NR_rmdir: - return !fast_check || allowed_command_set_.test(COMMAND_RMDIR); - #endif - --#if !defined(__aarch64__) -+#if !defined(__aarch64__) && !defined(OS_ANDROID) - case __NR_stat: - case __NR_lstat: - #endif -@@ -175,7 +180,7 @@ bool BrokerProcess::IsSyscallBrokerable(int sysno, bool fast_check) const { - return !fast_check || allowed_command_set_.test(COMMAND_STAT); - #endif - --#if !defined(__aarch64__) -+#if !defined(__aarch64__) && !defined(OS_ANDROID) - case __NR_unlink: - return !fast_check || allowed_command_set_.test(COMMAND_UNLINK); - #endif -diff --git a/sandbox/linux/syscall_broker/broker_process_unittest.cc b/sandbox/linux/syscall_broker/broker_process_unittest.cc -index c65f25a78a..f0db08d84e 100644 ---- a/sandbox/linux/syscall_broker/broker_process_unittest.cc -+++ b/sandbox/linux/syscall_broker/broker_process_unittest.cc -@@ -1596,52 +1596,52 @@ TEST(BrokerProcess, IsSyscallAllowed) { - const base::flat_map<BrokerCommand, base::flat_set<int>> kSysnosForCommand = { - {COMMAND_ACCESS, - {__NR_faccessat, --#if defined(__NR_access) -+#if defined(__NR_access) && !defined(OS_ANDROID) - __NR_access - #endif - }}, - {COMMAND_MKDIR, - {__NR_mkdirat, --#if defined(__NR_mkdir) -+#if defined(__NR_mkdir) && !defined(OS_ANDROID) - __NR_mkdir - #endif - }}, - {COMMAND_OPEN, - {__NR_openat, --#if defined(__NR_open) -+#if defined(__NR_open) && !defined(OS_ANDROID) - __NR_open - #endif - }}, - {COMMAND_READLINK, - {__NR_readlinkat, --#if defined(__NR_readlink) -+#if defined(__NR_readlink) && !defined(OS_ANDROID) - __NR_readlink - #endif - }}, - {COMMAND_RENAME, - {__NR_renameat, --#if defined(__NR_rename) -+#if defined(__NR_rename) && !defined(OS_ANDROID) - __NR_rename - #endif - }}, - {COMMAND_UNLINK, - {__NR_unlinkat, --#if defined(__NR_unlink) -+#if defined(__NR_unlink) && !defined(OS_ANDROID) - __NR_unlink - #endif - }}, - {COMMAND_RMDIR, - {__NR_unlinkat, --#if defined(__NR_rmdir) -+#if defined(__NR_rmdir) && !defined(OS_ANDROID) - __NR_rmdir - #endif - }}, - {COMMAND_STAT, - { --#if defined(__NR_stat) -+#if defined(__NR_stat) && !defined(OS_ANDROID) - __NR_stat, - #endif --#if defined(__NR_lstat) -+#if defined(__NR_lstat) && !defined(OS_ANDROID) - __NR_lstat, - #endif - #if defined(__NR_fstatat) -diff --git a/sandbox/linux/system_headers/linux_stat.h b/sandbox/linux/system_headers/linux_stat.h -index 35788eb22a..83b89efc75 100644 ---- a/sandbox/linux/system_headers/linux_stat.h -+++ b/sandbox/linux/system_headers/linux_stat.h -@@ -157,6 +157,10 @@ struct kernel_stat { - }; - #endif - -+#if !defined(AT_EMPTY_PATH) -+#define AT_EMPTY_PATH 0x1000 -+#endif -+ - // On 32-bit systems, we default to the 64-bit stat struct like libc - // implementations do. Otherwise we default to the normal stat struct which is - // already 64-bit. |