bug-coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

'sort' race condition with atexit cleanup and signals


From: Paul Eggert
Subject: 'sort' race condition with atexit cleanup and signals
Date: Fri, 19 Jan 2007 12:25:13 -0800
User-agent: Gnus/5.1008 (Gnus v5.10.8) Emacs/21.4 (gnu/linux)

Here's a patch to fix a race condition that Dan Hipschman and I
thought of while walking across the courtyard to Boelter Hall at UCLA.
The problem is that a signal can come in while cleanup is running
during a premature exit (e.g., due to an I/O error), causing 'sort' to
unlink a file that has already been unlinked.  This might in theory
cause 'sort' to unlink some other process's temp file.

Dan wrote the initial version of the patch and I tweaked it a bit.  I
thought it a bit cleaner to have a single cleanup function than
multiple calls to atexit, since we should not invoke some of this
stuff until the signal mask is known.

2007-01-19  Dan Hipschman  <address@hidden>
        and Paul Eggert  <address@hidden>

        * src/sort.c (cleanup): Clear temphead at the end.
        (exit_cleanup): New function.
        (main): Don't invoke atexit until we're ready.
        Invoke it with exit_cleanup, not with cleanup and close_stdout,
        to avoid a race condition with cleanup and signal handling.

diff --git a/src/sort.c b/src/sort.c
index f03237c..326866f 100644
--- a/src/sort.c
+++ b/src/sort.c
@@ -417,6 +417,25 @@ cleanup (void)

   for (node = temphead; node; node = node->next)
     unlink (node->name);
+  temphead = NULL;
+}
+
+/* Cleanup actions to take when exiting.  */
+
+static void
+exit_cleanup (void)
+{
+  if (temphead)
+    {
+      /* Clean up any remaining temporary files in a critical section so
+        that a signal handler does not try to clean them too.  */
+      sigset_t oldset;
+      sigprocmask (SIG_BLOCK, &caught_signals, &oldset);
+      cleanup ();
+      sigprocmask (SIG_SETMASK, &oldset, NULL);
+    }
+
+  close_stdout ();
 }

 /* Create a new temporary file, returning its newly allocated name.
@@ -2302,10 +2321,7 @@ main (int argc, char **argv)
   bindtextdomain (PACKAGE, LOCALEDIR);
   textdomain (PACKAGE);

-  atexit (cleanup);
-
   initialize_exit_failure (SORT_FAILURE);
-  atexit (close_stdout);

   hard_LC_COLLATE = hard_locale (LC_COLLATE);
 #if HAVE_NL_LANGINFO
@@ -2365,6 +2381,9 @@ main (int argc, char **argv)
 #endif
   }

+  /* The signal mask is known, so it is safe to invoke exit_cleanup.  */
+  atexit (exit_cleanup);
+
   gkey.sword = gkey.eword = SIZE_MAX;
   gkey.ignore = NULL;
   gkey.translate = NULL;
M ChangeLog
M src/sort.c
Committed as 600ef4a9207574c4459963a7948faa67de142e9c




reply via email to

[Prev in Thread] Current Thread [Next in Thread]