Curious behaviour with calloc(-1, -1)

[ohnx56]

I noticed some curious behaviour with calloc(-1, -1) on hrev51615.

Since calloc uses unsigned ints, -1 = 4294967295.

4294967295 * 4294967295 overflows when multiplying to obtain 1.

Haiku does not check the overflow after multiplying here: ​https://github.com/haiku/haiku/blob/17b2a3cfcbc4fb1eb25d7eeb61e8fac997d7d835/src/system/libroot/posix/malloc/wrapper.cpp#L330

This results in Haiku assuming that the user wants 1 byte of memory allocated, which is technically incorrect...

A way to check for overflow would be to divide the result by one of the inputs (n or elem size) and ensure it equals the other input. From my understanding, glibc uses this: ​https://github.com/lattera/glibc/blob/master/malloc/malloc.c#L3170

This issue is notable because there are other ways for a 32-bit unsigned integer multiplication to overflow, possibly resulting in incorrect values being returned from Haiku.

Bottom line is this: When I call calloc() with impossibly large amounts of memory requirements, I expect a return value of NULL, and not a pointer :)

[ohnx56]

Thanks to sphinxbase for having a weird test like this that demonstrates this issue...

​https://github.com/cmusphinx/sphinxbase/blob/master/test/unit/test_alloc/test_ckd_alloc_catch.c

[waddlesplash]

Patch seems good to me. CC'ing axeld so he can take a look.

[ohnx56]

Update patch to meet coding guidelines

[ohnx56]

Hmm, looks like sometimes calloc() gets called with 0-values... This patch now also checks to ensure it does not end up diving by zero.

I have tested this patch applied onto hrev51641 by booting to the patched Haiku and writing a program using calloc(-1, -1) as well as calloc(4294967295, 123). Now, calloc() behaves as expected by returning a NULL to silly requests for memory like these ones. No application appears to be broken by this change.

[pulkomandy]

Adding a test in src/tests would be nice, too (testing one or the other parameter being 0, the -1 -1 case, and the "incredibly large" case, as well as a normal allocation).

[ohnx56]

Where should the test go? I can write one and submit a patch for the test as well.

[pulkomandy]

src/tests/system/libroot/posix sounds appropriate.

[waddlesplash]

Applied in hrev51689. Thanks!