Fix overflow testing in sbrk.
Modify the overflow testing for sbrk.
Bug: 15188366
Change-Id: Ia83f85f7c1789454d872279bd41f38f1ce6b8a34
diff --git a/libc/bionic/brk.cpp b/libc/bionic/brk.cpp
index 4d08a61..4e46193 100644
--- a/libc/bionic/brk.cpp
+++ b/libc/bionic/brk.cpp
@@ -56,19 +56,19 @@
}
// Avoid overflow.
- intptr_t old_brk = reinterpret_cast<intptr_t>(__bionic_brk);
- if ((increment > 0 && INTPTR_MAX - increment > old_brk) ||
- (increment < 0 && (increment == PTRDIFF_MIN || old_brk < -increment))) {
+ uintptr_t old_brk = reinterpret_cast<uintptr_t>(__bionic_brk);
+ if ((increment > 0 && static_cast<uintptr_t>(increment) > (UINTPTR_MAX - old_brk)) ||
+ (increment < 0 && static_cast<uintptr_t>(-increment) > old_brk)) {
errno = ENOMEM;
return reinterpret_cast<void*>(-1);
}
void* desired_brk = reinterpret_cast<void*>(old_brk + increment);
__bionic_brk = __brk(desired_brk);
-
if (__bionic_brk < desired_brk) {
errno = ENOMEM;
return reinterpret_cast<void*>(-1);
}
+
return reinterpret_cast<void*>(old_brk);
}
diff --git a/tests/unistd_test.cpp b/tests/unistd_test.cpp
index 3f31861..6aa259a 100644
--- a/tests/unistd_test.cpp
+++ b/tests/unistd_test.cpp
@@ -56,21 +56,82 @@
ASSERT_EQ(ENOMEM, errno);
}
+#if defined(__GLIBC__)
+#define SBRK_MIN INTPTR_MIN
+#define SBRK_MAX INTPTR_MAX
+#else
+#define SBRK_MIN PTRDIFF_MIN
+#define SBRK_MAX PTRDIFF_MAX
+#endif
+
TEST(unistd, sbrk_ENOMEM) {
- intptr_t current_brk = reinterpret_cast<intptr_t>(get_brk());
+#if defined(__BIONIC__) && !defined(__LP64__)
+ // There is no way to guarantee that all overflow conditions can be tested
+ // without manipulating the underlying values of the current break.
+ extern void* __bionic_brk;
+
+ class ScopedBrk {
+ public:
+ ScopedBrk() : saved_brk_(__bionic_brk) {}
+ virtual ~ScopedBrk() { __bionic_brk = saved_brk_; }
+
+ private:
+ void* saved_brk_;
+ };
+
+ ScopedBrk scope_brk;
+
+ // Set the current break to a point that will cause an overflow.
+ __bionic_brk = reinterpret_cast<void*>(static_cast<uintptr_t>(PTRDIFF_MAX) + 2);
// Can't increase by so much that we'd overflow.
ASSERT_EQ(reinterpret_cast<void*>(-1), sbrk(PTRDIFF_MAX));
ASSERT_EQ(ENOMEM, errno);
- // Can't reduce by more than the current break.
- ASSERT_EQ(reinterpret_cast<void*>(-1), sbrk(-(current_brk + 1)));
- ASSERT_EQ(ENOMEM, errno);
+ // Set the current break to a point that will cause an overflow.
+ __bionic_brk = reinterpret_cast<void*>(static_cast<uintptr_t>(PTRDIFF_MAX));
-#if defined(__BIONIC__)
- // The maximum negative value is an interesting special case that glibc gets wrong.
ASSERT_EQ(reinterpret_cast<void*>(-1), sbrk(PTRDIFF_MIN));
ASSERT_EQ(ENOMEM, errno);
+
+ __bionic_brk = reinterpret_cast<void*>(static_cast<uintptr_t>(PTRDIFF_MAX) - 1);
+
+ ASSERT_EQ(reinterpret_cast<void*>(-1), sbrk(PTRDIFF_MIN + 1));
+ ASSERT_EQ(ENOMEM, errno);
+#else
+ class ScopedBrk {
+ public:
+ ScopedBrk() : saved_brk_(get_brk()) {}
+ virtual ~ScopedBrk() { brk(saved_brk_); }
+
+ private:
+ void* saved_brk_;
+ };
+
+ ScopedBrk scope_brk;
+
+ uintptr_t cur_brk = reinterpret_cast<uintptr_t>(get_brk());
+ if (cur_brk < static_cast<uintptr_t>(-(SBRK_MIN+1))) {
+ // Do the overflow test for a max negative increment.
+ ASSERT_EQ(reinterpret_cast<void*>(-1), sbrk(SBRK_MIN));
+#if defined(__BIONIC__)
+ // GLIBC does not set errno in overflow case.
+ ASSERT_EQ(ENOMEM, errno);
+#endif
+ }
+
+ uintptr_t overflow_brk = static_cast<uintptr_t>(SBRK_MAX) + 2;
+ if (cur_brk < overflow_brk) {
+ // Try and move the value to PTRDIFF_MAX + 2.
+ cur_brk = reinterpret_cast<uintptr_t>(sbrk(overflow_brk));
+ }
+ if (cur_brk >= overflow_brk) {
+ ASSERT_EQ(reinterpret_cast<void*>(-1), sbrk(SBRK_MAX));
+#if defined(__BIONIC__)
+ // GLIBC does not set errno in overflow case.
+ ASSERT_EQ(ENOMEM, errno);
+#endif
+ }
#endif
}
