Remove --writepid file on program exit.

For whatever reason, we never removed the pid file on program exit.

Not only this is unclean, but it also makes testing for "I want this
test case to FAIL" in t_client.sh more annoying to code for "is the
OpenVPN process still around?"...

Do not unlink the file if chroot() is active (might be outside the
chroot arena - testing for realpath etc. is left for someone else).

v2: make this work on M_FATAL exit, by unlinking from openvpn_exit() in
error.h - this requires moving write_pid() to init.c so module hierarchy
is maintained and introducing a static variable to save the PID file
name (otherwise it is no longer available when the top level GC is gone).

Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20200707084220.45753-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20224.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

diff --git a/src/openvpn/error.c b/src/openvpn/error.c
index ad4f0ef44f7256609b671eb55fa50a28ca2ffae9..d6247fec7943ac6fa3b6754e2ce751d1090b2053 100644 (file)
--- a/src/openvpn/error.c
+++ b/src/openvpn/error.c
@@ -743,6 +743,7 @@ openvpn_exit(const int status)
 #ifdef _WIN32
         uninit_win32();
 #endif
+        remove_pid_file();
 
         close_syslog();
 

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index dd1747f337e65a3f485e6725b6f7283f2329caf8..91b919d5af656b8db74bcd5a57ae9cd7f85fe4ca 100644 (file)
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -58,6 +58,7 @@
 
 
 static struct context *static_context; /* GLOBAL */
+static const char *saved_pid_file_name; /* GLOBAL */
 
 /*
  * Crypto initialization flags
@@ -4687,6 +4688,47 @@ close_context(struct context *c, int sig, unsigned int flags)
     }
 }
 
+/* Write our PID to a file */
+void
+write_pid_file(const char *filename, const char *chroot_dir)
+{
+    if (filename)
+    {
+        unsigned int pid = 0;
+        FILE *fp = platform_fopen(filename, "w");
+        if (!fp)
+        {
+            msg(M_ERR, "Open error on pid file %s", filename);
+            return;
+        }
+
+        pid = platform_getpid();
+        fprintf(fp, "%u\n", pid);
+        if (fclose(fp))
+        {
+            msg(M_ERR, "Close error on pid file %s", filename);
+        }
+
+        /* remember file name so it can be deleted "out of context" later */
+        /* (the chroot case is more complex and not handled today) */
+        if (!chroot_dir)
+        {
+            saved_pid_file_name = strdup(filename);
+        }
+    }
+}
+
+/* remove PID file on exit, called from openvpn_exit() */
+void
+remove_pid_file(void)
+{
+    if (saved_pid_file_name)
+    {
+        platform_unlink(saved_pid_file_name);
+    }
+}
+
+
 /*
  * Do a loopback test
  * on the crypto subsystem.

diff --git a/src/openvpn/init.h b/src/openvpn/init.h
index 0e6258f0b8de63886780cb6cf07fa4f4a244bb66..a2fdccd36d6a96875390be08d6d1ff776d21954c 100644 (file)
--- a/src/openvpn/init.h
+++ b/src/openvpn/init.h
@@ -143,4 +143,7 @@ void open_plugins(struct context *c, const bool import_options, int init_point);
 
 void tun_abort(void);
 
+void write_pid_file(const char *filename, const char *chroot_dir);
+void remove_pid_file(void);
+
 #endif /* ifndef INIT_H */

diff --git a/src/openvpn/openvpn.c b/src/openvpn/openvpn.c
index dc7001dce6cc4182f2d288ca0992faf4f803f9c5..857c5faa377145d27879389806d464c48ac963ce 100644 (file)
--- a/src/openvpn/openvpn.c
+++ b/src/openvpn/openvpn.c
@@ -46,28 +46,6 @@ process_signal_p2p(struct context *c)
     return process_signal(c);
 }
 
-/* Write our PID to a file */
-static void
-write_pid(const char *filename)
-{
-    if (filename)
-    {
-        unsigned int pid = 0;
-        FILE *fp = platform_fopen(filename, "w");
-        if (!fp)
-        {
-            msg(M_ERR, "Open error on pid file %s", filename);
-        }
-
-        pid = platform_getpid();
-        fprintf(fp, "%u\n", pid);
-        if (fclose(fp))
-        {
-            msg(M_ERR, "Close error on pid file %s", filename);
-        }
-    }
-}
-
 
 /**************************************************************************/
 /**
@@ -274,7 +252,7 @@ openvpn_main(int argc, char *argv[])
             if (c.first_time)
             {
                 c.did_we_daemonize = possibly_become_daemon(&c.options);
-                write_pid(c.options.writepid);
+                write_pid_file(c.options.writepid, c.options.chroot_dir);
             }
 
 #ifdef ENABLE_MANAGEMENT