]> git.ipfire.org Git - thirdparty/curl.git/commitdiff
tests/server: make the signal handler signal-safe
authorViktor Szakats <commit@vsz.me>
Fri, 28 Mar 2025 00:41:28 +0000 (01:41 +0100)
committerViktor Szakats <commit@vsz.me>
Fri, 28 Mar 2025 11:02:38 +0000 (12:02 +0100)
Before this patch the signal handler called `logmsg()` which in turn
called `printf()` variants (internal implementations), and `FILE *`
functions, `localtime()`. Some of these called `malloc`/`free`, which
isn't supported in s signal handler. Replace them with `write` calls,
losing some logging functionality.

Also:
- De-dupe and move `STD*_FILENO` macros to `lib/curl_setup.h`. Revert
  the `src` definition to point to `stderr`, instead of `tool_stderr`.
  Follow-up to e5bb88b8f824ed87620bd923552534c83c2a516e #11958

POSIX specs with list of functions allowed in a signal handler:
2004: https://pubs.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_04.html#tag_02_04_03
2017: https://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_04_03
2024: https://pubs.opengroup.org/onlinepubs/9799919799/functions/V2_chap02.html#tag_16_04_03

Linux CI run with the thread sanitizer going crazy when
hitting the signal handler in test 1238 and 1242 (TFTP):
```
WARNING: ThreadSanitizer: signal-unsafe call inside of a signal (pid=12582)
    #0 malloc <null> (servers+0x5ed70)
    #1 _IO_file_doallocate <null> (libc.so.6+0x851b4)
    #2 formatf /home/runner/work/curl/curl/bld/tests/server/../../lib/../../lib/mprintf.c:886:9 (servers+0xdff77)
[...]
WARNING: ThreadSanitizer: signal-unsafe call inside of a signal (pid=12582)
    #0 free <null> (servers+0x5f453)
    #1 fclose <null> (libc.so.6+0x8532f)
    #2 logmsg /home/runner/work/curl/curl/bld/tests/server/../../../tests/server/util.c:134:5 (servers+0xe684d)
```
Ref: https://github.com/curl/curl/actions/runs/14118903372/job/39555309490?pr=16851

Closes #16852

lib/curl_setup.h
src/tool_main.h
src/tool_setup.h
tests/libtest/lib556.c
tests/server/util.c

index c8605fd200e6f7710eca6de96cc187a8a32cd5b9..ff63d3d72f142f3e3105d6dbecfc9a3c2af8ac60 100644 (file)
 #define EINVAL 22
 #define ENOSPC 28
 #define strerror(x) "?"
+#undef STDIN_FILENO
+#define STDIN_FILENO 0
 #else
 #define CURL_SETERRNO(x) (errno = (x))
 #endif
 #define S_ISDIR(m) (((m) & S_IFMT) == S_IFDIR)
 #endif
 
+/* For MSVC (all versions as of VS2022) */
+#ifndef STDIN_FILENO
+#define STDIN_FILENO  fileno(stdin)
+#endif
+#ifndef STDOUT_FILENO
+#define STDOUT_FILENO  fileno(stdout)
+#endif
+#ifndef STDERR_FILENO
+#define STDERR_FILENO  fileno(stderr)
+#endif
+
 /* Since O_BINARY is used in bitmasks, setting it to zero makes it usable in
    source code but yet it does not ruin anything */
 #ifdef O_BINARY
index c4da177e1150eb648f1fdd819a1e00496ad0adbe..95d603f4a056281832b2661dd4b2e5b4d2a39874 100644 (file)
 #define MAX_PARALLEL 300 /* conservative */
 #define PARALLEL_DEFAULT 50
 
-#ifndef STDIN_FILENO
-#  define STDIN_FILENO  fileno(stdin)
-#endif
-
-#ifndef STDOUT_FILENO
-#  define STDOUT_FILENO  fileno(stdout)
-#endif
-
-#ifndef STDERR_FILENO
-#  define STDERR_FILENO  fileno(tool_stderr)
-#endif
-
 #endif /* HEADER_CURL_TOOL_MAIN_H */
index 404b447a422452b6e3ba997d0e1d9602b47bcd91..b198d89dee1c6aecd5a4ea9352e16d4c4fd17990 100644 (file)
@@ -98,8 +98,6 @@ extern bool tool_term_has_bold;
 #  define _get_osfhandle(fd) (fd)
 #  undef _getch
 #  define _getch() 0
-#  undef STDIN_FILENO
-#  define STDIN_FILENO 0
 #endif
 
 #ifndef HAVE_FTRUNCATE
index 03c364164ab95e2828fe9c243627996122156ef2..16844973407a5ec9d4da1719de31cc30a5544fc7 100644 (file)
 #include "warnless.h"
 #include "memdebug.h"
 
-/* For Windows, mainly (may be moved in a config file?) */
-#ifndef STDIN_FILENO
-#define STDIN_FILENO 0
-#endif
-#ifndef STDOUT_FILENO
-#define STDOUT_FILENO 1
-#endif
-#ifndef STDERR_FILENO
-#define STDERR_FILENO 2
-#endif
-
 CURLcode test(char *URL)
 {
   CURLcode res;
index de25857498b34f75f6b02cd03173a77bf413955b..6fcf70579baef3d7c3dff7c4a5fad768f629848d 100644 (file)
@@ -445,10 +445,39 @@ HANDLE exit_event = NULL;
  * store in exit_signal the signal that triggered its execution.
  */
 #ifndef UNDER_CE
+/*
+ * Only call signal-safe functions from the signal handler, as required by
+ * the POSIX specification:
+ *   https://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html
+ * Hence, do not call 'logmsg()', and instead use 'open/write/close' to
+ * log errors.
+ */
 static void exit_signal_handler(int signum)
 {
   int old_errno = errno;
-  logmsg("exit_signal_handler (%d)", signum);
+  if(!serverlogfile) {
+    static const char msg[] = "exit_signal_handler: serverlogfile not set\n";
+    (void)!write(STDERR_FILENO, msg, sizeof(msg) - 1);
+  }
+  else {
+#ifdef _WIN32
+#define OPENMODE S_IREAD | S_IWRITE
+#else
+#define OPENMODE S_IRUSR | S_IWUSR
+#endif
+    int fd = open(serverlogfile, O_WRONLY|O_CREAT|O_APPEND, OPENMODE);
+    if(fd != -1) {
+      static const char msg[] = "exit_signal_handler: called\n";
+      (void)!write(fd, msg, sizeof(msg) - 1);
+      close(fd);
+    }
+    else {
+      static const char msg[] = "exit_signal_handler: failed opening ";
+      (void)!write(STDERR_FILENO, msg, sizeof(msg) - 1);
+      (void)!write(STDERR_FILENO, serverlogfile, strlen(serverlogfile));
+      (void)!write(STDERR_FILENO, "\n", 1);
+    }
+  }
   if(got_exit_signal == 0) {
     got_exit_signal = 1;
     exit_signal = signum;
@@ -525,9 +554,9 @@ static LRESULT CALLBACK main_window_proc(HWND hwnd, UINT uMsg,
   if(hwnd == hidden_main_window) {
     switch(uMsg) {
 #ifdef SIGTERM
-      case WM_CLOSE:
-        signum = SIGTERM;
-        break;
+    case WM_CLOSE:
+      signum = SIGTERM;
+      break;
 #endif
     case WM_DESTROY:
       PostQuitMessage(0);