From b4bc8f361b24536734cc6f85b070d718bffe11a3 Mon Sep 17 00:00:00 2001 From: Mateusz Nosek Date: Thu, 16 Jul 2026 12:04:57 +0100 Subject: [PATCH] install: avoid redundant metadata updates with -C "install -C" (--compare) has always called chown/chmod on the destination even after determining that copying was unnecessary, ever since --compare was added in commit v7.0-182-gdac5f12c6. need_copy() already verifies that the destination's mode, owner, and group match (and, with --preserve-context, that the SELinux context matches too) before skipping the copy, so re-applying them is redundant, and can needlessly fail on a destination that cannot be chown/chmod'd even though nothing would actually change, e.g. one marked immutable with chattr +i. * src/install.c (install_file_in_file): Skip metadata updates if we already skipped the data, as previous checks ensure that the metadata is already as required. * tests/install/install-C-immutable.sh: Add a test. (based on tests/rm/empty-immutable-skip.sh). * tests/local.mk: Reference the new test. * NEWS: Mention the improvement. --- NEWS | 3 ++ src/install.c | 4 +- tests/install/install-C-immutable.sh | 58 ++++++++++++++++++++++++++++ tests/local.mk | 1 + 4 files changed, 65 insertions(+), 1 deletion(-) create mode 100755 tests/install/install-C-immutable.sh diff --git a/NEWS b/NEWS index 191d2ec63c..501f813b55 100644 --- a/NEWS +++ b/NEWS @@ -67,6 +67,9 @@ GNU coreutils NEWS -*- outline -*- 'df', 'du', 'ls', 'od', 'pr', and 'sort' now escape invalid arguments in error messages for options expecting an integer. + 'install -C' will now avoid updating file metadata when the destination + already has the appropriate ownership and permissions. + 'sort' will now better use available memory and parallel operation when reading from unknown sized inputs like pipes. diff --git a/src/install.c b/src/install.c index bccfe606fa..1eae1a5fff 100644 --- a/src/install.c +++ b/src/install.c @@ -737,7 +737,9 @@ install_file_in_file (char const *from, char const *to, || ! S_ISREG (from_sb.st_mode)) && ! change_timestamps (&from_sb, to, to_dirfd, to_relname)) return false; - return change_attributes (to, to_dirfd, to_relname); + + return copy_status == COPY_SKIPPED + || change_attributes (to, to_dirfd, to_relname); } /* Create any missing parent directories of TO, diff --git a/tests/install/install-C-immutable.sh b/tests/install/install-C-immutable.sh new file mode 100755 index 0000000000..3b0cbf4d28 --- /dev/null +++ b/tests/install/install-C-immutable.sh @@ -0,0 +1,58 @@ +#!/bin/sh +# Ensure "install -C" does not attempt to chown/chmod an unchanged, +# immutable destination file. +# Requires root access to do chattr +i, as well as a file system +# that supports the immutable attribute (e.g., ext2/3/4, xfs, btrfs). + +# Copyright (C) 2026 Free Software Foundation, Inc. + +# This program 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. + +# This program 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 . + +. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src +print_ver_ ginstall +require_root_ + +# These simple one-file operations are expected to work even in the +# presence of this bug, and we need them to set up the rest of the test. +chattr_i_works=1 +touch f || framework_failure_ +chattr +i f 2>/dev/null || chattr_i_works=0 +rm f 2>/dev/null +test -f f || chattr_i_works=0 +chattr -i f 2>/dev/null || chattr_i_works=0 +rm f 2>/dev/null || chattr_i_works=0 +test -f f && chattr_i_works=0 + +if test $chattr_i_works = 0; then + skip_ "chattr +i doesn't work on this file system" +fi + +u1=1 +g1=1 + +echo test > a || framework_failure_ + +# Install once to create the destination, then make it immutable. +ginstall -Cv -m0644 -o$u1 -g$g1 a b || framework_failure_ +chattr +i b || framework_failure_ + +# A second "-C" install with an identical source, mode, owner and group +# must be a no-op: it must not attempt to chown/chmod the destination, +# since that would fail on an immutable file even though nothing about +# it would actually change. +ginstall -Cv -m0644 -o$u1 -g$g1 a b || fail=1 + +chattr -i b + +Exit $fail diff --git a/tests/local.mk b/tests/local.mk index e2afbe134b..18bf798618 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -121,6 +121,7 @@ all_root_tests = \ tests/du/bind-mount-dir-cycle.sh \ tests/du/bind-mount-dir-cycle-v2.sh \ tests/id/setgid.sh \ + tests/install/install-C-immutable.sh \ tests/install/install-C-root.sh \ tests/ls/capability.sh \ tests/ls/no-cap.sh \ -- 2.47.3