| Message ID | PS1PR03MB500329AE6D7A3778B9B54166DB100@PS1PR03MB5003.apcprd03.prod.outlook.com |
|---|---|
| State | Rejected |
| Delegated to: | Daniel Golle |
| Headers | show |
| Series | ustream-ssl: openssl: fix bio memory leak | expand |
On Mon, Nov 02, 2020 at 09:53:28AM +0800, serial115200@outlook.com wrote: > From: Pan Chen <serial115200@outlook.com> > > free memory of bio method when ustream be freed Looked good on the first view, but doesn't compile: ustream-io-openssl.c: In function 's_ustream_free': ustream-io-openssl.c:45:17: error: dereferencing pointer to incomplete type 'BIO' {aka 'struct bio_st'} BIO_meth_free(b->method); ^~ This is due to type BIO being opaque. We may have to keep a reference of the ustream method in order to be able to free it. > > Signed-off-by: Pan Chen <serial115200@outlook.com> > --- > openssl_bio_compat.h | 7 +++++++ > ustream-io-openssl.c | 1 + > 2 files changed, 8 insertions(+) > > diff --git a/openssl_bio_compat.h b/openssl_bio_compat.h > index 9355c86..39a0455 100644 > --- a/openssl_bio_compat.h > +++ b/openssl_bio_compat.h > @@ -28,6 +28,13 @@ static inline BIO_METHOD *BIO_meth_new(int type, const char *name) > return bm; > } > > +static inline void BIO_meth_free(BIO_METHOD *bm) > +{ > + if (bm != NULL) { > + free(bm); > + } > +} > + > #endif /* OPENSSL_VERSION_NUMBER */ > > #endif /* OPENSSL_BIO_COMPAT_H */ > diff --git a/ustream-io-openssl.c b/ustream-io-openssl.c > index 606ed4a..c9d7fae 100644 > --- a/ustream-io-openssl.c > +++ b/ustream-io-openssl.c > @@ -42,6 +42,7 @@ s_ustream_free(BIO *b) > BIO_set_data(b, NULL); > BIO_set_init(b, 0); > BIO_clear_flags(b, ~0); > + BIO_meth_free(b->method); > return 1; > } > > -- > 2.24.1.windows.2 > > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
On Sun, Nov 1, 2020 at 6:13 PM <serial115200@outlook.com> wrote: > > From: Pan Chen <serial115200@outlook.com> > > free memory of bio method when ustream be freed > > Signed-off-by: Pan Chen <serial115200@outlook.com> > --- > openssl_bio_compat.h | 7 +++++++ > ustream-io-openssl.c | 1 + > 2 files changed, 8 insertions(+) > > diff --git a/openssl_bio_compat.h b/openssl_bio_compat.h > index 9355c86..39a0455 100644 > --- a/openssl_bio_compat.h > +++ b/openssl_bio_compat.h > @@ -28,6 +28,13 @@ static inline BIO_METHOD *BIO_meth_new(int type, const char *name) > return bm; > } > > +static inline void BIO_meth_free(BIO_METHOD *bm) > +{ > + if (bm != NULL) { > + free(bm); > + } > +} > + > #endif /* OPENSSL_VERSION_NUMBER */ > > #endif /* OPENSSL_BIO_COMPAT_H */ > diff --git a/ustream-io-openssl.c b/ustream-io-openssl.c > index 606ed4a..c9d7fae 100644 > --- a/ustream-io-openssl.c > +++ b/ustream-io-openssl.c > @@ -42,6 +42,7 @@ s_ustream_free(BIO *b) > BIO_set_data(b, NULL); > BIO_set_init(b, 0); > BIO_clear_flags(b, ~0); > + BIO_meth_free(b->method); Is this needed? AFAIK, OpenSSL 1.1 frees everything automatically. > return 1; > } > > -- > 2.24.1.windows.2 > > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Rosen Penev <rosenp@gmail.com> [2020-12-08 17:02:03]:
Hi,
> Is this needed? AFAIK, OpenSSL 1.1 frees everything automatically.
LeakSanitizer:
$ uclient-fetch-san -q -O /dev/null 'https://expired.badssl.com/'
=================================================================
==1990==ERROR: LeakSanitizer: detected memory leaks
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).
[1]
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)
==1966==
-- ynezz
diff --git a/openssl_bio_compat.h b/openssl_bio_compat.h index 9355c86..39a0455 100644 --- a/openssl_bio_compat.h +++ b/openssl_bio_compat.h @@ -28,6 +28,13 @@ static inline BIO_METHOD *BIO_meth_new(int type, const char *name) return bm; } +static inline void BIO_meth_free(BIO_METHOD *bm) +{ + if (bm != NULL) { + free(bm); + } +} + #endif /* OPENSSL_VERSION_NUMBER */ #endif /* OPENSSL_BIO_COMPAT_H */ diff --git a/ustream-io-openssl.c b/ustream-io-openssl.c index 606ed4a..c9d7fae 100644 --- a/ustream-io-openssl.c +++ b/ustream-io-openssl.c @@ -42,6 +42,7 @@ s_ustream_free(BIO *b) BIO_set_data(b, NULL); BIO_set_init(b, 0); BIO_clear_flags(b, ~0); + BIO_meth_free(b->method); return 1; }