tar: fix race condition

Problem reported by James Abbatiello in:
https://lists.gnu.org/r/bug-tar/2022-03/msg00000.html
* src/extract.c (make_directories): Do not assume that when
mkdirat fails with errno == EEXIST that there is an existing file
that can be statted.  It could be a dangling symlink.  Instead,
wait until the end and stat it.

diff --git a/NEWS b/NEWS
index 6700c04f23090b4af744eb69e8bb333b51910a2f..94529ab16838be9937837c327d35a1216e51f495 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -1,4 +1,4 @@
-GNU tar NEWS - User visible changes. 2022-06-09
+GNU tar NEWS - User visible changes. 2022-06-10
 Please send GNU tar bug reports to <bug-tar@gnu.org>
 \f
 version 1.34.90 (git)
@@ -28,6 +28,9 @@ version 1.34.90 (git)
 ** Fix --atime-preserve=replace to not fail if there was no need to replace,
    either because we did not read the file, or the atime did not change.
 
+** Fix race when creating a parent directory while another process is
+   also doing so.
+
 ** Fix handling of prefix keywords not followed by "." in pax headers.
 
 ** Fix handling of out-of-range sparse entries in pax headers.

diff --git a/src/extract.c b/src/extract.c
index e7be463ed6a7148197c8670b9ffd4e41c3d76b1c..0753decfb203b99d627fa9f661b02b595280e579 100644 (file)
--- a/src/extract.c
+++ b/src/extract.c
@@ -636,8 +636,7 @@ fixup_delayed_set_stat (char const *src, char const *dst)
     }
 }
 
-/* After a file/link/directory creation has failed, see if
-   it's because some required directory was not present, and if so,
+/* After a file/link/directory creation has failed due to ENOENT,
    create all required directories.  Return zero if all the required
    directories were created, nonzero (issuing a diagnostic) otherwise.
    Set *INTERDIR_MADE if at least one directory was created.  */
@@ -646,6 +645,8 @@ make_directories (char *file_name, bool *interdir_made)
 {
   char *cursor0 = file_name + FILE_SYSTEM_PREFIX_LEN (file_name);
   char *cursor;                        /* points into the file name */
+  char *parent_end = NULL;
+  int parent_errno;
 
   for (cursor = cursor0; *cursor; cursor++)
     {
@@ -685,31 +686,47 @@ make_directories (char *file_name, bool *interdir_made)
 
          print_for_mkdir (file_name, cursor - file_name, desired_mode);
          *interdir_made = true;
+         parent_end = NULL;
        }
-      else if (errno == EEXIST)
-       status = 0;
       else
-       {
-         /* Check whether the desired file exists.  Even when the
-            file exists, mkdir can fail with some errno value E other
-            than EEXIST, so long as E describes an error condition
-            that also applies.  */
-         int e = errno;
-         struct stat st;
-         status = fstatat (chdir_fd, file_name, &st, 0);
-         if (status)
-           {
-             errno = e;
-             mkdir_error (file_name);
-           }
-       }
+       switch (errno)
+         {
+         case ELOOP: case ENAMETOOLONG: case ENOENT: case ENOTDIR:
+           /* FILE_NAME doesn't exist and couldn't be created; fail now.  */
+           mkdir_error (file_name);
+           *cursor = '/';
+           return status;
+
+         default:
+           /* FILE_NAME may be an existing directory so do not fail now.
+              Instead, arrange to check at loop exit, assuming this is
+              the last loop iteration.  */
+           parent_end = cursor;
+           parent_errno = errno;
+           break;
+         }
 
       *cursor = '/';
-      if (status)
-       return status;
     }
 
-  return 0;
+  if (!parent_end)
+    return 0;
+
+  /* Although we did not create the parent directory, some other
+     process may have created it, so check whether it exists now.  */
+  *parent_end = '\0';
+  struct stat st;
+  int stat_status = fstatat (chdir_fd, file_name, &st, 0);
+  if (!stat_status && !S_ISDIR (st.st_mode))
+    stat_status = -1;
+  if (stat_status)
+    {
+      errno = parent_errno;
+      mkdir_error (file_name);
+    }
+  *parent_end = '/';
+
+  return stat_status;
 }
 
 /* Return true if FILE_NAME (with status *STP, if STP) is not a
@@ -824,7 +841,7 @@ maybe_recoverable (char *file_name, bool regular, bool *interdir_made)
 
     case ENOENT:
       /* Attempt creating missing intermediate directories.  */
-      if (make_directories (file_name, interdir_made) == 0 && *interdir_made)
+      if (make_directories (file_name, interdir_made) == 0)
        return RECOVER_OK;
       break;