Fix out-of-memory handling
This should fix #215 and close #214 as redundant. It also fixes some potential problems not mentioned in #215.
The main problem is described in the second commit
Parts of the code use realloc like
x = safe_realloc(x, somesize);
when this is the case, the safe_realloc variant used must free the
old memory block in case it fails, otherwise it will leak. However,
there are also instances in the code where handling is different:
if (0 == (x = safe_realloc(y, somesize)))
return false
in this case, y should not be freed, as y is not set to NULL we
could encounter double frees. Here the safe_realloc_nofree
functions are used.
@ltx2018, @petterreinholdtsen as you both commented on #214 and #215 and @lvqcl as you contributed a few patches for out-of-memory handling, could you spare some time to take a look at this solution to that problem? I can't find a way to simulate out-of-memory situations in fuzzing, so it would be great to have someone else look at this too.
I've found a way to verify this patch through fuzzing. I've added two global variables (declared and defined only #ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION) which the fuzzer uses, a counter and a threshold. As soon as the threshold is hit, the next malloc/calloc/realloc fails. If FLAC__STREAM_ENCODER_MEMORY_ALLOCATION_ERROR or FLAC__STREAM_ENCODER_FRAMING_ERROR is returned to the fuzzer without leaking or double-freeing any memory, the fuzzer won't complain.
This added to the fuzzers turns up a double-free (first free in bitwriter_grow) without this patch, and this patch fixes it.
I'll leave this PR open for review. I plan to integrate it in a larger PR with the fuzzing additions next week
Closing this as #419 (which included these patches) has been merged