Bug 14483 - obstack uses int for allocation sizes
Summary: obstack uses int for allocation sizes
Status: NEW
Alias: None
Product: glibc
Classification: Unclassified
Component: malloc (show other bugs)
Version: 2.16
: P2 enhancement
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL: https://sourceware.org/ml/libc-alpha/...
Keywords:
Depends on:
Blocks:
 
Reported: 2012-08-16 22:22 UTC by Joseph Myers
Modified: 2016-12-29 13:50 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
fweimer: security+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Myers 2012-08-16 22:22:49 UTC
The obstack code uses int throughout as a type to store and return sizes of memory allocations.  It should use size_t to avoid arbitrary limits on the size of objects (or incorrect returns in the case of obstack_memory_used).

I don't know offhand if fixing this will require new symbol versions for obstack functions and a change to _GNU_OBSTACK_INTERFACE_VERSION.
Comment 1 Andreas Schwab 2013-01-09 16:52:00 UTC
The obstack interface was last incompatibly changed in 2006 when Paul Eggert's alignment changes were merged.  _GNU_OBSTACK_INTERFACE_VERSION should be abandoned completely.
Comment 2 Ondrej Bilka 2013-10-20 21:08:29 UTC
Returning a stack sie as size_t could be feasible. However extending arguments from ints is nonsense, obstack is used for quick allocation of lot of small objects. You can have only few large objects until you run out of memory, Also returning huge chunks of memory to system looks better than keeping them in obstack.
Comment 3 Alan Modra 2014-07-24 07:00:56 UTC
Using int for sizes in the current obstack code is also a security breach, a buffer overflow on steroids.  A default buffer of around 4k is seen as sufficiently large to write a 2G+ object..  This is true for both 32-bit and 64-bit targets.

See https://sourceware.org/bugzilla/show_bug.cgi?id=17133
Comment 4 Florian Weimer 2014-08-04 11:19:39 UTC
(In reply to Alan Modra from comment #3)
> Using int for sizes in the current obstack code is also a security breach, a
> buffer overflow on steroids.  A default buffer of around 4k is seen as
> sufficiently large to write a 2G+ object..  This is true for both 32-bit and
> 64-bit targets.
> 
> See https://sourceware.org/bugzilla/show_bug.cgi?id=17133

I think this is a bug in the caller, not in obstack.  But it's an easy mistake to make, and I consider the use of int for memory region sizes to be bad practice which should be avoided.  Unfortunately, it's difficult to change legacy APIs.
Comment 5 Florian Weimer 2016-09-16 08:37:00 UTC
There is a series of fixes in gnulib:

commit 266ac1b98dbf76a78cdc584b10ed73355a084e7a
Author: Alan Modra <amodra@gmail.com>
Date:   Wed Oct 29 14:02:31 2014 +1030

    obstack: 64-bit obstack support, part 1

commit bb2ab7ecb6c92895e69837414f238568f013a603
Author: Alan Modra <amodra@gmail.com>
Date:   Wed Oct 29 14:02:40 2014 +1030

    obstack: 64-bit obstack support, part 2

commit 82a38a0d7776d99e7afc03e0d8df93fa253f1e35
Author: Alan Modra <amodra@gmail.com>
Date:   Wed Oct 29 14:03:00 2014 +1030

    obstack: 64-bit obstack support, part 3

commit 6c34297a9143b66bd69a1748a59662c45257f5f6
Author: Paul Eggert <eggert@cs.ucla.edu>
Date:   Tue Oct 28 23:58:42 2014 -0700

    obstack: use size_t alignments and check for overflow

commit d15b2da0ac25e085ce30a9e2672624999ce910a6
Author: Alan Modra <amodra@gmail.com>
Date:   Mon Nov 3 17:32:27 2014 -0800

    obstack: fix macro return values


If I'm not mistaken, the 64-bit gnulib functions preempt the 32-bit glibc implementations, which causes problems.

Upon reconsideration, we should probably treat this as a security issue.
Comment 6 Alan Modra 2016-09-16 15:28:48 UTC
Yes, 64-bit gnulib (and libiberty) obstack functions are used rather than the glibc versions.  How does this cause problems?  obstack_vprintf?

Note: I haven't pushed the patch in the URL due to knowing that changing the glibc obstack interface will cause problems with things like libsanitizer.
Comment 7 Florian Weimer 2016-09-16 15:53:54 UTC
(In reply to Alan Modra from comment #6)
> Yes, 64-bit gnulib (and libiberty) obstack functions are used rather than
> the glibc versions.  How does this cause problems?  obstack_vprintf?

A library is compiled against glibc's <obstack.h>, but something else links in the implementation from gnulib.  As far as I can tell, the two aren't ABI-compatible, so you end up with crashes.

obstack_vprintf is another problem if gnulib doesn't provide that.
Comment 8 Alan Modra 2016-12-29 13:50:46 UTC
Maybe the bug will get more attention if I'm not the assignee.