Linux sandbox: allow 32GB address space size

content/common/sandbox_linux.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <fcntl.h>
 #include <sys/resource.h>
 #include <sys/stat.h>
 #include <sys/time.h>
 #include <sys/types.h>
 
+#include <limits>
+
 #include "base/command_line.h"
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/memory/singleton.h"
 #include "base/posix/eintr_wrapper.h"
 #include "base/time.h"
 #include "content/common/sandbox_linux.h"
 #include "content/common/seccomp_sandbox.h"
 #include "content/common/sandbox_seccomp_bpf_linux.h"
 #include "content/public/common/content_switches.h"
@@ skipping 38 matching lines @@
 // supported.
 bool ShouldEnableSeccompLegacy(const std::string& process_type) {
   if (IsSeccompLegacyDesired() &&
       process_type == switches::kRendererProcess) {
     return true;
   } else {
     return false;
   }
 }
 
+bool AddResourceLimit(int resource, rlim_t limit) {
+  struct rlimit old_rlimit;
+  if (getrlimit(resource, &old_rlimit))
+    return false;
+  // Make sure we don't raise the existing limit.
+  const struct rlimit new_rlimit = {
+      std::min(old_rlimit.rlim_cur, limit),
+      std::min(old_rlimit.rlim_max, limit)
+      };
+  int rc = setrlimit(resource, &new_rlimit);
+  return rc == 0;
+}
+
 }  // namespace
 
 namespace content {
 
 LinuxSandbox::LinuxSandbox()
     : proc_fd_(-1),
       seccomp_bpf_started_(false),
       pre_initialized_(false),
       seccomp_legacy_supported_(false),
       seccomp_bpf_supported_(false),
@@ skipping 171 matching lines @@
   return seccomp_legacy_supported_;
 }
 
 bool LinuxSandbox::seccomp_bpf_supported() const {
   CHECK(pre_initialized_);
   return seccomp_bpf_supported_;
 }
 
 bool LinuxSandbox::LimitAddressSpace(const std::string& process_type) {
   (void) process_type;
-#if defined(__x86_64__) && !defined(ADDRESS_SANITIZER)
+#if !defined(ADDRESS_SANITIZER)
   CommandLine* command_line = CommandLine::ForCurrentProcess();
   if (command_line->HasSwitch(switches::kNoSandbox)) {
     return false;
   }
-  // Limit the address space to 4GB.
-  const rlim_t kNewAddressSpaceMaxSize = 0x100000000L;
-  struct rlimit old_address_space_limit;
-  if (getrlimit(RLIMIT_AS, &old_address_space_limit))
-    return false;
-  // Make sure we don't raise the existing limit.
-  const struct rlimit new_address_space_limit = {
-      std::min(old_address_space_limit.rlim_cur, kNewAddressSpaceMaxSize),
-      std::min(old_address_space_limit.rlim_max, kNewAddressSpaceMaxSize)
-      };
-  int rc = setrlimit(RLIMIT_AS, &new_address_space_limit);
-  return (rc == 0);
+#if defined(__x86_64__)

[Chris Evans, 2013/01/15 09:07:56]

I wonder if this can be done more generically so that we get ARM64 correct
out-of-the-box?

BITS_PER_LONG maybe?

[jln (very slow on Chromium), 2013/01/15 19:44:19]

I hesitated a bit when I originally wrote this, but I then thought it was proper
to list full architectures instead.

But since I was on the fence, you convinced me, and I check for LP64 now.

+  // On 64 bits, limit the address space to 16GB. This is in the hope of making
+  // some kernel exploits more complex and less reliable.  This limit has to be
+  // very high because V8 and possibly others will reserve memory ranges and
+  // rely on on-demand paging for allocation.  Unfortunately, even
+  // MADV_DONTNEED ranges  count towards RLIMIT_AS so this is not an option.

[Chris Evans, 2013/01/15 09:07:56]

Also, we believe this is a good defence to a know WebKit issue with ref count
overflowing, for the case where an object needs to be allocated per ref. On
64-bit, the smallest tcmalloc allocation is 16 bytes, so 2^31 * 16 == 32GB
address space.
Not sure if that nuance is worth capturing in this comment :)

[jln (very slow on Chromium), 2013/01/15 19:44:19]

This should probably be captured in the bug. I added an explicit link to the bug
in the comment.

+  const rlim_t kNewAddressSpaceMaxSize = 1L << 34;
+#else
+  // On 32 bits, enforce the 4GB limit. On a 64 bits kernel, this could
+  // prevent far calling to 64 bits and abuse the memory allocator to exploit
+  // a kernel vulnerability.
+  const rlim_t kNewAddressSpaceMaxSize = std::numeric_limits<uint32_t>::max();
+#endif  // defined(__x86_64__)
+  // On all platforms, add a limit to the brk() heap that would prevent
+  // allocations that can't be index by an int.
+  const rlim_t kNewDataSegmentMaxSize = std::numeric_limits<int>::max();
+
+  bool limited_as = AddResourceLimit(RLIMIT_AS, kNewAddressSpaceMaxSize);
+  bool limited_data = AddResourceLimit(RLIMIT_DATA, kNewDataSegmentMaxSize);

[Chris Evans, 2013/01/15 09:07:56]

I wonder if this causes a subtle interaction? Really interested in your
thoughts.

With a 4GB brk() limit but a 16GB address space limit, something interesting
will happen with a e.g. 8GB heap spray. After the first 4GB of heap is filled,
brk() will start failing with ENOMEM due to the RLIMIT_DATA. At this moment,
tcmalloc will start falling back to the mmap() allocator.

How does mmap() decide where to put mappings on 64-bit Linux? For normal
kernels, does it just stack them after the last successful mapping? If so, does
this mean it might be placed inappropriately close to a v8 code mapping? Does
the situation (i.e. mmap address allocation strategy) change on a grsecurity
kernel? etc.?

[jln (very slow on Chromium), 2013/01/15 19:44:19]

In release mode we try the brk allocator first, then mmap (in debug we do the
reverse).

I think there are plenty of things we could do in this area (but I would reserve
them for another CL). I'm not too worried about v8, because I think v8 gives
hints to mmap mappings to add ASLR (we should check).

My inclination would be to disable the brk allocator and improve the mmap one.
You are correct that on stock Linux, mmap without a hint will just "pile down".
I would either add hints to the mmap allocator orn even bettern create guard
mappings after every mmap.

We should discuss this in another security improvement bug though.

+  return limited_as && limited_data;
 #else
   return false;
-#endif  // __x86_64__ && !defined(ADDRESS_SANITIZER)
+#endif  // !defined(ADDRESS_SANITIZER)
 }
 
 }  // namespace content