From 5fb18d3c9dae3250e4256b84c2071a3ae07c08fb Mon Sep 17 00:00:00 2001
From: Wiktor Garbacz <wiktorg@google.com>
Date: Fri, 6 Nov 2020 01:29:52 -0800
Subject: [PATCH] Add policy on both mmap & mmap2

PiperOrigin-RevId: 341007959
Change-Id: I3c2e74cc973d2603cf7b3a858fa8aabd05c41137
---
 sandboxed_api/sandbox2/policybuilder.cc | 35 +++++++++----------------
 sandboxed_api/sandbox2/policybuilder.h  | 19 ++++++--------
 2 files changed, 21 insertions(+), 33 deletions(-)

diff --git a/sandboxed_api/sandbox2/policybuilder.cc b/sandboxed_api/sandbox2/policybuilder.cc
index 94dd3a6..2ec94aa 100644
--- a/sandboxed_api/sandbox2/policybuilder.cc
+++ b/sandboxed_api/sandbox2/policybuilder.cc
@@ -46,6 +46,15 @@
 namespace sandbox2 {
 namespace {
 
+constexpr PolicyBuilder::SyscallInitializer kMmapSyscalls = {
+#ifdef __NR_mmap2
+    __NR_mmap2,
+#endif
+#ifdef __NR_mmap
+    __NR_mmap,
+#endif
+};
+
 }  // namespace
 
 PolicyBuilder& PolicyBuilder::AllowSyscall(unsigned int num) {
@@ -204,13 +213,7 @@ PolicyBuilder& PolicyBuilder::AllowLimitedMadvise() {
 }
 
 PolicyBuilder& PolicyBuilder::AllowMmap() {
-  // Consistently with policy.cc, when mmap2 exists then mmap is denied (not
-  // allowed).
-#ifdef __NR_mmap2
-  return AllowSyscall(__NR_mmap2);
-#else
-  return AllowSyscall(__NR_mmap);
-#endif
+  return AllowSyscalls(kMmapSyscalls);
 }
 
 PolicyBuilder& PolicyBuilder::AllowOpen() {
@@ -648,28 +651,16 @@ PolicyBuilder& PolicyBuilder::AddPolicyOnSyscalls(SyscallInitializer nums,
 }
 
 PolicyBuilder& PolicyBuilder::AddPolicyOnMmap(BpfInitializer policy) {
-#ifdef __NR_mmap2
-  return AddPolicyOnSyscall(__NR_mmap2, policy);
-#else
-  return AddPolicyOnSyscall(__NR_mmap, policy);
-#endif
+  return AddPolicyOnSyscalls(kMmapSyscalls, policy);
 }
 
 PolicyBuilder& PolicyBuilder::AddPolicyOnMmap(
     const std::vector<sock_filter>& policy) {
-#ifdef __NR_mmap2
-  return AddPolicyOnSyscall(__NR_mmap2, policy);
-#else
-  return AddPolicyOnSyscall(__NR_mmap, policy);
-#endif
+  return AddPolicyOnSyscalls(kMmapSyscalls, policy);
 }
 
 PolicyBuilder& PolicyBuilder::AddPolicyOnMmap(BpfFunc f) {
-#ifdef __NR_mmap2
-  return AddPolicyOnSyscall(__NR_mmap2, f);
-#else
-  return AddPolicyOnSyscall(__NR_mmap, f);
-#endif
+  return AddPolicyOnSyscalls(kMmapSyscalls, f);
 }
 
 PolicyBuilder& PolicyBuilder::DangerDefaultAllowAll() {
diff --git a/sandboxed_api/sandbox2/policybuilder.h b/sandboxed_api/sandbox2/policybuilder.h
index c5155e5..9ac75bf 100644
--- a/sandboxed_api/sandbox2/policybuilder.h
+++ b/sandboxed_api/sandbox2/policybuilder.h
@@ -141,13 +141,8 @@ class PolicyBuilder final {
   // all binaries.
   PolicyBuilder& AllowLlvmSanitizers();
 
-  // Appends code to allow mmap. Specifically this allows the mmap2 syscall on
-  // architectures where this syscalls exist and the mmap syscall on all other
-  // architectures.
-  //
-  // Note: while this function allows the calls, the default policy is run first
-  // and it has checks for dangerous flags which can create a violation. See
-  // sandbox2/policy.cc for more details.
+  // Appends code to allow mmap. Specifically this allows mmap and mmap2 syscall
+  // on architectures where this syscalls exist.
   PolicyBuilder& AllowMmap();
 
   // Appends code to allow calling futex with the given operation.
@@ -385,13 +380,15 @@ class PolicyBuilder final {
   // This policy may use labels.
   PolicyBuilder& AddPolicyOnSyscalls(SyscallInitializer nums, BpfFunc f);
 
-  // Equivalent to AddPolicyOnSyscall(mmap_syscall_no, policy), where
-  // mmap_syscall_no is either __NR_mmap or __NR_mmap2.
+  // Equivalent to AddPolicyOnSyscalls(mmap_syscalls, policy), where
+  // mmap_syscalls is a subset of {__NR_mmap, __NR_mmap2}, which exists on the
+  // target architecture.
   PolicyBuilder& AddPolicyOnMmap(BpfInitializer policy);
   PolicyBuilder& AddPolicyOnMmap(const std::vector<sock_filter>& policy);
 
-  // Equivalent to AddPolicyOnSyscall(mmap_syscall_no, f), where
-  // mmap_syscall_no is either __NR_mmap or __NR_mmap2.
+  // Equivalent to AddPolicyOnSyscalls(mmap_syscalls, f), where mmap_syscalls is
+  // a subset of {__NR_mmap, __NR_mmap2}, which exists on the target
+  // architecture.
   PolicyBuilder& AddPolicyOnMmap(BpfFunc f);
 
   // Builds the policy returning a unique_ptr to it. This should only be called