svn commit: r265367 - head/lib/libc/regex
brde at optusnet.com.au
Tue May 6 04:46:47 UTC 2014
On Tue, 6 May 2014, Andrey Chernov wrote:
> On 06.05.2014 2:12, David Chisnall wrote:
>> On 5 May 2014, at 22:51, Andrey Chernov <ache at freebsd.org> wrote:
>>> For standard malloc/realloc interface it is up to the caller to check
>>> n*size not overflows. You must trust caller already does such check.
>> Do a search of the CVE database sometime to see how well placed that trust generally is. Or even look at the code in question, where none of the realloc() or malloc() calls does overflow checking.
> I know current situation and disagree with OpenBSD way to fix it. Public
> interface assumes that caller should be trusted. Period. How well it is
> really trusted is up to the caller and should be fixed in it clearly,
> allowing human to trace the logic.
>>> Using calloc() to enforce it instead of caller is semantically wrong,
>> Relying on a standard function to behave according to the standard is semantically wrong?
The standard behaviour is undefined. It cannot be relied on. From C99
% 188.8.131.52 The calloc function
% #include <stdlib.h>
% void *calloc(size_t nmemb, size_t size);
% [#2] The calloc function allocates space for an array of
% nmemb objects, each of whose size is size. The space is
% initialized to all bits zero.238)
Oops, there is no object to begin with, so perhaps the behaviour is
defined after all. This is unclear. It is also unclear if objects
can have size too large to represent as a size_t. C99 says that
sizeof(object) is the size in bytes of an object, but it also says
that the value of sizeof() is implementation-defined. If the multiplication
overflows, then it can be argued that the behaviour is undefined (because
the object cannot exist since sizeof() is required to actually return the
size), and it can be argued that the behaviour is defined in some cases
even when the multiplication overflows (because sizeof() is only required
to return an implementation-defined value like the actual size modulo
SIZE_MAX; then the object might exist).
calloc() may have been actually useful orginally to handle the weird
second case. In K&R1, size_t didn't exist and whether sizeof() worked
was even less clear than now. The type of sizeof() was "an integer".
malloc() took an int arg IIRC. malloc() is not even in the index in
K&R1. But objects of size larger than INT_MAX were useful, and it
would be reasonable to ask for calloc() to allocate one. K&R1 has
calloc() in the index and documents it as calloc(n, sizeof(object)),
where n and sizeof() are apparently implicit-int. So calloc(16368, 2)
should give an object of size 32768 if possible. sizeof(this) is
then unrepresentable as a 16-bit int. I used arrays larger than
32768 quite often on 16-bit systems, but only with 16-bit unsigned
> Yes. Generally it is using a function outside of its purpose. I.e. you
> can use calloc() just to check n*size and nothing else (free() result
> immediately afterwards) instead of writing just single check by
> yourself. It will be legal usage but semantically wrong and misleading.
>>> and especially strange when the caller is standard C library under your
>> I don't follow this. If libc can't rely on standards conformance from itself then other code stands no chance.
calloc() in FreeBSD is controlled too, but in 4.4BSD it just did the
multiplication blindly. This was fixed (if it is a bug) in FreeBSD
in 2002 (by tjr). The errno was the nondescript ENOMEM. Now, calloc()
is sophisticated but the errno still seems to be ENOMEM.
I think calloc() should check for overflow but callers shouldn't depend
In practice, the multiplication is less likely to overflow than malloc()
is to fail, which "can't happen". You could limit the number of
elements to something reasonable like 2**28 to ensure that the
multiplication can't overflow with an element size of 8. The rare
program that needs to support allocating more than 2**28 elements on
32-bit systems according to user input can be more careful. On 64-bit
systems, you can use a less modest limit.
More information about the svn-src-head