Browse Source

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.
Paul Eggert 3 years ago
parent
commit
79a442d7b0
2 changed files with 43 additions and 23 deletions
  1. 4 1
      NEWS
  2. 39 22
      src/extract.c

+ 4 - 1
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>
 Please send GNU tar bug reports to <bug-tar@gnu.org>
 
 
 version 1.34.90 (git)
 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,
 ** 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.
    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 prefix keywords not followed by "." in pax headers.
 
 
 ** Fix handling of out-of-range sparse entries in pax headers.
 ** Fix handling of out-of-range sparse entries in pax headers.

+ 39 - 22
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
    create all required directories.  Return zero if all the required
    directories were created, nonzero (issuing a diagnostic) otherwise.
    directories were created, nonzero (issuing a diagnostic) otherwise.
    Set *INTERDIR_MADE if at least one directory was created.  */
    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 *cursor0 = file_name + FILE_SYSTEM_PREFIX_LEN (file_name);
   char *cursor;	        	/* points into the file name */
   char *cursor;	        	/* points into the file name */
+  char *parent_end = NULL;
+  int parent_errno;
 
 
   for (cursor = cursor0; *cursor; cursor++)
   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);
 	  print_for_mkdir (file_name, cursor - file_name, desired_mode);
 	  *interdir_made = true;
 	  *interdir_made = true;
+	  parent_end = NULL;
 	}
 	}
-      else if (errno == EEXIST)
-	status = 0;
       else
       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 = '/';
       *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
 /* 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:
     case ENOENT:
       /* Attempt creating missing intermediate directories.  */
       /* 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;
 	return RECOVER_OK;
       break;
       break;