Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[scudo] Change tests that use setrlimit to cause mmap to fail. #87004

Merged
merged 5 commits into from
Mar 29, 2024

Conversation

cferris1000
Copy link
Contributor

It appears that qemu does not actually cause mmap to fail when calling setrlimit to limit the address space size. In the two tests that use setrlimit, detect if mmap still works and skip the tests in that case.

Since all Android targets should support setrlimit, compile out the mmap check code for them.

It appears that qemu does not actually cause mmap to fail when
calling setrlimit to limit the address space size. In the two
tests that use setrlimit, detect if mmap still works and skip
the tests in that case.

Since all Android targets should support setrlimit, compile out
the mmap check code for them.
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 28, 2024

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Christopher Ferris (cferris1000)

Changes

It appears that qemu does not actually cause mmap to fail when calling setrlimit to limit the address space size. In the two tests that use setrlimit, detect if mmap still works and skip the tests in that case.

Since all Android targets should support setrlimit, compile out the mmap check code for them.


Full diff: https://github.com/llvm/llvm-project/pull/87004.diff

2 Files Affected:

  • (modified) compiler-rt/lib/scudo/standalone/tests/strings_test.cpp (+13)
  • (modified) compiler-rt/lib/scudo/standalone/tests/vector_test.cpp (+11)
diff --git a/compiler-rt/lib/scudo/standalone/tests/strings_test.cpp b/compiler-rt/lib/scudo/standalone/tests/strings_test.cpp
index 17a596d712d0ca..c28ee7287aba14 100644
--- a/compiler-rt/lib/scudo/standalone/tests/strings_test.cpp
+++ b/compiler-rt/lib/scudo/standalone/tests/strings_test.cpp
@@ -129,6 +129,8 @@ TEST(ScudoStringsTest, Padding) {
 }
 
 #if defined(__linux__)
+
+#include <sys/mman.h>
 #include <sys/resource.h>
 
 TEST(ScudoStringsTest, CapacityIncreaseFails) {
@@ -136,9 +138,20 @@ TEST(ScudoStringsTest, CapacityIncreaseFails) {
 
   rlimit Limit = {};
   EXPECT_EQ(0, getrlimit(RLIMIT_AS, &Limit));
+
   rlimit EmptyLimit = {.rlim_cur = 0, .rlim_max = Limit.rlim_max};
   EXPECT_EQ(0, setrlimit(RLIMIT_AS, &EmptyLimit));
 
+#if !SCUDO_ANDROID
+  // qemu does not honor the setrlimit, so verify before proceeding.
+  void *ptr = mmap(nullptr, 100, PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
+  if (ptr != MAP_FAILED) {
+    munmap(ptr, 100);
+    setrlimit(RLIMIT_AS, &Limit);
+    GTEST_SKIP() << "Limiting address space does not prevent mmap.";
+  }
+#endif
+
   // Test requires that the default length is at least 6 characters.
   scudo::uptr MaxSize = Str.capacity();
   EXPECT_LE(6u, MaxSize);
diff --git a/compiler-rt/lib/scudo/standalone/tests/vector_test.cpp b/compiler-rt/lib/scudo/standalone/tests/vector_test.cpp
index add62c5a42a3e4..e65c5d6cc2759e 100644
--- a/compiler-rt/lib/scudo/standalone/tests/vector_test.cpp
+++ b/compiler-rt/lib/scudo/standalone/tests/vector_test.cpp
@@ -44,6 +44,7 @@ TEST(ScudoVectorTest, ResizeReduction) {
 
 #if defined(__linux__)
 
+#include <sys/mman.h>
 #include <sys/resource.h>
 
 // Verify that if the reallocate fails, nothing new is added.
@@ -58,6 +59,16 @@ TEST(ScudoVectorTest, ReallocateFails) {
   rlimit EmptyLimit = {.rlim_cur = 0, .rlim_max = Limit.rlim_max};
   EXPECT_EQ(0, setrlimit(RLIMIT_AS, &EmptyLimit));
 
+#if !SCUDO_ANDROID
+  // qemu does not honor the setrlimit, so verify before proceeding.
+  void *ptr = mmap(nullptr, 100, PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
+  if (ptr != MAP_FAILED) {
+    munmap(ptr, 100);
+    setrlimit(RLIMIT_AS, &Limit);
+    GTEST_SKIP() << "Limiting address space does not prevent mmap.";
+  }
+#endif
+
   V.resize(capacity);
   // Set the last element so we can check it later.
   V.back() = '\0';

Copy link

github-actions bot commented Mar 28, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@eugenis
Copy link
Contributor

eugenis commented Mar 28, 2024

I think, conceivably, someone could run SCUDO_ANDROID tests under usermode qemu. I would remove that check.

@cferris1000
Copy link
Contributor Author

I think, conceivably, someone could run SCUDO_ANDROID tests under usermode qemu. I would remove that check.

Done.

@eugenis
Copy link
Contributor

eugenis commented Mar 29, 2024

LGTM.
You could've just used getrlimit() to check - it would've shown the original (non-zero) limits on qemu. But this works, too.

@cferris1000 cferris1000 merged commit c0a3c5c into llvm:main Mar 29, 2024
4 checks passed
@cferris1000 cferris1000 deleted the string_test branch March 29, 2024 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants