Hello tar maintainers, the patch below is an attempt to fix issues reported in the thread "no-overwrite-dir does not preserve metadata of existing directories" ( https://www.mail-archive.com/bug-tar@gnu.org/msg06431.html ). One issue is the non-preservation of some metadata, other issue is the inability to extract an archive which contains an empty directory that already exists and is not writable and we are not allowed to make it writable. This used to succeed, as we are not supposed to overwrite the directory, and we don't need to, as we don't need to create any files in it - but since 14d8fc718f0c872274b90991ee634b0cd8e1a6f0 it has been failing. In addition to the cases demonstrated in the thread, I found another reproducer (/mnt/qa is a filesystem mounted read-only and it contains var/): # cd # mkdir -p mnt/qa/var # tar cf qa.tar mnt # cd / # tar -x --no-overwrite-dir -f ~/qa.tar tar errors out with: tar: mnt/qa: Cannot change mode to rwx------: Read-only file system tar: mnt/qa/var: Cannot change mode to rwx------: Read-only file system tar: Exiting with failure status due to previous errors
The patch removes the ability to extract with --no-overwrite-dir when there are files in the directory to be extracted and the existing directory is not writable. I think this is acceptable, as the original bug report that led to the change was about something else and the fix for that issue ("Option "--no-overwrite-dir" does not work for empty directories") remains. See the thread for more details, discussion, and alternative approaches. The patch adds only a test for the changing of directory permission bits issue. I suppose that testing the issue with directories not being writable could be possible with AT_PRIVILEGED_PREREQ. Best regards, Pavel -- >8 -- From: Pavel Cahyna <pcah...@redhat.com> Subject: [PATCH 1/1] Don't set directory mode with --no-overwrite-dir Since 14d8fc718f0c872274b90991ee634b0cd8e1a6f0, with --no-overwrite-dir, tar attempted to temporarily change the existing target directory mode to be able to create files in it, and then restore it back in order to preserve the directory metadata. This approach created several problems: - The directory metadata were not restored accurately. The operation led to a change of mtime and ctime, and sometimes to a loss of some mode bits (they got masked by the current umask). - If it was not possible to change the target directory mode, tar errored out, even if the directory being extracted was empty and there was actually no need to create files in it. Fix by removing code that makes the existing target directory writable with --no-overwrite-dir. This reverts part of commit 14d8fc718f0c872274b90991ee634b0cd8e1a6f0 "Fix the --no-overwrite-dir option", but the fix for the original problem ("Option "--no-overwrite-dir" does not work for empty directories") is preserved. As a result, extracting a directory with files when the directory already exists and is not writable now fails with --no-overwrite-dir (just as it did before 14d8fc718f0c872274b90991ee634b0cd8e1a6f0). Ensuring that the target directory is writable is now a responsibility of the user. See the thread "no-overwrite-dir does not preserve metadata of existing directories" (https://lists.gnu.org/archive/html/bug-tar/2024-11/msg00006.html) for the original bug report, discussion and details. * src/extract.c (extract_dir): Remove setting and restoring of directory permissions in --no-overwrite-dir mode. * tests/extrac23.at: Remove the test for the now-unsupported case, add a a test case for the problem with loss of mode bits. --- src/extract.c | 25 ------------------------- tests/extrac23.at | 30 ++++++++++++++++-------------- 2 files changed, 16 insertions(+), 39 deletions(-) diff --git a/src/extract.c b/src/extract.c index 5d89458a..0e92abe0 100644 --- a/src/extract.c +++ b/src/extract.c @@ -1158,31 +1158,6 @@ extract_dir (char *file_name, char typeflag) repair_delayed_set_stat (file_name, &st); return true; } - else if (old_files_option == NO_OVERWRITE_DIR_OLD_FILES) - { - /* 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; } } diff --git a/tests/extrac23.at b/tests/extrac23.at index c8943a73..1f9ea2fc 100644 --- a/tests/extrac23.at +++ b/tests/extrac23.at @@ -21,16 +21,23 @@ AT_KEYWORDS([extract extrac23 no-overwrite-dir]) # Description: Implementation of the --no-overwrite-dir option was flawed in # tar versions up to 1.32.90. This option is intended to preserve metadata # of existing directories. In fact it worked only for non-empty directories. -# Moreover, if the actual directory was owned by the user tar runs as and the -# S_IWUSR bit was not set in its actual permissions, tar failed to create files -# in it. +# Moreover, the first fix caused some permissions (depending on the current +# umask) and modification times to get overwritten when the directory had not +# the S_IWUSR bit set in its actual permissions, and also caused failure +# to extract an empty directory if the already existing directory had not +# the S_IWUSR bit set in its actual permissions and changing the permissions +# was not possible. # # Reported by: Michael Kaufmann <m...@michael-kaufmann.ch> # References: <20200207112934.horde.anxzyhaj2chiwurw5cut...@webmail.michael-kaufmann.ch>, # https://lists.gnu.org/archive/html/bug-tar/2020-02/msg00003.html +# +# Reported by: Nate Simon <njsimo...@gmail.com> +# References: <e09b5362-548a-40b6-beec-99add9a88...@gmail.com>, +# https://lists.gnu.org/archive/html/bug-tar/2024-11/msg00006.html AT_TAR_CHECK([ -# Test if the directory permissions are restored properly. +# Test if the directory permissions are preserved properly. mkdir dir chmod 755 dir tar cf a.tar dir @@ -38,21 +45,16 @@ chmod 777 dir tar -xf a.tar --no-overwrite-dir genfile --stat=mode.777 dir -# Test if temporary permissions are set correctly to allow the owner -# to write to the directory. -genfile --file dir/file +# Test if the directory permissions are preserved even if it is not writable. +chmod 755 dir tar cf a.tar dir -rm dir/file -chmod 400 dir +chmod 444 dir +umask 066 tar -xf a.tar --no-overwrite-dir genfile --stat=mode.777 dir -chmod 700 dir -find dir ], [0], [777 -400 -dir -dir/file +444 ]) AT_CLEANUP -- 2.39.5