Commit 40faf3df authored by Eric Blake's avatar Eric Blake
Browse files

server: Don't free(gnutls_session_t) on gnutls_init error

This is a blatant bug, where our refactoring now causes us to try and
use free(gnutls_session_t) on an opaque type if gnutls_init() fails,
even though gnutls says that only gnutls_deinit() is safe to use at
that point.  Missed when refactoring the code, where we had instead
been using gnutls_session_t* at which point free(session) was correct.

Sadly, as of gnutls 3.7.8 we CANNOT call gnutls_deinit(session) after
gnutls_init(&session, ...) fails, because the gnutls code has an error
path (FAIL_IF_LIB_ERROR) where *session is left uninitialized, yet
does not document a sane value for us to pre-store into the opaque
gnutls_session_t variable.  I'm filing that as a bug with the gnutls
folks.

At any rate, it so happens that gnutls_init() uses calloc() to
populate gnutls_session_t (so the opaque type is indeed a pointer, and
one that can be free()d); but even so, that is liable to leak any
secondary pointers stored within the calloc'd storage if the failure
is partway through gnutls_init() - so I'm not sure why Coverity didn't
flag it.  If gnutls were to mark their function with __attribute__
((malloc(gnutls_deinit, 1))), gcc -fanalyzer would be able to flag us
as using the wrong deallocator function.

Fixes: bd9a52e0 ("server/crypto: Don't need to explicitly malloc the session pointer.", v1.13.4)
parent 9e2fdefc
Pipeline #666282029 failed with stage
in 34 minutes and 12 seconds
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment