[Openvpn-devel,v1] Avoid possible race condition that kill OpenVPN itself

Message ID 20251027213308.5588-1-gert@greenie.muc.de
State New
Headers show
Series [Openvpn-devel,v1] Avoid possible race condition that kill OpenVPN itself | expand

Commit Message

Gert Doering Oct. 27, 2025, 9:33 p.m. UTC
From: Arne Schwabe <arne@rfc2549.org>

If for whatever reason the child pid is zero, we would kill ourselves
since killing 0 means killing the own process group.

Reported-By: contact@joshua.hu
Found-By: Zeropath
Change-Id: I7b94de92723f9528b01cb932bb079eedf0f1f272
Signed-off-by: Arne Schwabe <arne-openvpn@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1319
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1319
This mail reflects revision 1 of this Change.

Signed-off-by line for the author was added as per our policy.

Acked-by according to Gerrit (reflected above):
Gert Doering <gert@greenie.muc.de>

Comments

Gert Doering Oct. 28, 2025, 9:34 a.m. UTC | #1
Amazing find by ZeroPath, and trivially correct fix.

I have completed Joshua's "reported-by:" line (full name) in the
commit message.

Your patch has been applied to the master branch.

commit 18309ff64833523c1ad19e7d56d6f756b53966af
Author: Arne Schwabe
Date:   Mon Oct 27 22:33:02 2025 +0100

     Avoid possible race condition that kill OpenVPN itself

     Signed-off-by: Arne Schwabe <arne-openvpn@rfc2549.org>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1319
     Message-Id: <20251027213308.5588-1-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg33910.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/tun_afunix.c b/src/openvpn/tun_afunix.c
index 4d48a31..124db6d 100644
--- a/src/openvpn/tun_afunix.c
+++ b/src/openvpn/tun_afunix.c
@@ -128,7 +128,12 @@ 
         close(tt->fd);
         tt->fd = 0;
     }
-    kill(tt->afunix.childprocess, SIGINT);
+    /* only kill the child process if the PID is not 0 to avoid killing
+     * ourselves by accident */
+    if (tt->afunix.childprocess)
+    {
+        kill(tt->afunix.childprocess, SIGINT);
+    }
 
     free(tt->actual_name);
     free(tt);