[scudo] Soft and hard RSS limit checks

[cryptoad]

Summary

This implements an opportunistic check for the RSS limit.

For ASan, this was implemented thanks to a background thread checking the
current RSS vs the set limit every 100ms. This was deemed problematic for Scudo
due to potential Android concerns (Zygote as pointed out by Aleksey) as well as
the general inconvenience of having a permanent background thread.

If a limit (soft or hard) is specified, we will attempt to update the RSS limit
status (exceeded or not) every 100ms. This is done in an opportunistic way: if
we can update it, we do it, if not we return the current status, mostly because
we don't need it to be fully consistent (it's done every 100ms anyway). If the
limit is exceeded allocate will act as if OOM for a soft limit, or just die
for a hard limit.

We use the common_flags()'s hard_rss_limit_mb & soft_rss_limit_mb for
configuration of the limits.

[cryptoad]

Make HardRssLimitMb & SoftRssLimitMb allocator globals. This will allow
later to set those at runtime rather than go through common_flags() every
time.

[alekseyshl]

lib/scudo/scudo_allocator.cpp

313

How about

if (LIKELY(CurrentCheck < LastCheck + (100ULL * 1000000ULL)))

318

How fast is it? It might read a file, right?

333

I find this structure to be easier to grasp, but it's up to you, just a suggestion (and maybe yours yield a better code):

if (atomic_load_relaxed(&RssLimitExceeded)) {
  if (CurrentRssMb <= SoftRssLimitMb)
    atomic_store_relaxed(&RssLimitExceeded, false);
} else {
  if (CurrentRssMb > SoftRssLimitMb) {
    atomic_store_relaxed(&RssLimitExceeded, true);
    Report("%s: soft RSS limit exhausted (%zdMb vs %zdMb)\n",
           SanitizerToolName, SoftRssLimitMb, CurrentRssMb);
  }
}

362

It needs UNLIKELY

test/scudo/rss.c

46

What's this for?

[cryptoad]

Addressing review comments.

[cryptoad]

lib/scudo/scudo_allocator.cpp

318

It either reads /proc/self/statm, or falls back to getrusage if statm is not allowed.
So yes, not the fastest of code paths.
I am not sure I see a way around it though.

test/scudo/rss.c

46

I added it because it seems that if there is nothing on stdout or stderr, FileCheck complains and fails, which is the case for the "nolimit" tests.

[cryptoad]

lib/scudo/scudo_allocator.cpp

318

Also the common flag is called can_use_proc_maps_statm, which is misdirecting, I think it really should be can_use_proc_self_statm.

[alekseyshl]

lib/scudo/scudo_allocator.cpp

318

Do not fall back to reading proc/self/statm, for example? Reading a file might cause unpredictable delays, right? Not too great for the allocator. Maybe increase the interval to 1s?

Also the general idea does not feel right, the allocator is one of the moving parts affecting RSS, not the only part, why should it be burdened with RSS check?

But yeah, I have questions, but not answers, no good solution either.

[cryptoad]

lib/scudo/scudo_allocator.cpp

318

I agree that it's not an ideal solution.

For some context, I am trying to address a problem that we are bringing to ourselves: due to the memory reservation made by the Primary, we can't enforce a sensible RLIMIT_AS for a process. Which some processes need (see limitProcessMemory in Android). The solution that comes to mind is to be able to constrain our RSS since we are the most likely consumer of memory. While not ideal, I don't think it's too unreasonable either.

Now for potential improvements I could code my own getMaxRSS that would be only getrusage based. That way, we skip the filesystem interaction. It would require a Fuchsia equivalent but that should be straightforward.

[alekseyshl]

Ok, add a TODO with the required improvements and let's get it in.

test/scudo/rss.c

46

There's --allow-empty flag, but this way is fine too.

[cryptoad]

Updating the test.

[cryptoad]

Adding a TODO with regard to GetRSS & /proc/self/statm.
Updating the tests to include can_use_proc_maps_statm again.

[cryptoad]

Adding a couple of additional restrictions to the output of the tests.

lib/scudo/scudo_allocator.cpp

struct ScudoAllocator {
   ScudoPrng GlobalPrng;
 
   u32 QuarantineChunksUpToSize;
 
   bool DeallocationTypeMismatch;
   bool ZeroContents;
   bool DeleteSizeMismatch;
 
+  bool CheckRssLimit;
+  uptr HardRssLimitMb;
+  uptr SoftRssLimitMb;
+  atomic_uint8_t RssLimitExceeded;
+  atomic_uint64_t RssLastCheckedAtNS;
+
   explicit ScudoAllocator(LinkerInitialized)
     : AllocatorQuarantine(LINKER_INITIALIZED) {}
 
   void init() {
     SanitizerToolName = "Scudo";
     initFlags();
 
     // Verify that the header offset field can hold the maximum offset. In the
void init() {
 
     // Check if hardware CRC32 is supported in the binary and by the platform,
     // if so, opt for the CRC32 hardware version of the checksum.
     if (computeHardwareCRC32 && testCPUFeature(CRC32CPUFeature))
       atomic_store_relaxed(&HashAlgorithm, CRC32Hardware);
 
     SetAllocatorMayReturnNull(common_flags()->allocator_may_return_null);
     BackendAllocator.init(common_flags()->allocator_release_to_os_interval_ms);
+    HardRssLimitMb = common_flags()->hard_rss_limit_mb;
+    SoftRssLimitMb = common_flags()->soft_rss_limit_mb;
     AllocatorQuarantine.Init(
         static_cast<uptr>(getFlags()->QuarantineSizeKb) << 10,
         static_cast<uptr>(getFlags()->ThreadLocalQuarantineSizeKb) << 10);
     QuarantineChunksUpToSize = getFlags()->QuarantineChunksUpToSize;
     DeallocationTypeMismatch = getFlags()->DeallocationTypeMismatch;
     DeleteSizeMismatch = getFlags()->DeleteSizeMismatch;
     ZeroContents = getFlags()->ZeroContents;
 
     GlobalPrng.init();
     Cookie = GlobalPrng.getU64();
+
+    CheckRssLimit = HardRssLimitMb || SoftRssLimitMb;
+    if (CheckRssLimit)
+      atomic_store_relaxed(&RssLastCheckedAtNS, NanoTime());
   }
 
   // Helper function that checks for a valid Scudo chunk. nullptr isn't.
   bool isValidPointer(const void *UserPtr) {
     initThreadMaybe();
     if (UNLIKELY(!UserPtr))
       return false;
     uptr UserBeg = reinterpret_cast<uptr>(UserPtr);
     if (!IsAligned(UserBeg, MinAlignment))
       return false;
     return getScudoChunk(UserBeg)->isValid();
   }
 
+  // Opportunistic RSS limit check. This will update the RSS limit status, if
+  // it can, every 100ms, otherwise it will just return the current one.
+  bool isRssLimitExceeded() {
+    u64 LastCheck = atomic_load_relaxed(&RssLastCheckedAtNS);
+    const u64 CurrentCheck = NanoTime();
+    if (LIKELY(CurrentCheck < LastCheck + (100ULL * 1000000ULL)))
+      return atomic_load_relaxed(&RssLimitExceeded);
+    if (!atomic_compare_exchange_weak(&RssLastCheckedAtNS, &LastCheck,
+                                      CurrentCheck, memory_order_relaxed))
+      return atomic_load_relaxed(&RssLimitExceeded);
+    // TODO(kostyak): We currently use sanitizer_common's GetRSS which reads the
+    //                RSS from /proc/self/statm by default. We might want to
+    //                call getrusage directly, even if it's less accurate.
+    const uptr CurrentRssMb = GetRSS() >> 20;
+    if (HardRssLimitMb && HardRssLimitMb < CurrentRssMb) {
+      Report("%s: hard RSS limit exhausted (%zdMb vs %zdMb)\n",
+             SanitizerToolName, HardRssLimitMb, CurrentRssMb);
+      DumpProcessMap();
+      Die();
+    }
+    if (SoftRssLimitMb) {
+      if (atomic_load_relaxed(&RssLimitExceeded)) {
+        if (CurrentRssMb <= SoftRssLimitMb)
+          atomic_store_relaxed(&RssLimitExceeded, false);
+      } else {
+        if (CurrentRssMb > SoftRssLimitMb) {
+          atomic_store_relaxed(&RssLimitExceeded, true);
+          Report("%s: soft RSS limit exhausted (%zdMb vs %zdMb)\n",
+                 SanitizerToolName, SoftRssLimitMb, CurrentRssMb);
+        }
+      }
+    }
+    return atomic_load_relaxed(&RssLimitExceeded);
+  }
+
   // Allocates a chunk.
   void *allocate(uptr Size, uptr Alignment, AllocType Type,
                  bool ForceZeroContents = false) {
     initThreadMaybe();
     if (UNLIKELY(Alignment > MaxAlignment))
       return FailureHandler::OnBadRequest();
     if (UNLIKELY(Alignment < MinAlignment))
       Alignment = MinAlignment;
     if (UNLIKELY(Size >= MaxAllowedMallocSize))
       return FailureHandler::OnBadRequest();
     if (UNLIKELY(Size == 0))
       Size = 1;
 
     uptr NeededSize = RoundUpTo(Size, MinAlignment) + AlignedChunkHeaderSize;
     uptr AlignedSize = (Alignment > MinAlignment) ?
         NeededSize + (Alignment - AlignedChunkHeaderSize) : NeededSize;
     if (UNLIKELY(AlignedSize >= MaxAllowedMallocSize))
       return FailureHandler::OnBadRequest();
 
+    if (CheckRssLimit && UNLIKELY(isRssLimitExceeded()))
+        return FailureHandler::OnOOM();
+
     // Primary and Secondary backed allocations have a different treatment. We
     // deal with alignment requirements of Primary serviced allocations here,
     // but the Secondary will take care of its own alignment needs.
     bool FromPrimary = PrimaryAllocator::CanAllocate(AlignedSize, MinAlignment);
 
     void *Ptr;
     u8 Salt;
     uptr AllocSize;

test/scudo/rss.c

+// RUN: %clang_scudo %s -o %t
+// RUN:                                                                                                  %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-nolimit
+// RUN: %env_scudo_opts="soft_rss_limit_mb=256"                                                          %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-nolimit
+// RUN: %env_scudo_opts="hard_rss_limit_mb=256"                                                          %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-nolimit
+// RUN: %env_scudo_opts="soft_rss_limit_mb=64:allocator_may_return_null=0"                           not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-softlimit
+// RUN: %env_scudo_opts="soft_rss_limit_mb=64:allocator_may_return_null=1"                               %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-softlimit-returnnull
+// RUN: %env_scudo_opts="soft_rss_limit_mb=64:allocator_may_return_null=0:can_use_proc_maps_statm=0" not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-softlimit
+// RUN: %env_scudo_opts="soft_rss_limit_mb=64:allocator_may_return_null=1:can_use_proc_maps_statm=0"     %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-softlimit-returnnull
+// RUN: %env_scudo_opts="hard_rss_limit_mb=64:allocator_may_return_null=0"                           not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-hardlimit
+// RUN: %env_scudo_opts="hard_rss_limit_mb=64:allocator_may_return_null=1"                           not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-hardlimit
+// RUN: %env_scudo_opts="hard_rss_limit_mb=64:allocator_may_return_null=0:can_use_proc_maps_statm=0" not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-hardlimit
+// RUN: %env_scudo_opts="hard_rss_limit_mb=64:allocator_may_return_null=1:can_use_proc_maps_statm=0" not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-hardlimit
+
+// Tests that the soft and hard RSS limits work as intended. Without limit or
+// with a high limit, the test should pass without any malloc returning NULL or
+// the program dying.
+// If a limit is specified, it should return some NULL or die depending on
+// allocator_may_return_null. This should also work without statm.
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+static const size_t kNumAllocs = 128;
+static const size_t kAllocSize = 1 << 20;  // 1MB.
+
+static void *allocs[kNumAllocs];
+
+int main(int argc, char *argv[]) {
+  int returned_null = 0;
+  for (int i = 0; i < kNumAllocs; i++) {
+    if ((i & 0xf) == 0)
+      usleep(50000);
+    allocs[i] = malloc(kAllocSize);
+    if (allocs[i])
+      memset(allocs[i], 0xff, kAllocSize);  // Dirty the pages.
+    else
+      returned_null++;
+  }
+  for (int i = 0; i < kNumAllocs; i++)
+    free(allocs[i]);
+  if (returned_null == 0)
+    printf("All malloc calls succeeded\n");
+  else
+    printf("%d malloc calls returned NULL\n", returned_null);
+  return 0;
+}
+
+// CHECK-nolimit: All malloc calls succeeded
+// CHECK-softlimit: soft RSS limit exhausted
+// CHECK-softlimit-NOT: malloc calls
+// CHECK-softlimit-returnnull: soft RSS limit exhausted
+// CHECK-softlimit-returnnull: malloc calls returned NULL
+// CHECK-hardlimit: hard RSS limit exhausted
+// CHECK-hardlimit-NOT: malloc calls