signal_reset(): combine check and reset operations

- "if (sig == X) signal_reset(sig)" now becomes
  "signal_reset(sig, X)" so that the check and assignment
  can be done in one place where signals are masked.
  This is required to avoid change of signal state between
  check and reset operations.

- Avoid resetting the signal except when absolutely necessary
  (resetting has the potential of losing signals)

- In 'pre_init_signal_catch()', when certain low priority signals
  are set to SIG_IGN, clear any pending signals of the same
  type. Also, reset signal at the end of the SIGUSR1 and
  SIGHUP loops where their values are checked instead of later. This
  avoids the need for 'signal_reset()' after SIGHUP or in 'init_instance()'
  which could cause a signal like SIGTERM to be lost.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20230128215901.2207208-2-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26088.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 6fb6900de67fde89de149ed8865ef7f057e83bb4..80c1b2e6a320e0d9ed798167c1e8a7914c094bd1 100644 (file)
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -4457,9 +4457,6 @@ init_instance(struct context *c, const struct env_set *env, const unsigned int f
         do_inherit_env(c, env);
     }
 
-    /* signals caught here will abort */
-    signal_reset(c->sig);
-
     if (c->mode == CM_P2P)
     {
         init_management_callback_p2p(c);

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index aad11b1d26992f2d33988a71c535b1fb6b62bae0..0d4e6f999fd961ab274fa7b4437e8decc34d05f0 100644 (file)
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -3846,7 +3846,7 @@ multi_push_restart_schedule_exit(struct multi_context *m, bool next_server)
                        &m->deferred_shutdown_signal.wakeup,
                        compute_wakeup_sigma(&m->deferred_shutdown_signal.wakeup));
 
-    signal_reset(m->top.sig);
+    signal_reset(m->top.sig, 0);
 }
 
 /*
@@ -3856,12 +3856,11 @@ multi_push_restart_schedule_exit(struct multi_context *m, bool next_server)
 bool
 multi_process_signal(struct multi_context *m)
 {
-    if (m->top.sig->signal_received == SIGUSR2)
+    if (signal_reset(m->top.sig, SIGUSR2) == SIGUSR2)
     {
         struct status_output *so = status_open(NULL, 0, M_INFO, NULL, 0);
         multi_print_status(m, so, m->status_file_version);
         status_close(so);
-        signal_reset(m->top.sig);
         return false;
     }
     else if (proto_is_dgram(m->top.options.ce.proto)

diff --git a/src/openvpn/openvpn.c b/src/openvpn/openvpn.c
index 348392e2b95c0174cfcf619e4fa0ffa1821f939f..874facf68de062fa9815af24fdc2c9ca1ced0190 100644 (file)
--- a/src/openvpn/openvpn.c
+++ b/src/openvpn/openvpn.c
@@ -192,7 +192,6 @@ openvpn_main(int argc, char *argv[])
             context_clear_all_except_first_time(&c);
 
             /* static signal info object */
-            CLEAR(siginfo_static);
             c.sig = &siginfo_static;
 
             /* initialize garbage collector scoped to context object */
@@ -333,14 +332,14 @@ openvpn_main(int argc, char *argv[])
                 /* pass restart status to management subsystem */
                 signal_restart_status(c.sig);
             }
-            while (c.sig->signal_received == SIGUSR1);
+            while (signal_reset(c.sig, SIGUSR1) == SIGUSR1);
 
             env_set_destroy(c.es);
             uninit_options(&c.options);
             gc_reset(&c.gc);
             uninit_early(&c);
         }
-        while (c.sig->signal_received == SIGHUP);
+        while (signal_reset(c.sig, SIGHUP) == SIGHUP);
     }
 
     context_gc_free(&c);

diff --git a/src/openvpn/sig.c b/src/openvpn/sig.c
index dae6350ca957861822addb1ae95673eb67a161fa..7f13c6cdf25e8e77501d220299dcb8bb069d8e01 100644 (file)
--- a/src/openvpn/sig.c
+++ b/src/openvpn/sig.c
@@ -257,15 +257,37 @@ register_signal(struct signal_info *si, int signum, const char *signal_text)
     }
 }
 
-void
-signal_reset(struct signal_info *si)
+/**
+ * Clear the signal if its current value equals signum. If
+ * signum is zero the signal is cleared independent of its current
+ * value. Returns the current value of the signal.
+ */
+int
+signal_reset(struct signal_info *si, int signum)
 {
+    int sig_saved = 0;
     if (si)
     {
-        si->signal_received = 0;
-        si->signal_text = NULL;
-        si->source = SIG_SOURCE_SOFT;
+        if (si == &siginfo_static) /* attempting to alter the global signal */
+        {
+            block_async_signals();
+        }
+
+        sig_saved = si->signal_received;
+        if (!signum || sig_saved == signum)
+        {
+            si->signal_received = 0;
+            si->signal_text = NULL;
+            si->source = SIG_SOURCE_SOFT;
+            msg(D_SIGNAL_DEBUG, "signal_reset: signal %s is cleared", signal_name(signum, true));
+        }
+
+        if (si == &siginfo_static)
+        {
+            unblock_async_signals();
+        }
     }
+    return sig_saved;
 }
 
 void
@@ -395,6 +417,10 @@ pre_init_signal_catch(void)
     sigaction(SIGUSR2, &sa, NULL);
     sigaction(SIGPIPE, &sa, NULL);
 #endif /* _WIN32 */
+    /* clear any pending signals of the ignored type */
+    signal_reset(&siginfo_static, SIGUSR1);
+    signal_reset(&siginfo_static, SIGUSR2);
+    signal_reset(&siginfo_static, SIGHUP);
 }
 
 void
@@ -522,7 +548,7 @@ process_explicit_exit_notification_init(struct context *c)
      * will be ignored during the exit notification period.
      */
     halt_low_priority_signals(); /* Set hard SIGUSR1/SIGHUP/SIGUSR2 to be ignored */
-    signal_reset(c->sig);
+    signal_reset(c->sig, 0);
 
     c->c2.explicit_exit_notification_time_wait = now;
 
@@ -573,7 +599,7 @@ process_sigusr2(struct context *c)
     struct status_output *so = status_open(NULL, 0, M_INFO, NULL, 0);
     print_status(c, so);
     status_close(so);
-    signal_reset(c->sig);
+    signal_reset(c->sig, SIGUSR2);
 }
 
 static bool

diff --git a/src/openvpn/sig.h b/src/openvpn/sig.h
index b09dfab6cb6c9aca82bf9428a476179619963c0c..54c599509a8edd27127f76c74444d3ce18c3de72 100644 (file)
--- a/src/openvpn/sig.h
+++ b/src/openvpn/sig.h
@@ -81,7 +81,12 @@ void register_signal(struct signal_info *si, int sig, const char *text);
 
 void process_explicit_exit_notification_timer_wakeup(struct context *c);
 
-void signal_reset(struct signal_info *si);
+/**
+ * Clear the signal if its current value equals signum. If signum is
+ * zero the signal is cleared independent of its current value.
+ * @returns the current value of the signal.
+ */
+int signal_reset(struct signal_info *si, int signum);
 
 static inline void
 halt_non_edge_triggered_signals(void)

diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
index 2d765cc3484626f98c5269ef4512b463d485b751..501e023ea366b97326c4b35f713e01b7a3be9090 100644 (file)
--- a/src/openvpn/socket.c
+++ b/src/openvpn/socket.c
@@ -565,12 +565,11 @@ openvpn_getaddrinfo(unsigned int flags,
                 if (sig_info->signal_received) /* were we interrupted by a signal? */
                 {
                     /* why are we overwriting SIGUSR1 ? */
-                    if (sig_info->signal_received == SIGUSR1) /* ignore SIGUSR1 */
+                    if (signal_reset(sig_info, SIGUSR1) == SIGUSR1) /* ignore SIGUSR1 */
                     {
                         msg(level,
                             "RESOLVE: Ignored SIGUSR1 signal received during "
                             "DNS resolution attempt");
-                        signal_reset(sig_info);
                     }
                     else
                     {
@@ -2179,7 +2178,7 @@ link_socket_init_phase2(struct context *c)
     if (sig_info->signal_received)
     {
         sig_save = *sig_info;
-        signal_reset(sig_info);
+        sig_save.signal_received = signal_reset(sig_info, 0);
     }
 
     /* initialize buffers */

diff --git a/src/openvpn/win32.c b/src/openvpn/win32.c
index 156ee7467104f2d03fff9dca828bb8a9b508fb89..47eb1fc209584dd011aff70b699ce4de8ae235eb 100644 (file)
--- a/src/openvpn/win32.c
+++ b/src/openvpn/win32.c
@@ -675,7 +675,7 @@ win32_signal_get(struct win32_signal *ws)
     }
     if (ret)
     {
-        throw_signal(ret); /* this will update signinfo_static.signal received */
+        throw_signal(ret); /* this will update siginfo_static.signal received */
     }
     return (siginfo_static.signal_received);
 }