Race condition in creating directories

[James Abbatiello]

I believe there is a race condition when tar creates directories
during extraction of files.  If creation of a file fails with ENOENT
then tar will try to create any needed subdirectories, in case they
don't exist.  However if something else creates that subdirectory
between the attempt to create the file and the attempt to create the
directory then tar will get back EEXIST for the directory creation.
It doesn't treat this as an error but it doesn't set `*interdir_made`,
meaning that the creation of the file is not retried.  Instead tar
dies with an error message.

The following script reproduces the problem for me.

======== begin script =========
#!/bin/sh
set -e

rm -rf test.tar subdir
mkdir subdir
touch subdir/a.txt
tar cvf test.tar subdir/a.txt

while true
do
    rm -rf subdir
    mkdir -p subdir &
    tar xvf test.tar
    wait
done
========= end script =========

When I run this it will run for a few seconds and then die with:
======== begin output ========
$ ./repro.sh
...
subdir/a.txt
subdir/a.txt
subdir/a.txt
tar: subdir/a.txt: Cannot open: No such file or directory
tar: Exiting with failure status due to previous errors
======== end output ========

The error message ("No such file or directory") is somewhat confusing
in this context.  The directory "subdir" does in fact exist and tar is
attempting to create "subdir/a.txt" so it isn't exactly clear what
file or directory that tar is complaining about not existing.

Since the script relies on hitting a race condition, it may not
reproduce as quickly for you.  It may help to have other processes
running in the background to introduce some jitter into the timing and
help hit the race window.

It seems like a simple fix is to set `*interdir_made` to true and to
retry the file creation as long as the directory exists, regardless of
whether tar created the directory or whether something else created
the directory  The below patch attempts to do this and it solves the
problem for me; the above script will then run indefinitely and not
die with an error.

-- 
James Abbatiello



diff --git a/src/extract.c b/src/extract.c
index d6d98cb9..ffafdf98 100644
--- a/src/extract.c
+++ b/src/extract.c
@@ -645,9 +645,9 @@ fixup_delayed_set_stat (char const *src, char const *dst)
    it's because some required directory was not present, and if so,
    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.  */
+   */
 static int
-make_directories (char *file_name, bool *interdir_made)
+make_directories (char *file_name)
 {
   char *cursor0 = file_name + FILE_SYSTEM_PREFIX_LEN (file_name);
   char *cursor;                        /* points into the file name */
@@ -689,7 +689,6 @@ make_directories (char *file_name, bool *interdir_made)
                          desired_mode, AT_SYMLINK_NOFOLLOW);

          print_for_mkdir (file_name, cursor - file_name, desired_mode);
-         *interdir_made = true;
        }
       else if (errno == EEXIST)
        status = 0;
@@ -829,8 +828,11 @@ 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)
-       return RECOVER_OK;
+      if (make_directories (file_name) == 0)
+       {
+         *interdir_made = true;
+         return RECOVER_OK;
+       }
       break;

     default:
@@ -1920,12 +1922,11 @@ rename_directory (char *src, char *dst)
   else
     {
       int e = errno;
-      bool interdir_made;

       switch (e)
        {
        case ENOENT:
-         if (make_directories (dst, &interdir_made) == 0)
+         if (make_directories (dst) == 0)
            {
              if (renameat (chdir_fd, src, chdir_fd, dst) == 0)
                return true;