Attention is currently required from: Arne Schwabe.
Hello Arne Schwabe,
I'd like you to reexamine a change. Please visit
https://gerrit.openvpn.net/c/openvpn/+/2173?usp=email
to look at the new patch set (#3).
Change subject: tun_afunix: reap the child process on close
......................................................................
tun_afunix: reap the child process on close
close_tun_afunix() sent SIGINT to the child but never waited for it,
leaving a zombie on every restart.
Add openvpn_stop_child() next to openvpn_waitpid_check(). It waits up
to a second for the child to exit, then SIGKILLs it and waits up to
another second. It doesn't wait when SIGINT can't be delivered, e.g. to
a child started before --user dropped privileges.
openvpn_waitpid_check() now returns false and logs why when waitpid()
fails, as documented, instead of reporting a process that isn't our
child as running. A clean exit is no longer logged as an error status.
Change-Id: I0705cff831cbd18686721dffbe5960976e3489da
Signed-off-by: Razvan Cojocaru <[email protected]>
---
M src/openvpn/run_command.c
M src/openvpn/run_command.h
M src/openvpn/tun_afunix.c
3 files changed, 60 insertions(+), 2 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/73/2173/3
diff --git a/src/openvpn/run_command.c b/src/openvpn/run_command.c
index 0e36dad..db25792 100644
--- a/src/openvpn/run_command.c
+++ b/src/openvpn/run_command.c
@@ -115,10 +115,15 @@
}
int status;
pid_t pidret = waitpid(pid, &status, WNOHANG);
- if (pidret != pid)
+ if (pidret == 0)
{
return true;
}
+ if (pidret != pid)
+ {
+ msg(msglevel | M_ERRNO, "%scannot wait for external program",
msg_prefix);
+ return false;
+ }
if (WIFEXITED(status))
{
@@ -128,6 +133,10 @@
{
msg(msglevel, "%scould not execute external program (exit code
127)", msg_prefix);
}
+ else if (exitcode == 0)
+ {
+ msg(msglevel, "%sexternal program exited", msg_prefix);
+ }
else
{
msg(msglevel, "%sexternal program exited with error status: %d",
msg_prefix, exitcode);
@@ -140,6 +149,44 @@
return false;
}
+
+/* Returns true if the child was reaped (or is gone) within ms milliseconds. */
+static bool
+wait_for_child_exit(pid_t pid, const char *msg_prefix, unsigned int ms)
+{
+ for (unsigned int waited = 0; openvpn_waitpid_check(pid, msg_prefix,
D_CLOSE); waited += 10)
+ {
+ if (waited >= ms)
+ {
+ return false;
+ }
+ platform_sleep_milliseconds(10);
+ }
+ return true;
+}
+
+void
+openvpn_stop_child(pid_t pid, const char *msg_prefix)
+{
+ if (kill(pid, SIGINT) != 0)
+ {
+ msg(M_WARN | M_ERRNO, "%scannot stop child process %d", msg_prefix,
(int)pid);
+ return;
+ }
+
+ if (wait_for_child_exit(pid, msg_prefix, 1000))
+ {
+ return;
+ }
+
+ msg(M_WARN, "%schild process %d did not exit on SIGINT, killing it",
msg_prefix, (int)pid);
+ kill(pid, SIGKILL);
+
+ if (!wait_for_child_exit(pid, msg_prefix, 1000))
+ {
+ msg(M_WARN, "%scannot reap child process %d", msg_prefix, (int)pid);
+ }
+}
#endif /* ifndef WIN32 */
bool
diff --git a/src/openvpn/run_command.h b/src/openvpn/run_command.h
index e11b6a1..c1f1f4c 100644
--- a/src/openvpn/run_command.h
+++ b/src/openvpn/run_command.h
@@ -79,6 +79,17 @@
*/
bool openvpn_waitpid_check(pid_t pid, const char *msg_prefix, msglvl_t
msglevel);
+/**
+ * Stops and reaps a process started with \c S_NOWAITPID: sends SIGINT,
+ * then SIGKILL if it hasn't exited after a second, and gives up after
+ * another second. Doesn't wait if SIGINT can't be delivered, e.g. to a
+ * process started before --user dropped privileges.
+ *
+ * @param pid pid of the process to be stopped
+ * @param msg_prefix prefix of the messages to be printed
+ */
+void openvpn_stop_child(pid_t pid, const char *msg_prefix);
+
#endif
/**
diff --git a/src/openvpn/tun_afunix.c b/src/openvpn/tun_afunix.c
index 79d706a..b4daf9d 100644
--- a/src/openvpn/tun_afunix.c
+++ b/src/openvpn/tun_afunix.c
@@ -138,7 +138,7 @@
* ourselves by accident */
if (tt->afunix.childprocess)
{
- kill(tt->afunix.childprocess, SIGINT);
+ openvpn_stop_child(tt->afunix.childprocess, "AF_UNIX tun: ");
}
free(tt->actual_name);
--
To view, visit https://gerrit.openvpn.net/c/openvpn/+/2173?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I0705cff831cbd18686721dffbe5960976e3489da
Gerrit-Change-Number: 2173
Gerrit-PatchSet: 3
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
