| Message ID | 5056F210.7060404@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Sep 17, 2012 at 2:49 AM, Florian Weimer <fweimer@redhat.com> wrote: > On 09/05/2012 07:31 AM, Ian Lance Taylor wrote: >> >> On Wed, Aug 29, 2012 at 10:32 AM, Florian Weimer <fw@deneb.enyo.de> wrote: >>> >>> >>> This patches fixes an integer overflow in libiberty, which leads to >>> crashes in binutils. The long version of the objalloc_alloc macro >>> would have needed another conditional, so I removed that and replaced >>> it with a call to the actual implementation. >> >> >> I guess I don't see why removing the macro is desirable. In many uses >> of objalloc_alloc the conditional can be optimized out anyhow. It's >> true that it can't always be, but, so what? The macro is probably >> still a win. > > > Fair enough. I've added a wraparound check to the macro. Okay for trunk? > { > + unsigned long len = original_len; > /* We avoid confusion from zero sized objects by always allocating > at least 1 byte. */ Please add a blank line after the variable declaration. > - (__len <= __o->current_space \ > + (__len && __len <= __o->current_space \ Please write __len != 0 or len > 0. This is OK with those changes. Thanks. Ian
On 09/17/2012 05:59 PM, Ian Lance Taylor wrote: >> Fair enough. I've added a wraparound check to the macro. Okay for trunk? > >> { >> + unsigned long len = original_len; >> /* We avoid confusion from zero sized objects by always allocating >> at least 1 byte. */ > > Please add a blank line after the variable declaration. > >> - (__len <= __o->current_space \ >> + (__len && __len <= __o->current_space \ > > Please write __len != 0 or len > 0. > > This is OK with those changes. Thanks, committed with these changes.
2012-09-17 Florian Weimer <fweimer@redhat.com>
PR other/54411
* objalloc.h (objalloc_alloc): Do not use fast path on wraparound.
2012-09-17 Florian Weimer <fweimer@redhat.com>
PR other/54411
* objalloc.c (_objalloc_alloc): Add overflow check covering
alignment and CHUNK_HEADER_SIZE addition.
Index: libiberty/objalloc.c
===================================================================
--- libiberty/objalloc.c (revision 191378)
+++ libiberty/objalloc.c (working copy)
@@ -1,5 +1,5 @@
/* objalloc.c -- routines to allocate memory for objects
- Copyright 1997 Free Software Foundation, Inc.
+ Copyright 1997-2012 Free Software Foundation, Inc.
Written by Ian Lance Taylor, Cygnus Solutions.
This program is free software; you can redistribute it and/or modify it
@@ -112,8 +112,9 @@
/* Allocate space from an objalloc structure. */
PTR
-_objalloc_alloc (struct objalloc *o, unsigned long len)
+_objalloc_alloc (struct objalloc *o, unsigned long original_len)
{
+ unsigned long len = original_len;
/* We avoid confusion from zero sized objects by always allocating
at least 1 byte. */
if (len == 0)
@@ -121,6 +122,11 @@
len = (len + OBJALLOC_ALIGN - 1) &~ (OBJALLOC_ALIGN - 1);
+ /* Check for overflow in the alignment operation above and the
+ malloc argument below. */
+ if (len + CHUNK_HEADER_SIZE < original_len)
+ return NULL;
+
if (len <= o->current_space)
{
o->current_ptr += len;
Index: include/objalloc.h
===================================================================
--- include/objalloc.h (revision 191378)
+++ include/objalloc.h (working copy)
@@ -1,5 +1,5 @@
/* objalloc.h -- routines to allocate memory for objects
- Copyright 1997, 2001 Free Software Foundation, Inc.
+ Copyright 1997-2012 Free Software Foundation, Inc.
Written by Ian Lance Taylor, Cygnus Solutions.
This program is free software; you can redistribute it and/or modify it
@@ -91,7 +91,7 @@
if (__len == 0) \
__len = 1; \
__len = (__len + OBJALLOC_ALIGN - 1) &~ (OBJALLOC_ALIGN - 1); \
- (__len <= __o->current_space \
+ (__len && __len <= __o->current_space \
? (__o->current_ptr += __len, \
__o->current_space -= __len, \
(void *) (__o->current_ptr - __len)) \