Skip to contentSkip to conversation

Plug memory leak if push is interrupted

PATCH In review Steffan Karger 2 participants 2 messages Jan 17, 2018 – Jan 25, 2018

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Plug memory leak if push is interrupted

If a push is interrupted due to a timeout, c->c2.pulled_options_state is never freed. Fix that by always cleaning up any remaining pulled options state when we close a connection.

This changes the mbedtls implementation of md_ctx_cleanup to actually clean up the context, which was not needed earlier.

---
 src/openvpn/crypto_mbedtls.c | 1 +
 src/openvpn/init.c           | 6 ++++++
 2 files changed, 7 insertions(+)
src/openvpn/crypto_mbedtls.c +1 -0
index 8fa03da..8fc252f 100644
@@ -804,6 +804,7 @@ md_ctx_init(mbedtls_md_context_t *ctx, const mbedtls_md_info_t *kt)
 void
 md_ctx_cleanup(mbedtls_md_context_t *ctx)
 {
+    mbedtls_md_free(ctx);
 }
 
 int
src/openvpn/init.c +6 -0
index 30beadb..abf8da2 100644
@@ -3384,6 +3384,12 @@ do_close_tls(struct context *c)
     }
     c->c2.options_string_local = c->c2.options_string_remote = NULL;
 #endif
+
+    if (c->c2.pulled_options_state)
+    {
+        md_ctx_cleanup(c->c2.pulled_options_state);
+        md_ctx_free(c->c2.pulled_options_state);
+    }
 }
 
 /*
Signature · 1 line
2.7.4
#1 lore raw

ACK. Makes sense.

The changes to crypto_mbedtls.c match the documentation for the mbedtls md functions, and the rest is just basic "if you make it, clean it up later on again" :-) - lightly tested ("t_client"), too lazy to come up with a test rig to trigger and truly test this.

Your patch has been applied to the master and release/2.4 branch.

commit 07036fd3c456ed4ebf1809d8d9f34941d42865d0 (master) commit d04032c3f236cf7c726d6b162686bcb71fda2e15 (release/2.4)

Author: Steffan Karger
Date:   Wed Jan 17 14:16:24 2018 +0100
     Plug memory leak if push is interrupted

     Signed-off-by: Steffan Karger <[email protected]...>
     Acked-by: Gert Doering <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/[email protected]/msg16265.html
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering