search

tpm2-software / tpm2-tss-engine

OpenSSL Engine for TPM2 devices
https://tpm2-software.github.io
BSD 3-Clause "New" or "Revised" License
150 stars 100 forks source link

Possible memleak in tpm2tss_rsa_makekey #161

Closed lcuminato closed 3 years ago

lcuminato commented 4 years ago

Hi,

I'm trying to assess if a possible memory leak exists in tpm2tss_rsa_makekey. The following code stores the tpm2Data object into the rsa object by using this macro RSA_set_app_data

if (!RSA_set_app_data(rsa, tpm2Data)) {
        ERR(populate_rsa, TPM2TSS_R_GENERAL_FAILURE);
        goto error;
    }

I think the problem here is that when the application code is doing cleanup and calls EVP_PKEY_free to destroy the EVP_PKEY object, which will then eventually call RSA_free() to free the RSA object, RSA_free() is not aware of the tpm2Data object and therefore it won't deallocate it.

I figure this could be it by running valgrind on my application, which is currently leaking memory at a rate of 2kb per CURL call (which uses the tpm2 as a OpenSSL engine).

This is my valgrind report:

==15907== 173,096 bytes in 77 blocks are definitely lost in loss record 58 of 58
==15907==    at 0x4A07ECB: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==15907==    by 0x31FBD8DED7: CRYPTO_malloc (in /usr/lib/libcrypto.so.1.0.2)
==15907==    by 0xCB0E292: ???
==15907==    by 0xCB0D7B5: ???
==15907==    by 0x31FBD0E15C: ENGINE_load_private_key (in /usr/lib/libcrypto.so.1.0.2)
==15907==    by 0x31FB849E35: ??? (in /usr/lib/libcurl.so.4.6.0)
==15907==    by 0x31FB84A2DF: ??? (in /usr/lib/libcurl.so.4.6.0)
==15907==    by 0x31FB84B086: ??? (in /usr/lib/libcurl.so.4.6.0)
==15907==    by 0x31FB80F6E1: ??? (in /usr/lib/libcurl.so.4.6.0)
==15907==    by 0x31FB810EDA: ??? (in /usr/lib/libcurl.so.4.6.0)
==15907==    by 0x31FB82BC2F: ??? (in /usr/lib/libcurl.so.4.6.0)
==15907==    by 0x31FB82CEDA: curl_multi_perform (in /usr/lib/libcurl.so.4.6.0)

gdb:

#0  loadkey (e=0x709140, key_id=0x6e22b0 "0x81010024", ui=0x7b1b60, cb_data=0x7a5e60) at /usr/src/debug/tpm2-tss-engine/v1.0.0-r0/git/src/tpm2-tss-engine.c:178
#1  0x00000031fbd0e15d in ENGINE_load_private_key (e=0x709140, key_id=0x6e22b0 "0x81010024", ui_method=0x7b1b60, callback_data=0x7a5e60)
    at /usr/src/debug/openssl/1.0.2n-r0/openssl-1.0.2n/crypto/engine/eng_pkey.c:121
#2  0x00000031fb849e36 in ossl_connect_step1 () from /build/toolchain/sysroots/corei7-64-poky-linux/usr/lib/libcurl.so.4

I believe the code needs to provide a finish function to the RSA object in order to free the application data up by using RSA_meth_set_finish ?

Thanks

lcuminato commented 4 years ago

So I confirmed the leak went away by creating a finish method. Do you want me to upstream my patch ?

AndreasFuchsTPM commented 4 years ago

Sure, Please provide a PR with your patch and I'll review it.