summarylogtreecommitdiffstats
path: root/linux-sandbox-fix-fstatat-crash.patch
diff options
context:
space:
mode:
Diffstat (limited to 'linux-sandbox-fix-fstatat-crash.patch')
-rw-r--r--linux-sandbox-fix-fstatat-crash.patch348
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.