I would like to see pull request 420 (or something similar) merged into the
next release. It's been fairly complicated to trace through the various
changes to this section of code, but I believe I have figured out the
sequence of changes.
From what I can tell, there was a memory leak with the OpenSSL AES-CTR
ciphers. These cipher structs were created in _libssh2_openssl_crypto_init(),
but never cleaned up. The leak was fixed in pull request 244, which added
the _libssh2_openssl_crypto_exit() function. This change was included in
the 1.9.0 release.
However, that change also introduced a use-after-free bug. The function
scoped static pointers in the _libssh2_EVP_aes_XXX_ctr() functions would
never be reset to NULL when _libssh2_openssl_crypto_exit() is called, and
therefore pointers to already freed structs would be returned from the the
_libssh2_EVP_aes_XXX_ctr() functions. This leads to crashes during repeated
init/cleanup cycles. The use-after-free problem was fixed in pull request
387, which was merged into master after the 1.9.0 release, so would be
included in a 1.9.1 release. A crash in our application due to the
use-after free bug is what made me look into this in the first place.
However, that change re-introduced a similar memory leak as existed
originally. The AES-CTR ciphers can now be created without being cleaned
up, if the _libssh2_EVP_aes_XXX_ctr() functions are ever called outside the
_libssh2_openssl_crypto_init() function. Previously, callers of these
functions did not need to worry about freeing the result, as it was
intended that there would only ever be one instance of each of the ciphers,
and they would be cleaned up in the library shutdown. I believe that pull
request 420 reinstates this behaviour.
https://github.com/libssh2/libssh2/pull/244
https://github.com/libssh2/libssh2/pull/387
https://github.com/libssh2/libssh2/pull/42
Thanks,
Joel
_______________________________________________
libssh2-devel https://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
Received on 2019-11-15