Menu
▾
▴
openvpn-devel
|
From: plaisthos (C. Review) <ge...@op...> - 2025-10-27 21:10:47
|
Attention is currently required from: flichtenheld.
Hello flichtenheld,
I'd like you to do a code review.
Please visit
http://gerrit.openvpn.net/c/openvpn/+/1319?usp=email
to review the following change.
Change subject: Avoid possible race condition that kill OpenVPN itself
......................................................................
Avoid possible race condition that kill OpenVPN itself
If for whatever reason the child pid is zero, we would kill ourselves
since killing 0 means killing the own process group.
Reported-By: co...@jo...
Found-By: Zeropath
Change-Id: I7b94de92723f9528b01cb932bb079eedf0f1f272
---
M src/openvpn/tun_afunix.c
1 file changed, 6 insertions(+), 1 deletion(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/19/1319/1
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);
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1319?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I7b94de92723f9528b01cb932bb079eedf0f1f272
Gerrit-Change-Number: 1319
Gerrit-PatchSet: 1
Gerrit-Owner: plaisthos <arn...@rf...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: flichtenheld <fr...@li...>
|
|
From: cron2 (C. Review) <ge...@op...> - 2025-10-27 21:33:08
|
Attention is currently required from: flichtenheld, plaisthos. cron2 has posted comments on this change by plaisthos. ( http://gerrit.openvpn.net/c/openvpn/+/1319?usp=email ) Change subject: Avoid possible race condition that kill OpenVPN itself ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1319?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I7b94de92723f9528b01cb932bb079eedf0f1f272 Gerrit-Change-Number: 1319 Gerrit-PatchSet: 1 Gerrit-Owner: plaisthos <arn...@rf...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-Comment-Date: Mon, 27 Oct 2025 21:32:54 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes |
|
From: Gert D. <ge...@gr...> - 2025-10-27 21:33:16
|
From: Arne Schwabe <ar...@rf...> If for whatever reason the child pid is zero, we would kill ourselves since killing 0 means killing the own process group. Reported-By: co...@jo... Found-By: Zeropath Change-Id: I7b94de92723f9528b01cb932bb079eedf0f1f272 Signed-off-by: Arne Schwabe <arn...@rf...> Acked-by: Gert Doering <ge...@gr...> 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 <ge...@gr...> 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); |
|
From: Gert D. <ge...@gr...> - 2025-10-28 09:34:50
|
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 <arn...@rf...>
Acked-by: Gert Doering <ge...@gr...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1319
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg33910.html
Signed-off-by: Gert Doering <ge...@gr...>
--
kind regards,
Gert Doering
|
|
From: cron2 (C. Review) <ge...@op...> - 2025-10-28 09:35:11
|
cron2 has uploaded a new patch set (#2) to the change originally created by plaisthos. ( http://gerrit.openvpn.net/c/openvpn/+/1319?usp=email ) The following approvals got outdated and were removed: Code-Review+2 by cron2 Change subject: Avoid possible race condition that kill OpenVPN itself ...................................................................... Avoid possible race condition that kill OpenVPN itself If for whatever reason the child pid is zero, we would kill ourselves since killing 0 means killing the own process group. Reported-By: Joshua Rogers <co...@jo...> Found-By: Zeropath Change-Id: I7b94de92723f9528b01cb932bb079eedf0f1f272 Signed-off-by: Arne Schwabe <arn...@rf...> Acked-by: Gert Doering <ge...@gr...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1319 Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg33910.html Signed-off-by: Gert Doering <ge...@gr...> --- M src/openvpn/tun_afunix.c 1 file changed, 6 insertions(+), 1 deletion(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/19/1319/2 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); -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1319?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I7b94de92723f9528b01cb932bb079eedf0f1f272 Gerrit-Change-Number: 1319 Gerrit-PatchSet: 2 Gerrit-Owner: plaisthos <arn...@rf...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-CC: openvpn-devel <ope...@li...> |
|
From: cron2 (C. Review) <ge...@op...> - 2025-10-28 09:35:12
|
cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/1319?usp=email ) Change subject: Avoid possible race condition that kill OpenVPN itself ...................................................................... Avoid possible race condition that kill OpenVPN itself If for whatever reason the child pid is zero, we would kill ourselves since killing 0 means killing the own process group. Reported-By: Joshua Rogers <co...@jo...> Found-By: Zeropath Change-Id: I7b94de92723f9528b01cb932bb079eedf0f1f272 Signed-off-by: Arne Schwabe <arn...@rf...> Acked-by: Gert Doering <ge...@gr...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1319 Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg33910.html Signed-off-by: Gert Doering <ge...@gr...> --- M src/openvpn/tun_afunix.c 1 file changed, 6 insertions(+), 1 deletion(-) 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); -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1319?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I7b94de92723f9528b01cb932bb079eedf0f1f272 Gerrit-Change-Number: 1319 Gerrit-PatchSet: 2 Gerrit-Owner: plaisthos <arn...@rf...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-CC: openvpn-devel <ope...@li...> |
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X