On 3/31/22 18:27, Paul Eggert wrote:
Thanks for the bug report. How about the attached patch instead? It's a
bit more conservative than what you proposed. (I haven't tested it.)
After testing it I see that patch wasn't right and that the patch you
proposed was better. However, I now see another bug in this area. So I
installed the attached three patches. The first two add a test case and
fix for the other bug; the third one is very close to your patch so I'm
giving you author credit (it's small enough to not require copyright
paperwork).
Thanks again for reporting this.
From 77ca3050c6b57010c8650bdd3473b802ccf9cf42 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Fri, 10 Jun 2022 15:12:17 -0700
Subject: [PATCH 1/3] test: new test extrac25.at
* tests/Makefile.am (TESTSUITE_AT): Add extrac25.at.
* tests/extrac25.at: New file.
* tests/testsuite.at: Include it.
---
tests/Makefile.am | 1 +
tests/extrac25.at | 46 ++++++++++++++++++++++++++++++++++++++++++++++
tests/testsuite.at | 1 +
3 files changed, 48 insertions(+)
create mode 100644 tests/extrac25.at
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 2b98af62..eff8a3bf 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -123,6 +123,7 @@ TESTSUITE_AT = \
extrac22.at\
extrac23.at\
extrac24.at\
+ extrac25.at\
filerem01.at\
filerem02.at\
dirrem01.at\
diff --git a/tests/extrac25.at b/tests/extrac25.at
new file mode 100644
index 00000000..67e7cabe
--- /dev/null
+++ b/tests/extrac25.at
@@ -0,0 +1,46 @@
+# Test suite for GNU tar. -*- autotest -*-
+# Copyright 2022 Free Software Foundation, Inc.
+#
+# This file is part of GNU tar.
+#
+# GNU tar is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# GNU tar is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+AT_SETUP([extract over parent dir that is dangling symlink])
+AT_KEYWORDS([extract extrac25 to-stdout])
+
+# Description: When extracting over pipe, only regular files should be
+# extracted. In tar 1.33 this was broken, so that members of other types
+# (in particular, directories) were extracted as usual. This test ensures
+# that this is no longer the case.
+#
+# References: https://bugs.archlinux.org/task/69373,
+# https://savannah.gnu.org/bugs/?60002
+
+AT_TAR_CHECK([
+mkdir subdir
+touch subdir/a.txt
+tar -cf test.tar subdir/a.txt
+
+rm -rf subdir
+ln -s does_not_exist subdir
+tar -xvf test.tar
+],
+[2],
+[subdir/a.txt
+],
+[tar: subdir: Cannot mkdir: File exists
+tar: subdir/a.txt: Cannot open: No such file or directory
+tar: Exiting with failure status due to previous errors
+])
+
+AT_CLEANUP
diff --git a/tests/testsuite.at b/tests/testsuite.at
index f373159e..0769e71b 100644
--- a/tests/testsuite.at
+++ b/tests/testsuite.at
@@ -345,6 +345,7 @@ m4_include([extrac21.at])
m4_include([extrac22.at])
m4_include([extrac23.at])
m4_include([extrac24.at])
+m4_include([extrac25.at])
m4_include([backup01.at])
--
2.34.1
From 7893c2fc5b4636212713ea2640b4286c3d86c909 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Thu, 9 Jun 2022 22:09:34 -0700
Subject: [PATCH 2/3] 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.
---
NEWS | 5 ++++-
src/extract.c | 61 ++++++++++++++++++++++++++++++++-------------------
2 files changed, 43 insertions(+), 23 deletions(-)
diff --git a/NEWS b/NEWS
index 6700c04f..94529ab1 100644
--- 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>
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 e7be463e..0753decf 100644
--- 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;
--
2.34.1
From 4c37a14ee932c613af8a8becdff40267001b04ad Mon Sep 17 00:00:00 2001
From: James Abbatiello <abb...@gmail.com>
Date: Fri, 10 Jun 2022 18:25:13 -0700
Subject: [PATCH 3/3] tar: fix race condition
Problem reported in:
https://lists.gnu.org/r/bug-tar/2022-03/msg00000.html
* src/extract.c (make_directories): Retry the file creation as
long as the directory exists, regardless of whether tar itself
created the directory.
Copyright-paperwork-exempt: Yes
---
src/extract.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/extract.c b/src/extract.c
index 0753decf..fda4617d 100644
--- a/src/extract.c
+++ b/src/extract.c
@@ -638,10 +638,9 @@ fixup_delayed_set_stat (char const *src, char const *dst)
/* 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. */
+ directories were created, nonzero (issuing a diagnostic) otherwise. */
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 */
@@ -685,7 +684,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;
parent_end = NULL;
}
else
@@ -841,8 +839,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)
- return RECOVER_OK;
+ if (make_directories (file_name) == 0)
+ {
+ *interdir_made = true;
+ return RECOVER_OK;
+ }
break;
default:
@@ -1944,12 +1945,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;
--
2.34.1
