Browse code

Add calls to nvlist_destroy to avoid leaks

Some memory leaks were detected by valgrind on the openvpn daemon, using
DCO mode on a FreeBSD platform. The leaks are caused by missing
nvlist_destroy calls in the file dco_freebsd.c.

Calls to nvlist_destroy were added, sometimes using local variables to
store nvlist pointers temporarly. A valgrind run on the updated daemon
confirmed that the leaks were gone.

Github: OpenVPN/openvpn#636
Signed-off-by: Rémi Farault <remi.farault@stormshield.eu>

Acked-by: Kristof Provost <kp@freebsd.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <f8845c0c5aa74e5bab537463249a251d@stormshield.eu>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29701.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit dee0748a1e0f57c326cf2b83f8499998ac9d1187)

Rémi Farault authored on 2024/10/29 20:06:35
Showing 1 changed files
... ...
@@ -78,7 +78,7 @@ dco_new_peer(dco_context_t *dco, unsigned int peerid, int sd,
78 78
              struct in_addr *remote_in4, struct in6_addr *remote_in6)
79 79
 {
80 80
     struct ifdrv drv;
81
-    nvlist_t *nvl;
81
+    nvlist_t *nvl, *local_nvl, *remote_nvl;
82 82
     int ret;
83 83
 
84 84
     nvl = nvlist_create(0);
... ...
@@ -87,12 +87,14 @@ dco_new_peer(dco_context_t *dco, unsigned int peerid, int sd,
87 87
 
88 88
     if (localaddr)
89 89
     {
90
-        nvlist_add_nvlist(nvl, "local", sockaddr_to_nvlist(localaddr));
90
+        local_nvl = sockaddr_to_nvlist(localaddr);
91
+        nvlist_add_nvlist(nvl, "local", local_nvl);
91 92
     }
92 93
 
93 94
     if (remoteaddr)
94 95
     {
95
-        nvlist_add_nvlist(nvl, "remote", sockaddr_to_nvlist(remoteaddr));
96
+        remote_nvl = sockaddr_to_nvlist(remoteaddr);
97
+        nvlist_add_nvlist(nvl, "remote", remote_nvl);
96 98
     }
97 99
 
98 100
     if (remote_in4)
... ...
@@ -121,6 +123,14 @@ dco_new_peer(dco_context_t *dco, unsigned int peerid, int sd,
121 121
     }
122 122
 
123 123
     free(drv.ifd_data);
124
+    if (localaddr)
125
+    {
126
+        nvlist_destroy(local_nvl);
127
+    }
128
+    if (remoteaddr)
129
+    {
130
+        nvlist_destroy(remote_nvl);
131
+    }
124 132
     nvlist_destroy(nvl);
125 133
 
126 134
     return ret;
... ...
@@ -418,7 +428,7 @@ dco_new_key(dco_context_t *dco, unsigned int peerid, int keyid,
418 418
             const char *ciphername)
419 419
 {
420 420
     struct ifdrv drv;
421
-    nvlist_t *nvl;
421
+    nvlist_t *nvl, *encrypt_nvl, *decrypt_nvl;
422 422
     int ret;
423 423
 
424 424
     msg(D_DCO_DEBUG, "%s: slot %d, key-id %d, peer-id %d, cipher %s",
... ...
@@ -430,10 +440,11 @@ dco_new_key(dco_context_t *dco, unsigned int peerid, int keyid,
430 430
     nvlist_add_number(nvl, "keyid", keyid);
431 431
     nvlist_add_number(nvl, "peerid", peerid);
432 432
 
433
-    nvlist_add_nvlist(nvl, "encrypt",
434
-                      key_to_nvlist(encrypt_key, encrypt_iv, ciphername));
435
-    nvlist_add_nvlist(nvl, "decrypt",
436
-                      key_to_nvlist(decrypt_key, decrypt_iv, ciphername));
433
+    encrypt_nvl = key_to_nvlist(encrypt_key, encrypt_iv, ciphername);
434
+    decrypt_nvl = key_to_nvlist(decrypt_key, decrypt_iv, ciphername);
435
+
436
+    nvlist_add_nvlist(nvl, "encrypt", encrypt_nvl);
437
+    nvlist_add_nvlist(nvl, "decrypt", decrypt_nvl);
437 438
 
438 439
     CLEAR(drv);
439 440
     snprintf(drv.ifd_name, IFNAMSIZ, "%s", dco->ifname);
... ...
@@ -451,6 +462,8 @@ dco_new_key(dco_context_t *dco, unsigned int peerid, int keyid,
451 451
     }
452 452
 
453 453
     free(drv.ifd_data);
454
+    nvlist_destroy(encrypt_nvl);
455
+    nvlist_destroy(decrypt_nvl);
454 456
     nvlist_destroy(nvl);
455 457
 
456 458
     return ret;
... ...
@@ -750,6 +763,7 @@ retry:
750 750
     if (!nvlist_exists_nvlist_array(nvl, "peers"))
751 751
     {
752 752
         /* no peers */
753
+        nvlist_destroy(nvl);
753 754
         return 0;
754 755
     }
755 756
 
... ...
@@ -762,6 +776,7 @@ retry:
762 762
         dco_update_peer_stat(m, peerid, nvlist_get_nvlist(peer, "bytes"));
763 763
     }
764 764
 
765
+    nvlist_destroy(nvl);
765 766
     return 0;
766 767
 }
767 768