Skip to content

Commit 410a6d5

Browse files
cferris1000andi34
authored andcommitted
Fix overflow testing in sbrk.
Modify the overflow testing for sbrk. Bug: 15188366 Bug: 28740702 (cherry picked from commit 738b0cc) Change-Id: Id79a2314d86333d689b291bc32e513664df7e058
1 parent b457cb7 commit 410a6d5

File tree

2 files changed

+71
-12
lines changed

2 files changed

+71
-12
lines changed

libc/bionic/brk.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,19 +59,19 @@ void* sbrk(ptrdiff_t increment) {
5959
}
6060

6161
// Avoid overflow.
62-
intptr_t old_brk = reinterpret_cast<intptr_t>(__bionic_brk);
63-
if ((increment > 0 && INTPTR_MAX - increment > old_brk) ||
64-
(increment < 0 && (increment == PTRDIFF_MIN || old_brk < -increment))) {
62+
uintptr_t old_brk = reinterpret_cast<uintptr_t>(__bionic_brk);
63+
if ((increment > 0 && static_cast<uintptr_t>(increment) > (UINTPTR_MAX - old_brk)) ||
64+
(increment < 0 && static_cast<uintptr_t>(-increment) > old_brk)) {
6565
errno = ENOMEM;
6666
return reinterpret_cast<void*>(-1);
6767
}
6868

6969
void* desired_brk = reinterpret_cast<void*>(old_brk + increment);
7070
__bionic_brk = __brk(desired_brk);
71-
7271
if (__bionic_brk < desired_brk) {
7372
errno = ENOMEM;
7473
return reinterpret_cast<void*>(-1);
7574
}
75+
7676
return reinterpret_cast<void*>(old_brk);
7777
}

tests/unistd_test.cpp

Lines changed: 67 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -53,22 +53,81 @@ TEST(unistd, brk_ENOMEM) {
5353
ASSERT_EQ(ENOMEM, errno);
5454
}
5555

56+
#if defined(__GLIBC__)
57+
#define SBRK_MIN (-2147483647-1)
58+
#define SBRK_MAX (2147483647)
59+
#else
60+
#define SBRK_MIN PTRDIFF_MIN
61+
#define SBRK_MAX PTRDIFF_MAX
62+
#endif
63+
5664
TEST(unistd, sbrk_ENOMEM) {
57-
intptr_t current_brk = reinterpret_cast<intptr_t>(get_brk());
65+
#if defined(__BIONIC__) && !defined(__LP64__)
66+
// There is no way to guarantee that all overflow conditions can be tested
67+
// without manipulating the underlying values of the current break.
68+
extern void* __bionic_brk;
69+
70+
class ScopedBrk {
71+
public:
72+
ScopedBrk() : saved_brk_(__bionic_brk) {}
73+
virtual ~ScopedBrk() { __bionic_brk = saved_brk_; }
74+
75+
private:
76+
void* saved_brk_;
77+
};
78+
79+
ScopedBrk scope_brk;
80+
81+
// Set the current break to a point that will cause an overflow.
82+
__bionic_brk = reinterpret_cast<void*>(static_cast<uintptr_t>(PTRDIFF_MAX) + 2);
5883

59-
#if !defined(__GLIBC__)
6084
// Can't increase by so much that we'd overflow.
6185
ASSERT_EQ(reinterpret_cast<void*>(-1), sbrk(PTRDIFF_MAX));
6286
ASSERT_EQ(ENOMEM, errno);
63-
#endif
6487

65-
// Can't reduce by more than the current break.
66-
ASSERT_EQ(reinterpret_cast<void*>(-1), sbrk(-(current_brk + 1)));
67-
ASSERT_EQ(ENOMEM, errno);
88+
// Set the current break to a point that will cause an overflow.
89+
__bionic_brk = reinterpret_cast<void*>(static_cast<uintptr_t>(PTRDIFF_MAX));
6890

69-
#if !defined(__GLIBC__)
70-
// The maximum negative value is an interesting special case that glibc gets wrong.
7191
ASSERT_EQ(reinterpret_cast<void*>(-1), sbrk(PTRDIFF_MIN));
7292
ASSERT_EQ(ENOMEM, errno);
93+
94+
__bionic_brk = reinterpret_cast<void*>(static_cast<uintptr_t>(PTRDIFF_MAX) - 1);
95+
96+
ASSERT_EQ(reinterpret_cast<void*>(-1), sbrk(PTRDIFF_MIN + 1));
97+
ASSERT_EQ(ENOMEM, errno);
98+
#else
99+
class ScopedBrk {
100+
public:
101+
ScopedBrk() : saved_brk_(get_brk()) {}
102+
virtual ~ScopedBrk() { brk(saved_brk_); }
103+
104+
private:
105+
void* saved_brk_;
106+
};
107+
108+
ScopedBrk scope_brk;
109+
110+
uintptr_t cur_brk = reinterpret_cast<uintptr_t>(get_brk());
111+
if (cur_brk < static_cast<uintptr_t>(-(SBRK_MIN+1))) {
112+
// Do the overflow test for a max negative increment.
113+
ASSERT_EQ(reinterpret_cast<void*>(-1), sbrk(SBRK_MIN));
114+
#if defined(__BIONIC__)
115+
// GLIBC does not set errno in overflow case.
116+
ASSERT_EQ(ENOMEM, errno);
117+
#endif
118+
}
119+
120+
uintptr_t overflow_brk = static_cast<uintptr_t>(SBRK_MAX) + 2;
121+
if (cur_brk < overflow_brk) {
122+
// Try and move the value to PTRDIFF_MAX + 2.
123+
cur_brk = reinterpret_cast<uintptr_t>(sbrk(overflow_brk));
124+
}
125+
if (cur_brk >= overflow_brk) {
126+
ASSERT_EQ(reinterpret_cast<void*>(-1), sbrk(SBRK_MAX));
127+
#if defined(__BIONIC__)
128+
// GLIBC does not set errno in overflow case.
129+
ASSERT_EQ(ENOMEM, errno);
130+
#endif
131+
}
73132
#endif
74133
}

0 commit comments

Comments
 (0)