| Message ID | 20240820144912.320744-6-peter.maydell@linaro.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | tests/qtest/migration-test: Fix various leaks | expand |
Peter Maydell <peter.maydell@linaro.org> writes: > In the migration test we create several TLS certificates with > the TLS_* macros from crypto-tls-x509-helpers.h. These macros > create both a QCryptoTLSCertReq object which must be deinitialized > and also an on-disk certificate file. The migration test currently > removes the on-disk file in test_migrate_tls_x509_finish() but > never deinitializes the QCryptoTLSCertReq, which means that memory > allocated as part of it is leaked: > > Indirect leak of 2 byte(s) in 1 object(s) allocated from: > #0 0x5558ba33712e in malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/tests/qtest/migration-test+0x22f12e) (BuildId: 4c8618f663e538538cad19d35233124cea161491) > #1 0x7f64afc131f4 (/lib/x86_64-linux-gnu/libtasn1.so.6+0x81f4) (BuildId: 2fde6ecb43c586fe4077118f771077aa1298e7ea) > #2 0x7f64afc18d58 in asn1_write_value (/lib/x86_64-linux-gnu/libtasn1.so.6+0xdd58) (BuildId: 2fde6ecb43c586fe4077118f771077aa1298e7ea) > #3 0x7f64af8fc678 in gnutls_x509_crt_set_version (/lib/x86_64-linux-gnu/libgnutls.so.30+0xe7678) (BuildId: 97b8f99f392f1fd37b969a7164bcea884e23649b) > #4 0x5558ba470035 in test_tls_generate_cert tests/unit/crypto-tls-x509-helpers.c:234:5 > #5 0x5558ba464e4a in test_migrate_tls_x509_start_common tests/qtest/migration-test.c:1058:5 > #6 0x5558ba462c8a in test_migrate_tls_x509_start_default_host tests/qtest/migration-test.c:1123:12 > #7 0x5558ba45ab40 in test_precopy_common tests/qtest/migration-test.c:1786:21 > #8 0x5558ba450015 in test_precopy_unix_tls_x509_default_host tests/qtest/migration-test.c:2077:5 > #9 0x5558ba46d3c7 in migration_test_wrapper tests/qtest/migration-helpers.c:456:5 > > (and similar reports). > > The only function currently provided to deinit a QCryptoTLSCertReq is > test_tls_discard_cert(), which also removes the on-disk certificate > file. For the migration tests we need to retain the on-disk files > until we've finished running the test, so the simplest fix is to > provide a new function test_tls_deinit_cert() which does only the > cleanup of the QCryptoTLSCertReq, and call it in the right places. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Fabiano Rosas <farosas@suse.de>
diff --git a/tests/unit/crypto-tls-x509-helpers.h b/tests/unit/crypto-tls-x509-helpers.h index 562c1606535..2a0f7c04fd8 100644 --- a/tests/unit/crypto-tls-x509-helpers.h +++ b/tests/unit/crypto-tls-x509-helpers.h @@ -73,6 +73,12 @@ void test_tls_generate_cert(QCryptoTLSTestCertReq *req, void test_tls_write_cert_chain(const char *filename, gnutls_x509_crt_t *certs, size_t ncerts); +/* + * Deinitialize the QCryptoTLSTestCertReq, but don't delete the certificate + * file on disk. (The caller is then responsible for doing that themselves. + */ +void test_tls_deinit_cert(QCryptoTLSTestCertReq *req); +/* Deinit the QCryptoTLSTestCertReq, and delete the certificate file */ void test_tls_discard_cert(QCryptoTLSTestCertReq *req); void test_tls_init(const char *keyfile); diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c index 9811047eeb4..a659609ccb0 100644 --- a/tests/qtest/migration-test.c +++ b/tests/qtest/migration-test.c @@ -1062,12 +1062,15 @@ test_migrate_tls_x509_start_common(QTestState *from, QCRYPTO_TLS_TEST_CLIENT_HOSTILE_NAME : QCRYPTO_TLS_TEST_CLIENT_NAME, data->clientcert); + test_tls_deinit_cert(&servercertreq); } TLS_CERT_REQ_SIMPLE_SERVER(clientcertreq, cacertreq, data->servercert, args->certhostname, args->certipaddr); + test_tls_deinit_cert(&clientcertreq); + test_tls_deinit_cert(&cacertreq); qtest_qmp_assert_success(from, "{ 'execute': 'object-add'," diff --git a/tests/unit/crypto-tls-x509-helpers.c b/tests/unit/crypto-tls-x509-helpers.c index 3e74ec5b5d4..b316155d6a6 100644 --- a/tests/unit/crypto-tls-x509-helpers.c +++ b/tests/unit/crypto-tls-x509-helpers.c @@ -502,8 +502,7 @@ void test_tls_write_cert_chain(const char *filename, g_free(buffer); } - -void test_tls_discard_cert(QCryptoTLSTestCertReq *req) +void test_tls_deinit_cert(QCryptoTLSTestCertReq *req) { if (!req->crt) { return; @@ -511,6 +510,15 @@ void test_tls_discard_cert(QCryptoTLSTestCertReq *req) gnutls_x509_crt_deinit(req->crt); req->crt = NULL; +} + +void test_tls_discard_cert(QCryptoTLSTestCertReq *req) +{ + if (!req->crt) { + return; + } + + test_tls_deinit_cert(req); if (getenv("QEMU_TEST_DEBUG_CERTS") == NULL) { unlink(req->filename);
In the migration test we create several TLS certificates with the TLS_* macros from crypto-tls-x509-helpers.h. These macros create both a QCryptoTLSCertReq object which must be deinitialized and also an on-disk certificate file. The migration test currently removes the on-disk file in test_migrate_tls_x509_finish() but never deinitializes the QCryptoTLSCertReq, which means that memory allocated as part of it is leaked: Indirect leak of 2 byte(s) in 1 object(s) allocated from: #0 0x5558ba33712e in malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/tests/qtest/migration-test+0x22f12e) (BuildId: 4c8618f663e538538cad19d35233124cea161491) #1 0x7f64afc131f4 (/lib/x86_64-linux-gnu/libtasn1.so.6+0x81f4) (BuildId: 2fde6ecb43c586fe4077118f771077aa1298e7ea) #2 0x7f64afc18d58 in asn1_write_value (/lib/x86_64-linux-gnu/libtasn1.so.6+0xdd58) (BuildId: 2fde6ecb43c586fe4077118f771077aa1298e7ea) #3 0x7f64af8fc678 in gnutls_x509_crt_set_version (/lib/x86_64-linux-gnu/libgnutls.so.30+0xe7678) (BuildId: 97b8f99f392f1fd37b969a7164bcea884e23649b) #4 0x5558ba470035 in test_tls_generate_cert tests/unit/crypto-tls-x509-helpers.c:234:5 #5 0x5558ba464e4a in test_migrate_tls_x509_start_common tests/qtest/migration-test.c:1058:5 #6 0x5558ba462c8a in test_migrate_tls_x509_start_default_host tests/qtest/migration-test.c:1123:12 #7 0x5558ba45ab40 in test_precopy_common tests/qtest/migration-test.c:1786:21 #8 0x5558ba450015 in test_precopy_unix_tls_x509_default_host tests/qtest/migration-test.c:2077:5 #9 0x5558ba46d3c7 in migration_test_wrapper tests/qtest/migration-helpers.c:456:5 (and similar reports). The only function currently provided to deinit a QCryptoTLSCertReq is test_tls_discard_cert(), which also removes the on-disk certificate file. For the migration tests we need to retain the on-disk files until we've finished running the test, so the simplest fix is to provide a new function test_tls_deinit_cert() which does only the cleanup of the QCryptoTLSCertReq, and call it in the right places. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- tests/unit/crypto-tls-x509-helpers.h | 6 ++++++ tests/qtest/migration-test.c | 3 +++ tests/unit/crypto-tls-x509-helpers.c | 12 ++++++++++-- 3 files changed, 19 insertions(+), 2 deletions(-)