From: Petr Štetiar Date: Wed, 9 Dec 2020 12:46:58 +0000 (+0100) Subject: ustream-openssl: fix BIO_method memory leak X-Git-Url: http://git.lede-project.org./?a=commitdiff_plain;h=5896991e46a39040000841dfbb6e401473853c80;p=project%2Fustream-ssl.git ustream-openssl: fix BIO_method memory leak Fixes following issues as reported by clang-12 LeakSanitizer: $ uclient-fetch-san -q -O /dev/null 'https://expired.badssl.com/' Direct leak of 96 byte(s) in 1 object(s) allocated from: #0 0x49716d in malloc (uclient-fetch-san+0x49716d) #1 0x7f551cbabe58 in CRYPTO_zalloc (/usr/lib/x86_64-linux-gnu/libcrypto.so.1.1+0x17ae58) Indirect leak of 8 byte(s) in 1 object(s) allocated from: #0 0x49716d in malloc (uclient-fetch-san+0x49716d) #1 0x7f551cbb51c5 in CRYPTO_strdup (/usr/lib/x86_64-linux-gnu/libcrypto.so.1.1+0x1841c5) SUMMARY: AddressSanitizer: 104 byte(s) leaked in 2 allocation(s). and Valgrind: $ valgrind --quiet --leak-check=full uclient-fetch -q -O /dev/null 'https://expired.badssl.com/' ==1966== 104 (96 direct, 8 indirect) bytes in 1 blocks are definitely lost in loss record 4 of 9 ==1966== at 0x4C31B0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==1966== by 0x5FC4E58: CRYPTO_zalloc (in /usr/lib/x86_64-linux-gnu/libcrypto.so.1.1) ==1966== by 0x5EF712F: BIO_meth_new (in /usr/lib/x86_64-linux-gnu/libcrypto.so.1.1) ==1966== by 0x5C48039: ustream_bio_new (ustream-io-openssl.c:125) ==1966== by 0x5C48039: ustream_set_io (ustream-io-openssl.c:141) ==1966== by 0x5C47CB0: _ustream_ssl_init (ustream-ssl.c:210) ==1966== by 0x4E4117A: uclient_setup_https (uclient-http.c:914) ==1966== by 0x4E4117A: uclient_http_connect (uclient-http.c:936) ==1966== by 0x401FD9: init_request (uclient-fetch.c:333) ==1966== by 0x401E08: main (uclient-fetch.c:745) Suggested-by: Pan Chen Signed-off-by: Petr Štetiar --- diff --git a/ustream-io-openssl.c b/ustream-io-openssl.c index 606ed4a..7045bb6 100644 --- a/ustream-io-openssl.c +++ b/ustream-io-openssl.c @@ -48,18 +48,18 @@ s_ustream_free(BIO *b) static int s_ustream_read(BIO *b, char *buf, int len) { - struct ustream *s; + struct bio_ctx *ctx; char *sbuf; int slen; if (!buf || len <= 0) return 0; - s = (struct ustream *)BIO_get_data(b); - if (!s) + ctx = (struct bio_ctx *)BIO_get_data(b); + if (!ctx || !ctx->stream) return 0; - sbuf = ustream_get_read_buf(s, &slen); + sbuf = ustream_get_read_buf(ctx->stream, &slen); BIO_clear_retry_flags(b); if (!slen) { @@ -71,7 +71,7 @@ s_ustream_read(BIO *b, char *buf, int len) slen = len; memcpy(buf, sbuf, slen); - ustream_consume(s, slen); + ustream_consume(ctx->stream, slen); return slen; } @@ -79,19 +79,19 @@ s_ustream_read(BIO *b, char *buf, int len) static int s_ustream_write(BIO *b, const char *buf, int len) { - struct ustream *s; + struct bio_ctx *ctx; if (!buf || len <= 0) return 0; - s = (struct ustream *)BIO_get_data(b); - if (!s) + ctx = (struct bio_ctx *)BIO_get_data(b); + if (!ctx || !ctx->stream) return 0; - if (s->write_error) + if (ctx->stream->write_error) return len; - return ustream_write(s, buf, len, false); + return ustream_write(ctx->stream, buf, len, false); } static int @@ -119,19 +119,20 @@ static long s_ustream_ctrl(BIO *b, int cmd, long num, void *ptr) static BIO *ustream_bio_new(struct ustream *s) { BIO *bio; - - BIO_METHOD *methods_ustream; - - methods_ustream = BIO_meth_new(100 | BIO_TYPE_SOURCE_SINK, "ustream"); - BIO_meth_set_write(methods_ustream, s_ustream_write); - BIO_meth_set_read(methods_ustream, s_ustream_read); - BIO_meth_set_puts(methods_ustream, s_ustream_puts); - BIO_meth_set_gets(methods_ustream, s_ustream_gets); - BIO_meth_set_ctrl(methods_ustream, s_ustream_ctrl); - BIO_meth_set_create(methods_ustream, s_ustream_new); - BIO_meth_set_destroy(methods_ustream, s_ustream_free); - bio = BIO_new(methods_ustream); - BIO_set_data(bio, s); + struct bio_ctx *ctx = calloc(1, sizeof(struct bio_ctx)); + + ctx->stream = s; + ctx->meth = BIO_meth_new(100 | BIO_TYPE_SOURCE_SINK, "ustream"); + + BIO_meth_set_write(ctx->meth, s_ustream_write); + BIO_meth_set_read(ctx->meth, s_ustream_read); + BIO_meth_set_puts(ctx->meth, s_ustream_puts); + BIO_meth_set_gets(ctx->meth, s_ustream_gets); + BIO_meth_set_ctrl(ctx->meth, s_ustream_ctrl); + BIO_meth_set_create(ctx->meth, s_ustream_new); + BIO_meth_set_destroy(ctx->meth, s_ustream_free); + bio = BIO_new(ctx->meth); + BIO_set_data(bio, ctx); return bio; } diff --git a/ustream-openssl.c b/ustream-openssl.c index dec2b9f..ad77e72 100644 --- a/ustream-openssl.c +++ b/ustream-openssl.c @@ -210,8 +210,15 @@ __hidden void __ustream_ssl_context_free(struct ustream_ssl_ctx *ctx) void __ustream_ssl_session_free(void *ssl) { + BIO *bio = SSL_get_wbio(ssl); + struct bio_ctx *ctx = BIO_get_data(bio); + SSL_shutdown(ssl); SSL_free(ssl); + if (ctx) { + BIO_meth_free(ctx->meth); + free(ctx); + } } static void ustream_ssl_error(struct ustream_ssl *us, int ret) diff --git a/ustream-openssl.h b/ustream-openssl.h index 9663d21..90acc86 100644 --- a/ustream-openssl.h +++ b/ustream-openssl.h @@ -31,6 +31,11 @@ void __ustream_ssl_session_free(void *ssl); +struct bio_ctx { + BIO_METHOD *meth; + struct ustream *stream; +}; + static inline void *__ustream_ssl_session_new(void *ctx) { return SSL_new(ctx);