cURL / Mailing Lists / curl-library / Single Mail

curl-library

[PATCH] hostip: Fix signal race in Curl_resolv_timeout.

From: Tobias Stoeckmann <tobias_at_stoeckmann.org>
Date: Sat, 14 Mar 2015 14:32:42 +0100

A signal handler for SIGALRM is installed in Curl_resolv_timeout. It is
configured to interrupt system calls and uses siglongjmp to return into
the function if alarm() goes off.

The signal handler is installed before curl_jmpenv is initialized.
This means that an already installed alarm timer could trigger the
newly installed signal handler, leading to undefined behavior when it
accesses the uninitialized curl_jmpenv.

Even if there is no previously installed alarm available, the code in
Curl_resolv_timeout itself installs an alarm before the environment is
fully set up. If the process is sent into suspend right after that, the
signal handler could be called too early as in previous scenario.

To fix this, the signal handler should only be installed and the alarm
timer only be set after sigsetjmp has been called.

---
 lib/hostip.c | 53 +++++++++++++++++++++++++++--------------------------
 1 file changed, 27 insertions(+), 26 deletions(-)
diff --git a/lib/hostip.c b/lib/hostip.c
index 5783e07..d6f6b1b 100644
--- a/lib/hostip.c
+++ b/lib/hostip.c
@@ -607,32 +607,6 @@ int Curl_resolv_timeout(struct connectdata *conn,
        we want to wait less than one second we must bail out already now. */
     return CURLRESOLV_TIMEDOUT;
 
-  /*************************************************************
-   * Set signal handler to catch SIGALRM
-   * Store the old value to be able to set it back later!
-   *************************************************************/
-#ifdef HAVE_SIGACTION
-  sigaction(SIGALRM, NULL, &sigact);
-  keep_sigact = sigact;
-  keep_copysig = TRUE; /* yes, we have a copy */
-  sigact.sa_handler = alarmfunc;
-#ifdef SA_RESTART
-  /* HPUX doesn't have SA_RESTART but defaults to that behaviour! */
-  sigact.sa_flags &= ~SA_RESTART;
-#endif
-  /* now set the new struct */
-  sigaction(SIGALRM, &sigact, NULL);
-#else /* HAVE_SIGACTION */
-  /* no sigaction(), revert to the much lamer signal() */
-#ifdef HAVE_SIGNAL
-  keep_sigact = signal(SIGALRM, alarmfunc);
-#endif
-#endif /* HAVE_SIGACTION */
-
-  /* alarm() makes a signal get sent when the timeout fires off, and that
-     will abort system calls */
-  prev_alarm = alarm(curlx_sltoui(timeout/1000L));
-
   /* This allows us to time-out from the name resolver, as the timeout
      will generate a signal and we will siglongjmp() from that here.
      This technique has problems (see alarmfunc).
@@ -645,6 +619,33 @@ int Curl_resolv_timeout(struct connectdata *conn,
     rc = CURLRESOLV_ERROR;
     goto clean_up;
   }
+  else {
+    /*************************************************************
+     * Set signal handler to catch SIGALRM
+     * Store the old value to be able to set it back later!
+     *************************************************************/
+#ifdef HAVE_SIGACTION
+    sigaction(SIGALRM, NULL, &sigact);
+    keep_sigact = sigact;
+    keep_copysig = TRUE; /* yes, we have a copy */
+    sigact.sa_handler = alarmfunc;
+#ifdef SA_RESTART
+    /* HPUX doesn't have SA_RESTART but defaults to that behaviour! */
+    sigact.sa_flags &= ~SA_RESTART;
+#endif
+    /* now set the new struct */
+    sigaction(SIGALRM, &sigact, NULL);
+#else /* HAVE_SIGACTION */
+    /* no sigaction(), revert to the much lamer signal() */
+#ifdef HAVE_SIGNAL
+    keep_sigact = signal(SIGALRM, alarmfunc);
+#endif
+#endif /* HAVE_SIGACTION */
+
+    /* alarm() makes a signal get sent when the timeout fires off, and that
+       will abort system calls */
+    prev_alarm = alarm(curlx_sltoui(timeout/1000L));
+  }
 
 #else
 #ifndef CURLRES_ASYNCH
-- 
2.3.2
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette:  http://curl.haxx.se/mail/etiquette.html
Received on 2015-03-14