Attention is currently required from: Arne Schwabe.
Hello Arne Schwabe,
I'd like you to do a code review.
Please visit
https://gerrit.openvpn.net/c/openvpn/+/2180?usp=email
to review the following change.
Change subject: Free address lists by DNS cache ownership, not --preresolve
......................................................................
Free address lists by DNS cache ownership, not --preresolve
With --preresolve, remote_list and bind_local were never freed on the
assumption that they came from the DNS cache, which is freed with the
context gc. Lookups that miss the cache leaked a getaddrinfo() list on
every restart. Meanwhile phase2_socks_client() freed remote_list
unconditionally, a double free once the cache hits.
Free a list unless the cache owns it.
Change-Id: I4b07dbc6ad1f29021e1be4766500f4d291bbadc6
Signed-off-by: Razvan Cojocaru <[email protected]>
---
M src/openvpn/init.c
M src/openvpn/resolve.c
M src/openvpn/resolve.h
M src/openvpn/socket.c
4 files changed, 29 insertions(+), 7 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/80/2180/1
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 3bc4de4..d40959e 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -495,9 +495,9 @@
* Clear the remote address list
*/
static void
-clear_remote_addrlist(struct link_socket_addr *lsa, bool free)
+clear_remote_addrlist(struct link_socket_addr *lsa, const struct
cached_dns_entry *dns_cache)
{
- if (lsa->remote_list && free)
+ if (lsa->remote_list && !dns_cache_owns(dns_cache, lsa->remote_list))
{
freeaddrinfo(lsa->remote_list);
}
@@ -550,8 +550,7 @@
*/
if (c->c1.link_socket_addrs[0].remote_list)
{
- clear_remote_addrlist(&c->c1.link_socket_addrs[0],
- !c->options.resolve_in_advance);
+ clear_remote_addrlist(&c->c1.link_socket_addrs[0],
c->c1.dns_cache);
}
/* close_instance should have cleared the addrinfo objects
*/
@@ -4011,7 +4010,7 @@
&& c->c1.link_socket_addrs[0].current_remote->ai_next)
|| c->options.no_advance)))))
{
- clear_remote_addrlist(&c->c1.link_socket_addrs[0],
!c->options.resolve_in_advance);
+ clear_remote_addrlist(&c->c1.link_socket_addrs[0], c->c1.dns_cache);
}
/* Clear the remote actual address when persist_remote_ip is not in use */
@@ -4027,7 +4026,8 @@
{
for (int i = 0; i < c->c1.link_sockets_num; i++)
{
- if (c->c1.link_socket_addrs[i].bind_local &&
!c->options.resolve_in_advance)
+ if (c->c1.link_socket_addrs[i].bind_local
+ && !dns_cache_owns(c->c1.dns_cache,
c->c1.link_socket_addrs[i].bind_local))
{
freeaddrinfo(c->c1.link_socket_addrs[i].bind_local);
}
diff --git a/src/openvpn/resolve.c b/src/openvpn/resolve.c
index b8d9899..46a7f7b 100644
--- a/src/openvpn/resolve.c
+++ b/src/openvpn/resolve.c
@@ -524,6 +524,19 @@
return -1;
}
+bool
+dns_cache_owns(const struct cached_dns_entry *dns_cache, const struct addrinfo
*ai)
+{
+ for (const struct cached_dns_entry *ph = dns_cache; ph; ph = ph->next)
+ {
+ if (ph->ai == ai)
+ {
+ return true;
+ }
+ }
+ return false;
+}
+
static int
do_preresolve_host(struct context *c, const char *hostname, const char
*servname, const int af,
diff --git a/src/openvpn/resolve.h b/src/openvpn/resolve.h
index 9069af7..30b1a7e 100644
--- a/src/openvpn/resolve.h
+++ b/src/openvpn/resolve.h
@@ -103,4 +103,10 @@
const char *servname, int ai_family,
unsigned int resolve_flags, struct addrinfo **ai);
+/**
+ * Returns true if \c ai is a cached entry. Those are freed with the
+ * context gc, never with freeaddrinfo().
+ */
+bool dns_cache_owns(const struct cached_dns_entry *dns_cache, const struct
addrinfo *ai);
+
#endif /* RESOLVE_H */
diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
index 1fbd07b..d7670ae 100644
--- a/src/openvpn/socket.c
+++ b/src/openvpn/socket.c
@@ -1266,7 +1266,10 @@
addr_zero_host(&sock->info.lsa->actual.dest);
if (sock->info.lsa->remote_list)
{
- freeaddrinfo(sock->info.lsa->remote_list);
+ if (!dns_cache_owns(sock->dns_cache, sock->info.lsa->remote_list))
+ {
+ freeaddrinfo(sock->info.lsa->remote_list);
+ }
sock->info.lsa->current_remote = NULL;
sock->info.lsa->remote_list = NULL;
}
--
To view, visit https://gerrit.openvpn.net/c/openvpn/+/2180?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I4b07dbc6ad1f29021e1be4766500f4d291bbadc6
Gerrit-Change-Number: 2180
Gerrit-PatchSet: 1
Gerrit-Owner: Răzvan Cojocaru <[email protected]>
Gerrit-Reviewer: Arne Schwabe <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: Arne Schwabe <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
