]> git.ipfire.org Git - thirdparty/openvpn.git/commitdiff
Assign and honour signal priority order
authorSelva Nair <selva.nair@gmail.com>
Sun, 1 Jan 2023 21:51:07 +0000 (16:51 -0500)
committerGert Doering <gert@greenie.muc.de>
Sun, 8 Jan 2023 16:47:43 +0000 (17:47 +0100)
Signals are ordered as SIGUSR2, SIGUSR1, SIGHUP, SIGTERM, SIGINT
in increasing priority. Lower priority signals are not allowed to
overwrite higher ones.

This should fix Trac #311, #639 -- SIGTER/SIGINT lost during dns
resolution (except for the Windows-specific bug handled in previous commit).

On sending SIGTERM during dns resolution, it still takes several seconds
to terminate as the signal will get processed only after getaddrinfo times
out twice (in phase1 and phase2 inits).

Github: fixes OpenVPN/openvpn#205
Trac: #311, #639

Note: one has to still wait for address resolution to time out as
getaddrinfo() is no interruptible. But a single ctrl-C (and some
patience) is enough.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20230101215109.1521549-4-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25871.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
src/openvpn/proxy.c
src/openvpn/sig.c
src/openvpn/socket.c
src/openvpn/socks.c

index 91121f250fbd1f2bbf36cc2c284fbf408169e55c..120ba85e92a0d4d1e314936243918ce96881e2ec 100644 (file)
@@ -1080,10 +1080,7 @@ done:
     return ret;
 
 error:
-    if (!sig_info->signal_received)
-    {
-        register_signal(sig_info, SIGUSR1, "HTTP proxy error"); /* SOFT-SIGUSR1 -- HTTP proxy error */
-    }
+    register_signal(sig_info, SIGUSR1, "HTTP proxy error"); /* SOFT-SIGUSR1 -- HTTP proxy error */
     gc_free(&gc);
     return ret;
 }
index e462b93e4ac62ae92662fa91e23768af6f4aba94..d6b18cb191dfc1a5487ca732848d59766f0529ce 100644 (file)
@@ -47,16 +47,17 @@ struct signal_info siginfo_static; /* GLOBAL */
 
 struct signame {
     int value;
+    int priority;
     const char *upper;
     const char *lower;
 };
 
 static const struct signame signames[] = {
-    { SIGINT,  "SIGINT",  "sigint"},
-    { SIGTERM, "SIGTERM", "sigterm" },
-    { SIGHUP,  "SIGHUP",  "sighup" },
-    { SIGUSR1, "SIGUSR1", "sigusr1" },
-    { SIGUSR2, "SIGUSR2", "sigusr2" }
+    { SIGINT, 5, "SIGINT",  "sigint"},
+    { SIGTERM, 4, "SIGTERM", "sigterm" },
+    { SIGHUP, 3, "SIGHUP",  "sighup" },
+    { SIGUSR1, 2, "SIGUSR1", "sigusr1" },
+    { SIGUSR2, 1, "SIGUSR2", "sigusr2" }
 };
 
 int
@@ -73,6 +74,19 @@ parse_signal(const char *signame)
     return -1;
 }
 
+static int
+signal_priority(int sig)
+{
+    for (size_t i = 0; i < SIZE(signames); ++i)
+    {
+        if (sig == signames[i].value)
+        {
+            return signames[i].priority;
+        }
+    }
+    return -1;
+}
+
 const char *
 signal_name(const int sig, const bool upper)
 {
@@ -103,16 +117,22 @@ signal_description(const int signum, const char *sigtext)
 void
 throw_signal(const int signum)
 {
-    siginfo_static.signal_received = signum;
-    siginfo_static.source = SIG_SOURCE_HARD;
+    if (signal_priority(signum) >= signal_priority(siginfo_static.signal_received))
+    {
+        siginfo_static.signal_received = signum;
+        siginfo_static.source = SIG_SOURCE_HARD;
+    }
 }
 
 void
 throw_signal_soft(const int signum, const char *signal_text)
 {
-    siginfo_static.signal_received = signum;
-    siginfo_static.source = SIG_SOURCE_SOFT;
-    siginfo_static.signal_text = signal_text;
+    if (signal_priority(signum) >= signal_priority(siginfo_static.signal_received))
+    {
+        siginfo_static.signal_received = signum;
+        siginfo_static.source = SIG_SOURCE_SOFT;
+        siginfo_static.signal_text = signal_text;
+    }
 }
 
 void
@@ -472,9 +492,10 @@ process_signal(struct context *c)
 void
 register_signal(struct signal_info *si, int sig, const char *text)
 {
-    if (si->signal_received != SIGTERM)
+    if (signal_priority(sig) >= signal_priority(si->signal_received))
     {
         si->signal_received = sig;
+        si->signal_text = text;
+        si->source = SIG_SOURCE_SOFT;
     }
-    si->signal_text = text;
 }
index 77c5c2ff8bb90902daa39f8726bc931206f09480..bb3be42832affa876d7a8811f7d788d73c9259c4 100644 (file)
@@ -2277,8 +2277,12 @@ link_socket_init_phase2(struct context *c)
 done:
     if (sig_save.signal_received)
     {
-        /* This can potentially lose a saved high priority signal -- to be fixed */
-        if (!sig_info->signal_received)
+        /* Always restore the saved signal -- register/throw_signal will handle priority */
+        if (sig_save.source == SIG_SOURCE_HARD && sig_info == &siginfo_static)
+        {
+            throw_signal(sig_save.signal_received);
+        }
+        else
         {
             register_signal(sig_info, sig_save.signal_received, sig_save.signal_text);
         }
index b2ca37441b84468436a93b50c9735236ef3c4ff6..8f2ae226c28e1b6be2fb1173a7b65859fc729835 100644 (file)
@@ -499,11 +499,8 @@ establish_socks_proxy_passthru(struct socks_proxy_info *p,
     return;
 
 error:
-    if (!sig_info->signal_received)
-    {
-        /* SOFT-SIGUSR1 -- socks error */
-        register_signal(sig_info, SIGUSR1, "socks-error");
-    }
+    /* SOFT-SIGUSR1 -- socks error */
+    register_signal(sig_info, SIGUSR1, "socks-error");
     return;
 }
 
@@ -543,11 +540,8 @@ establish_socks_proxy_udpassoc(struct socks_proxy_info *p,
     return;
 
 error:
-    if (!sig_info->signal_received)
-    {
-        /* SOFT-SIGUSR1 -- socks error */
-        register_signal(sig_info, SIGUSR1, "socks-error");
-    }
+    /* SOFT-SIGUSR1 -- socks error */
+    register_signal(sig_info, SIGUSR1, "socks-error");
     return;
 }