From 1a8b3bd277fd8449a72e5913d5ff9a4b7f9a315684c5c949d4539cc66b7f4521 Mon Sep 17 00:00:00 2001 From: Danilo Spinella Date: Mon, 31 Oct 2022 11:54:41 +0000 Subject: [PATCH] Accepting request 1031561 from home:dspinella:branches:Base:System - Fix unexpected inconsistency when making directory, bsc#1203600 * tar-avoid-overflow-in-symlinks-tests.patch * tar-fix-extract-unlink.patch - Update race condition fix, bsc#1200657 * tar-fix-race-condition.patch - Refresh bsc1200657.patch OBS-URL: https://build.opensuse.org/request/show/1031561 OBS-URL: https://build.opensuse.org/package/show/Base:System/tar?expand=0&rev=116 --- bsc1200657.patch | 144 ++++------------ tar-avoid-overflow-in-symlinks-tests.patch | 76 +++++++++ tar-fix-extract-unlink.patch | 187 +++++++++++++++++++++ tar-fix-race-condition.patch | 117 +++++++++++++ tar.changes | 10 ++ tar.spec | 12 +- 6 files changed, 433 insertions(+), 113 deletions(-) create mode 100644 tar-avoid-overflow-in-symlinks-tests.patch create mode 100644 tar-fix-extract-unlink.patch create mode 100644 tar-fix-race-condition.patch diff --git a/bsc1200657.patch b/bsc1200657.patch index 146fde7..0be003b 100644 --- a/bsc1200657.patch +++ b/bsc1200657.patch @@ -1,139 +1,58 @@ -From 49b9f54ff66d126c4c0d58ccfbc2b85f96f845fc Mon Sep 17 00:00:00 2001 -From: Paul Eggert -Date: Thu, 31 Mar 2022 18:26:03 -0700 -Subject: [PATCH] Retry file creation more aggressively -MIME-Version: 1.0 -Content-Type: text/plain; charset=UTF-8 -Content-Transfer-Encoding: 8bit +From 79d1ac38c19faad64f0e993180bf1ad27f217072 Mon Sep 17 00:00:00 2001 +From: James Abbatiello +Date: Fri, 10 Jun 2022 18:25:13 -0700 +Subject: tar: fix race condition -* src/extract.c (maybe_recoverable): When deciding whether to -retry file creation, don’t insist on our making intermediate -subdirectories; it’s OK if some other process made them. -Problem reported by James Abbatiello in: +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 | 2 +- - 1 file changed, 1 insertion(+), 1 deletion(-) + src/extract.c | 16 ++++++++-------- + 1 file changed, 8 insertions(+), 8 deletions(-) -Index: tar-1.34/src/extract.c -=================================================================== ---- tar-1.34.orig/src/extract.c -+++ tar-1.34/src/extract.c -@@ -645,9 +645,9 @@ fixup_delayed_set_stat (char const *src, - it's because some required directory was not present, and if so, +diff --git a/src/extract.c b/src/extract.c +index 0753dec..fda4617 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. +- 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 */ -@@ -689,7 +689,6 @@ make_directories (char *file_name, bool +@@ -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 if (errno == EEXIST) - status = 0; -@@ -829,8 +828,11 @@ maybe_recoverable (char *file_name, bool + 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 && *interdir_made) +- if (make_directories (file_name, interdir_made) == 0) - return RECOVER_OK; + if (make_directories (file_name) == 0) -+ { -+ *interdir_made = true; -+ return RECOVER_OK; -+ } ++ { ++ *interdir_made = true; ++ return RECOVER_OK; ++ } break; default: -@@ -1329,7 +1331,7 @@ extract_file (char *file_name, int typef - first. If it doesn't exist, there is no matching entry in the list. - Otherwise, look for the entry in list which has the matching dev - and ino numbers. -- -+ - This approach avoids scanning the singly-linked list in obvious cases - and does not rely on comparing file names, which may differ for - various reasons (e.g. relative vs. absolute file names). -@@ -1342,14 +1344,14 @@ find_delayed_link_source (char const *na - - if (!delayed_link_head) - return NULL; -- -+ - if (fstatat (chdir_fd, name, &st, AT_SYMLINK_NOFOLLOW)) - { - if (errno != ENOENT) - stat_error (name); - return NULL; - } -- -+ - for (dl = delayed_link_head; dl; dl = dl->next) - { - if (dl->dev == st.st_dev && dl->ino == st.st_ino) -@@ -1357,7 +1359,7 @@ find_delayed_link_source (char const *na - } - return dl; - } -- -+ - /* Create a placeholder file with name FILE_NAME, which will be - replaced after other extraction is done by a symbolic link if - IS_SYMLINK is true, and by a hard link otherwise. Set -@@ -1385,7 +1387,7 @@ create_placeholder_file (char *file_name - */ - return 0; - } -- -+ - switch (maybe_recoverable (file_name, false, interdir_made)) - { - case RECOVER_OK: -@@ -1467,7 +1469,7 @@ extract_link (char *file_name, int typef - char const *link_name; - int rc; - struct delayed_link *dl; -- -+ - link_name = current_stat_info.link_name; - - if (! absolute_names_option && contains_dot_dot (link_name)) -@@ -1475,7 +1477,7 @@ extract_link (char *file_name, int typef - dl = find_delayed_link_source (link_name); - if (dl) - return create_placeholder_file (file_name, false, &interdir_made, dl); -- -+ - do - { - struct stat st1, st2; -@@ -1697,7 +1699,7 @@ prepare_to_extract (char const *file_nam - - case GNUTYPE_VOLHDR: - return false; -- -+ - case GNUTYPE_MULTIVOL: - ERROR ((0, 0, - _("%s: Cannot extract -- file is continued from another volume"), -@@ -1753,7 +1755,7 @@ prepare_to_extract (char const *file_nam - } - } - *fun = extractor; -- -+ - return true; - } - -@@ -1934,12 +1936,11 @@ rename_directory (char *src, char *dst) +@@ -1944,12 +1945,11 @@ rename_directory (char *src, char *dst) else { int e = errno; @@ -147,3 +66,6 @@ Index: tar-1.34/src/extract.c { if (renameat (chdir_fd, src, chdir_fd, dst) == 0) return true; +-- +cgit v1.1 + diff --git a/tar-avoid-overflow-in-symlinks-tests.patch b/tar-avoid-overflow-in-symlinks-tests.patch new file mode 100644 index 0000000..ee534a9 --- /dev/null +++ b/tar-avoid-overflow-in-symlinks-tests.patch @@ -0,0 +1,76 @@ +From d935dc7d1c150b3425dd43dc13a4dd2e2b712c26 Mon Sep 17 00:00:00 2001 +From: Paul Eggert +Date: Mon, 13 Jun 2022 17:02:54 -0700 +Subject: Avoid EOVERFLOW problems in some symlink tests + +* src/extract.c (is_directory_link): New arg ST. Caller changed. +(is_directory_link, open_output_file): +Use readlinkat, not fstatat, to determine whether a string +names a symlink. This avoids EOVERFLOW issues. +(extract_dir): Avoid duplicate calls to fstatat when +keep_directory_symlink_option && fstatat_flags == 0 +and the file is a symlink to an existing file. +--- + src/extract.c | 28 ++++++++++++---------------- + 1 file changed, 12 insertions(+), 16 deletions(-) + +diff --git a/src/extract.c b/src/extract.c +index fda4617..6d2543f 100644 +--- a/src/extract.c ++++ b/src/extract.c +@@ -982,18 +982,12 @@ apply_nonancestor_delayed_set_stat (char const *file_name, bool after_links) + + + static bool +-is_directory_link (const char *file_name) ++is_directory_link (char const *file_name, struct stat *st) + { +- struct stat st; +- int e = errno; +- int res; +- +- res = (fstatat (chdir_fd, file_name, &st, AT_SYMLINK_NOFOLLOW) == 0 && +- S_ISLNK (st.st_mode) && +- fstatat (chdir_fd, file_name, &st, 0) == 0 && +- S_ISDIR (st.st_mode)); +- errno = e; +- return res; ++ char buf[1]; ++ return (0 <= readlinkat (chdir_fd, file_name, buf, sizeof buf) ++ && fstatat (chdir_fd, file_name, st, 0) == 0 ++ && S_ISDIR (st->st_mode)); + } + + /* Given struct stat of a directory (or directory member) whose ownership +@@ -1066,11 +1060,14 @@ extract_dir (char *file_name, int typeflag) + || old_files_option == OVERWRITE_OLD_FILES)) + { + struct stat st; ++ st.st_mode = 0; + +- if (keep_directory_symlink_option && is_directory_link (file_name)) ++ if (keep_directory_symlink_option ++ && is_directory_link (file_name, &st)) + return 0; + +- if (deref_stat (file_name, &st) == 0) ++ if ((st.st_mode != 0 && fstatat_flags == 0) ++ || deref_stat (file_name, &st) == 0) + { + current_mode = st.st_mode; + current_mode_mask = ALL_MODE_BITS; +@@ -1178,9 +1175,8 @@ open_output_file (char const *file_name, int typeflag, mode_t mode, + if (! HAVE_WORKING_O_NOFOLLOW + && overwriting_old_files && ! dereference_option) + { +- struct stat st; +- if (fstatat (chdir_fd, file_name, &st, AT_SYMLINK_NOFOLLOW) == 0 +- && S_ISLNK (st.st_mode)) ++ char buf[1]; ++ if (0 <= readlinkat (chdir_fd, file_name, buf, sizeof buf)) + { + errno = ELOOP; + return -1; +-- +cgit v1.1 + diff --git a/tar-fix-extract-unlink.patch b/tar-fix-extract-unlink.patch new file mode 100644 index 0000000..01630b8 --- /dev/null +++ b/tar-fix-extract-unlink.patch @@ -0,0 +1,187 @@ +From 17debecd7300e94f590b8ce167a8c0735cb6d57d Mon Sep 17 00:00:00 2001 +From: Sergey Poznyakoff +Date: Sat, 22 Oct 2022 12:06:45 +0300 +Subject: Fix savannah bug #63123 + +The bug was introduced by commit 79d1ac38c1, which didn't take into +account all the consequences of returning RECOVER_OK on EEXIST, in +particular interactions with the delayed_set_stat logic. + +The commit 79d1ac38c1 is reverted (the bug it was intended to fix +was actually fixed by 79a442d7b0). Instead: + +* src/extract.c (maybe_recoverable): Don't call maybe_recoverable +if EEXIST is reported when UNLINK_FIRST_OLD_FILES option is set. +--- + src/extract.c | 108 +++++++++++++++++++++++++++++++--------------------------- + 1 file changed, 58 insertions(+), 50 deletions(-) + +diff --git a/src/extract.c b/src/extract.c +index 78de47f..37ab295 100644 +--- a/src/extract.c ++++ b/src/extract.c +@@ -679,9 +679,10 @@ 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. */ ++ 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) ++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 */ +@@ -725,6 +726,7 @@ make_directories (char *file_name) + desired_mode, AT_SYMLINK_NOFOLLOW); + + print_for_mkdir (file_name, cursor - file_name, desired_mode); ++ *interdir_made = true; + parent_end = NULL; + } + else +@@ -879,12 +881,9 @@ maybe_recoverable (char *file_name, bool regular, bool *interdir_made) + FALLTHROUGH; + + case ENOENT: +- /* Attempt creating missing intermediate directories. */ +- if (make_directories (file_name) == 0) +- { +- *interdir_made = true; +- return RECOVER_OK; +- } ++ /* Attempt creating missing intermediate directories. */ ++ if (make_directories (file_name, interdir_made) == 0) ++ return RECOVER_OK; + break; + + default: +@@ -1072,61 +1071,69 @@ extract_dir (char *file_name, int typeflag) + break; + } + +- if (errno == EEXIST +- && (interdir_made ++ if (errno == EEXIST) ++ { ++ if (interdir_made + || keep_directory_symlink_option + || old_files_option == NO_OVERWRITE_DIR_OLD_FILES + || old_files_option == DEFAULT_OLD_FILES +- || old_files_option == OVERWRITE_OLD_FILES)) +- { +- struct stat st; +- st.st_mode = 0; +- +- if (keep_directory_symlink_option +- && is_directory_link (file_name, &st)) +- return 0; +- +- if ((st.st_mode != 0 && fstatat_flags == 0) +- || deref_stat (file_name, &st) == 0) ++ || old_files_option == OVERWRITE_OLD_FILES) + { +- current_mode = st.st_mode; +- current_mode_mask = ALL_MODE_BITS; ++ struct stat st; ++ st.st_mode = 0; ++ ++ if (keep_directory_symlink_option ++ && is_directory_link (file_name, &st)) ++ return 0; + +- if (S_ISDIR (current_mode)) ++ if ((st.st_mode != 0 && fstatat_flags == 0) ++ || deref_stat (file_name, &st) == 0) + { +- if (interdir_made) +- { +- repair_delayed_set_stat (file_name, &st); +- return 0; +- } +- else if (old_files_option == NO_OVERWRITE_DIR_OLD_FILES) ++ current_mode = st.st_mode; ++ current_mode_mask = ALL_MODE_BITS; ++ ++ if (S_ISDIR (current_mode)) + { +- /* Temporarily change the directory mode to a safe +- value, to be able to create files in it, should +- the need be. +- */ +- mode = safe_dir_mode (&st); +- status = fd_chmod(-1, file_name, mode, +- AT_SYMLINK_NOFOLLOW, DIRTYPE); +- if (status == 0) ++ if (interdir_made) + { +- /* Store the actual directory mode, to be restored +- later. +- */ +- current_stat_info.stat = st; +- current_mode = mode & ~ current_umask; +- current_mode_mask = MODE_RWX; +- atflag = AT_SYMLINK_NOFOLLOW; +- break; ++ repair_delayed_set_stat (file_name, &st); ++ return 0; + } +- else ++ else if (old_files_option == NO_OVERWRITE_DIR_OLD_FILES) + { +- chmod_error_details (file_name, mode); ++ /* Temporarily change the directory mode to a safe ++ value, to be able to create files in it, should ++ the need be. ++ */ ++ mode = safe_dir_mode (&st); ++ status = fd_chmod (-1, file_name, mode, ++ AT_SYMLINK_NOFOLLOW, DIRTYPE); ++ if (status == 0) ++ { ++ /* Store the actual directory mode, to be restored ++ later. ++ */ ++ current_stat_info.stat = st; ++ current_mode = mode & ~ current_umask; ++ current_mode_mask = MODE_RWX; ++ atflag = AT_SYMLINK_NOFOLLOW; ++ break; ++ } ++ else ++ { ++ chmod_error_details (file_name, mode); ++ } + } ++ break; + } +- break; + } + } ++ else if (old_files_option == UNLINK_FIRST_OLD_FILES) ++ { ++ status = 0; ++ break; ++ } ++ + errno = EEXIST; + } + +@@ -1978,11 +1985,12 @@ rename_directory (char *src, char *dst) + else + { + int e = errno; ++ bool interdir_made; + + switch (e) + { + case ENOENT: +- if (make_directories (dst) == 0) ++ if (make_directories (dst, &interdir_made) == 0) + { + if (renameat (chdir_fd, src, chdir_fd, dst) == 0) + return true; +-- +cgit v1.1 + diff --git a/tar-fix-race-condition.patch b/tar-fix-race-condition.patch new file mode 100644 index 0000000..28c5120 --- /dev/null +++ b/tar-fix-race-condition.patch @@ -0,0 +1,117 @@ +From 79a442d7b0e92622794bfa41dee18a28e450a0dc Mon Sep 17 00:00:00 2001 +From: Paul Eggert +Date: Thu, 9 Jun 2022 22:09:34 -0700 +Subject: 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. +--- + src/extract.c | 61 ++++++++++++++++++++++++++++++++++++++--------------------- + 2 files changed, 43 insertions(+), 23 deletions(-) + +diff --git a/src/extract.c b/src/extract.c +index e7be463..0753dec 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; + +-- +cgit v1.1 + diff --git a/tar.changes b/tar.changes index 9f21239..91b417c 100644 --- a/tar.changes +++ b/tar.changes @@ -1,3 +1,13 @@ +------------------------------------------------------------------- +Thu Oct 27 13:55:03 UTC 2022 - Danilo Spinella + +- Fix unexpected inconsistency when making directory, bsc#1203600 + * tar-avoid-overflow-in-symlinks-tests.patch + * tar-fix-extract-unlink.patch +- Update race condition fix, bsc#1200657 + * tar-fix-race-condition.patch +- Refresh bsc1200657.patch + ------------------------------------------------------------------- Sat Aug 20 06:23:22 UTC 2022 - Dirk Müller diff --git a/tar.spec b/tar.spec index c315231..8f9781c 100644 --- a/tar.spec +++ b/tar.spec @@ -39,9 +39,14 @@ Patch3: %{name}-ignore_lone_zero_blocks.patch Patch5: add_readme-tests.patch Patch6: tar-PIE.patch Patch7: tests-skip-time01-on-32bit-time_t.patch -# PATCH-FIX-OPENSUSE danilo.spinella@suse.com bsc#1200657 +# PATCH-FIX-UPSTREAM danilo.spinella@suse.com bsc#1200657 # fix race condition while creating intermediate subdirectories -Patch8: bsc1200657.patch +Patch8: tar-fix-race-condition.patch +# PATCH-FIX-UPSTREAM danilo.spinella@suse.com bsc#1203600 +# Unexpected inconsistency when making directory +Patch9: tar-avoid-overflow-in-symlinks-tests.patch +Patch10: bsc1200657.patch +Patch11: tar-fix-extract-unlink.patch BuildRequires: automake >= 1.15 BuildRequires: libacl-devel BuildRequires: libselinux-devel @@ -113,6 +118,9 @@ it may as well access remote devices or files. %patch6 -p1 %patch7 -p1 %patch8 -p1 +%patch9 -p1 +%patch10 -p1 +%patch11 -p1 %build %define my_cflags -W -Wall -Wpointer-arith -Wstrict-prototypes -Wformat-security -Wno-unused-parameter -fPIE