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

[Kurt Roeckx via RT]

On Tue, Jun 02, 2015 at 03:50:19PM +0200, Pascal Cuoq via RT wrote:
> The attached archive contains a collection of patches for undefined behaviors
> that happen while the tests in directory tests/ are executed, with a recent
> (as of June 2015) OpenSSL git version.
> 
> Each undefined behavior really happens for at least one
> execution, the execution of the test. In other terms, none of these is a
> "false positive". The issues broadly fall in the following categories:

So some of the patches got applied, but I have some comments about
the remaining:

- cast_lcl.h.patch: Your patch has the same effect as defining
  PEDANTIC.  I recommend you at least run your tool with PEDANTIC
  defined.
- ssl_locl.h.patch: I don't see a struct timeval
  crypto/x509v3/v3_scts.c.  Does this comment still apply?  Maybe
  we fixed the issue in some other way.
- malloc.patch: I started looking at it, and I have some
  comments/questions:
  - I currently don't see a way that s->d1 can be NULL except
    after an dtls1_free() call.  The same seem to go for
    DTLS_RECORD_LAYER_free(), ssl3_free(), pkey_hmac_cleanup(),
    aes_gcm_cleanup() and aes_ocb_cleanup().
    Are you saying there are cases we could end up calling those
    twice?
  - It seems to contain changes to the test suite to check return
    values.  It seems non-obvious that this is about memory
    allocation that might have failed, but it's probably the only
    reasons those failures can happen.  It's a little confusing
    that it's in the same patch where you can't directly see the
    malloc failing.


Kurt