[openssl-dev] [openssl.org #3891] [PATCH] Fix undefined behavior executed through OpenSSL tests

[Salz, Rich via RT]

Generally, these look good.  I have concerns about three (that you raised); quoting from your README.  Any comments from others?

+ err.c.patch
The 'int_thread_del_item' function calls 'int_thread_release' that accesses (*hash), but this is invalid because  'int_thread_del_item' frees 'int_thread_hash' that can be an alias of 'hash'. This patch fixes the problem, but WARNING: it changes the program behavior since 'int_thread_release' now returns earlier and then doesn't call CRYPTO_add. Don't know whether this is the correct fix for this problem.

+ mem_dbg.c.patch
The 'pop_info' function return 'ret' after OPENSSL_free(ret), and the returned value is then tested (ret = (pop_info() != NULL)) in CRYPTO_pop_info,
which is incorrect since the address is now a dangling pointer ("indeterminate" in the C standard). This patch fixes the problem, but don't know whether this is the correct fix regarding the behavior of the 'pop_info' callers. Regardless, returning an address that has just been passed to free() is never useful and a change is necessary here.

+ Patches about catching memory allocation errors are grouped in malloc.patch
Most of them consist on adding tests about fields being non-NULL before accessing to sub-fields, or tests on the returned value of functions that where memory allocation may have failed.