Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add calls to nvlist_destroy to avoid memory leaks #636

Conversation

remif-stormshield
Copy link
Contributor

Fix nvlist memory leaks

Issue

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.

Patch content

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.

@remif-stormshield remif-stormshield marked this pull request as ready for review October 30, 2024 10:20
@cron2
Copy link
Contributor

cron2 commented Oct 30, 2024

Thanks for that. Copying in @kprovost as the initial author of the dco_freebsd.c source.

When approved, we'd ask you to send the patch to the openvpn-devel lists or to our Gerrit system, as we don't do GH merges. Thanks ;-)

@kprovost
Copy link
Contributor

LGTM

It's a common problem with nvlists-in-nvlists, and it's not the first time it's caught me.

@cron2
Copy link
Contributor

cron2 commented Oct 30, 2024

THAT was quick, I just see the patch come in via the list - thanks, will take it from there.

@remif-stormshield
Copy link
Contributor Author

haha yeah I sent it just after opening the PR, but wasn't sure the mail was received yet.
My pleasure to help, have a good day :)

}

if (remoteaddr)
{
nvlist_add_nvlist(nvl, "remote", sockaddr_to_nvlist(remoteaddr));
remote_nvl = sockaddr_to_nvlist(remoteaddr);
nvlist_add_nvlist(nvl, "remote", remove_nvl);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove_nvl is not remote_nvl, so this won't work :-) - I've replied by mail to the list post, and asked for a v2 there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops, sorry for this, I sent the patch too quickly... I'll send a corrected v2 today

cron2 pushed a commit that referenced this pull request Nov 5, 2024
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: #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>
cron2 pushed a commit that referenced this pull request Nov 5, 2024
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: #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 dee0748)
@cron2 cron2 closed this Nov 6, 2024
mrbff pushed a commit to mandelbitdev/openvpn that referenced this pull request Dec 16, 2024
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#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>
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants