You must be using calloc() to allocate arrays. There is a really good reason why malloc() and calloc() are having different semantics, and why they are boiling down to different kernel functions (kmalloc() and kzalloc() on Linux).

Consider such example code:

Running it on a system with 64GB physical memory and no swap, will give non-NULL pointer for every allocation using malloc(), even though obviously you can't use all the requested memory. Version with calloc() will properly return non-NULL for first iteration, but after that it will return NULL.

So, if we call this change buffer overflow vulnerabilities we have to use calloc for both MEM_calloc_arrayN AND MEM_malloc_arrayN, otherwise we are still exposed to the buffer overflow.

From this point of view:

• Always use calloc() for array allocations.
• Remove own safety check function which does integer overflow check (which isn't even covered with unit tests! How would one trust it?).
• Check result to be non-NULL, otherwise abort() in the same way as it is done for integer overflow.

Followup based on discussion with Bastien in IRC.

The approach i've noted above will slow things down (extra memset etc), which is not necessarily what we want. But there are ways around this. I would state: this part of our allocator is really easy to switch on runtime. What this means is:

• We can have current Brecht's implementation as mem_{c,m}alloc_array_fast.
• But we also should have proper calloc() based allocations as stated above, something like mem_{c,m}alloc_array_safe.
• Actual MEM_{c,m}alloc_arrayN pointer can be assigned to either of those. Similar to what we do with fully guarded allocator, we can have MEM_use_safe_array_allocator.
• Decision on whether we want fully safe allocator or not we make based on trusted source, similar to our python scripts.
• Current code with integer overflow we can keep, it doesn't add measurable slowdown, and will still be handy to catch obvious issues. But cover it with tests!

I'm not sure why using calloc() would be safer for the kind of vulnerability we are trying to protect against? malloc() lets you allocate more memory than you can actually use, but still the allocations should not overlap and you should not be able to overwrite other memory that is in use, as long as you don't write outside the bounds for each memory allocation? The program may crash when out of memory, but not in a way that can be exploited?

If it can be exploited, what makes an array allocation different than another allocation? Wouldn't it be possible to encounter the same issue for a fixed size allocation, or is the assumption that such allocations typically have a smaller size and so are handled differently in some way?

And yes we need unit tests for this, but I wanted some feedback on the overall approach first.

I am not expert in what's exactly is happening under the hood, but scary thing is: you can READ all the memory allocated by malloc(). So who's memory is being addressed then? Is it some "fake" memory page filled in with zeroes? What is the exact time when pages are getting created?

Things i know for sure: calloc() is designed to deal with this cases. And any crash is a potentially open door for exploits.

My understanding of other types of allocs, is that they are fixed-size allocations, and hence are rather really tricky to get use of them for exploits. Maybe you can still put some random data in them, but making them to reliably execute your code could be really tricky since of the really tight space available for this.

My guts are saying: unless there's an expert saying "memory returned by optimistic allocator in Linux is not exploitable" i'd go the safest possible way.

That being said, looked into libpng and ffmpeg. They don't use real calloc(). But libpng is leaving pixels allocations to the software, and ffmpeg is limiting array size quite dramatically. Not sure what to make of this.

Ok, so after deeper investigation and discussing things with Brecht in IRC it appears to be fine to use malloc(). Writes can't write to someone's else memory. So only harm here would be reads of memory for which pages aren't created yet. But that's usual read-of-uninitialized memory which is to be fixed anyway.

Additionally, in Linux kernel calloc() boils down to regular malloc() + memset().

Brecht also looked into STL, where new() is boiling down to posix_memalign() which has single argument for memory size.

That makes me convinced it's ok to ignore SIGSEGV generated when writing to optimistically-allocated malloc()-ed memory.

That makes me feel fine about changes in guarded allocator, and in the sources elsewhere. But would still love to see unit test for integer overflow test.

I discussed this with @Sergey Sharybin (sergey) in IRC, and conclusion was that malloc() should be ok to use here. The only real difference with calloc() appears to be the zeroing, and we are not trying to protect against reading uninitialized memory here.

When allocating a big chunk of memory, it may not be immediately assigned to a physical page, but rather get paged in by the kernel after a page fault. If the system runs out of memory this may cause Blender to crash, but this doesn't appear to be exploitable as far as we can tell.

Generally LGTM.

Not sure it matters but it will be easy to swap sizeof/length arguments. GCC/Clang can ensure the second argument is a constant - but there will be a handful of cases where the size isn't known at compile time so no simple solution for that afaics.