Issue 131924 in chromium: TCMalloc realloc() fails with 2GB -> 3GB on 64 bit platforms.

[chro...@googlecode.com]

Status: Unconfirmed
Owner: ----
Labels: OS-Linux Area-Undefined Type-Bug Pri-2

New issue 131924 by a...@ut.ee: TCMalloc realloc() fails with 2GB -> 3GB on
64 bit platforms.
http://code.google.com/p/chromium/issues/detail?id=131924

Chrome Version : 21.0.1163.0 (140236)
OS Version : Ubuntu 12.04 x86_64
URLs (if applicable) :
Other browsers tested:
Add OK or FAIL after other browsers where you have tested this issue:
Safari 5:
Firefox 4.x:
IE 7/8/9:

What steps will reproduce the problem?
buf = malloc(0x80000000);
realloc(buf, 0xc0000000);

What is the expected result?
buf is reallocated to 3GB.

What happens instead?
Out of memory. I think I see a bug in TCMalloc tcmalloc.cc function
do_realloc_with_callback. The code looks like:

const int lower_bound_to_grow = old_size + old_size / 4;
const int upper_bound_to_shrink = old_size / 2;
if ((new_size > old_size) || (new_size < upper_bound_to_shrink)) {
// Need to reallocate.
void* new_ptr = NULL;

if (new_size > old_size && new_size < lower_bound_to_grow) {
new_ptr = do_malloc_or_cpp_alloc(lower_bound_to_grow);

The problem is that lower_bound_to_grow and upper_bound_to_shrink have type
int.

What happens is:
old_size = 0x80000000
lower_bound_to_grow = 0x80000000 + 0x20000000 = 0xa0000000 (negative)
if (0xc0000000 > 0x80000000 && 0xc0000000 < 0xffffffffa0000000)
new_ptr = do_malloc_or_cpp_alloc(0xffffffffa0000000);

So lower_bound_to_grow hits sign extension and a huge malloc is attempted.

[chro...@googlecode.com]

Updates:
Cc: j...@chromium.org will...@chromium.org phajdan...@chromium.org
Labels: -Area-Undefined Area-Internals Internals-Core

Comment #1 on issue 131924 by phajdan...@chromium.org: TCMalloc realloc()

fails with 2GB -> 3GB on 64 bit platforms.
http://code.google.com/p/chromium/issues/detail?id=131924

(No comment was entered for this change.)

[chro...@googlecode.com]

Updates:
Status: ExternalDependency

Comment #2 on issue 131924 by will...@chromium.org: TCMalloc realloc()

fails with 2GB -> 3GB on 64 bit platforms.
http://code.google.com/p/chromium/issues/detail?id=131924

Looks like a real bug, with low severity, since it's problematic to
allocate so much anyway. I do not think there is a security impact, since
anyone who can control malloc/realloc() can trigger OOM anyway. This is a
bug in upstream google-perftools, so we should mark this as an external
dependency and fix upstream.

[chro...@googlecode.com]

Comment #3 on issue 131924 by j...@chromium.org: TCMalloc realloc() fails

with 2GB -> 3GB on 64 bit platforms.
http://code.google.com/p/chromium/issues/detail?id=131924

The bug in the code actually has two consequences. The first, reported
here, is that we fail on a viable realloc. The second is that the code
will sometimes fail to achieve its goal of freeing memory when the target
(new_size) allocation is small. Specifically, a realloc to far less than
half the current size, will sometimes simply continue to use the current
allocated region. The conversion of upper_bound_to_shrink into int32 can
produce a garbage small number, so that the test:

new_size < upper_bound_to_shrink

can fail to detect the need to get a new (small) reallocation (and free the
larger original block).

As noted by Will, this should be upstreamed.

Try as I might, I couldn't find a way to exploit any security ramifications
of the potentially bogus values of either lower_bound_to_grow and/or
upper_bound_to_shrink. When new_size *happens* to be less than
lower_bound_to_grow (the latter being cast back to int64), it is certainly
safe to malloc up (int64)lower_bound_to_grow. In all other cases, we
allocate new_size, which is also plenty safe.

[chro...@googlecode.com]

Updates:
Cc: cev...@chromium.org

Comment #4 on issue 131924 by j...@chromium.org: TCMalloc realloc() fails

with 2GB -> 3GB on 64 bit platforms.
http://code.google.com/p/chromium/issues/detail?id=131924

[chro...@googlecode.com]

Updates:
Labels: SecSeverity-None

Comment #5 on issue 131924 by scarybea...@gmail.com: TCMalloc realloc()

fails with 2GB -> 3GB on 64 bit platforms.
http://code.google.com/p/chromium/issues/detail?id=131924

Oh yeah, I'm pretty sure we've seen this before. Not sure of an existing
bug id? It's kind of amusing as a defense because it might stop a bug that
relies on a 64-bit porting error (e.g. inappropriate use of 32-bit type).

[chro...@googlecode.com]

Comment #6 on issue 131924 by j...@chromium.org: TCMalloc realloc() fails

with 2GB -> 3GB on 64 bit platforms.
http://code.google.com/p/chromium/issues/detail?id=131924

FWIW: This doesn't generally preclude large allocations (or reallocations),
it just blocks some cases hither and yonder. For instance, realloc from
2GB to 4GB would not be a problem (the code would do the allocation... even
though it went down some paths my accident). Strange cases that would have
trouble would be when going from 34GB to 35GB :-/, but it would be fine to
go from 34GB to 36GB.