[resend] Re: handling overflow in sbrk.

["Chris Demetriou" <cgd at google dot com>]

[Resent because a coworker pointed out that the actual patch and other
attachments enclosed in the last copy was base64 encoded, and that wasn't
the preferred patch submission format.  Sorry!]

On Tue, May 20, 2008 at 05:45, Carlos O'Donell <carlos@systemhalted.org> wrote:
>>  2008-05-16  Chris Demetriou  <cgd@google.com>
>>
>>        * misc/sbrk.c (__sbrk): If incrementing __curbrk by the requested
>>        amount would cause it to overflow, return an error (ENOMEM).
>
> 1. Please add a test case for this bug. Converting your program into a
> test would be a great idea.

See attached patch.  Also attached test run outputs from a system
running a 32-bit kernel and a 64-bit kernel, for good measure.  (I
wanted to make sure that the fact that the 32-bit kernel takes a chunk
out of the process's address space wouldn't harm the test.  This was a
change from my original test program.)

In order to make this test work (a) the test has to be linked
statically (see comments), and (b) the test can't limit memory
allocation.

I figured this wasn't reasonably testable on 64-bit systems (since I
know none that support remotely close to 64 VA bits -- i.e., the
kernel should ENOMEM before you get close enough to *try* to wrap
around), so punted there.  It might not be harmful to allow the test
to run normally on 64-bit, but while I'm comfortable w/ trying to
allocate 4GB of virtual memory, I'm less comfortable about filling a
64-bit process's address space.  8-)


> 2. Please run the glibc testsuite to verify that there were no regressions.

Done, no regressions.  Same failures before/after, except for the new
tst-sbrk1 which failed before and passed after.

This was on a CentOS 5 x86_64 kernel, uname -msrv -> Linux
2.6.18-53.1.13.el5xen #1 SMP Tue Feb 12 13:33:07 EST 2008 x86_64.


> 3. Do you have an FSF copyright assignment?

Yes, my understanding is that Google's copyright assignment covers
glibc.  (I asked around.)


thanks,

chris
---
2008-05-20  Chris Demetriou  <cgd@google.com>

        * misc/sbrk.c (__sbrk): If incrementing __curbrk by the requested
        amount would cause it to overflow, return an error (ENOMEM).
        * misc/tst-sbrk1.c: New test.
        * misc/Makefile (tests): Add new test.

Attachment: sbrk_patch_with_test.patch
Description: Text document

Attachment: 32-on-32.LOG
Description: Text document

Attachment: 32-on-64.LOG
Description: Text document