From df3e53f003533feb9a4ae1daa01ad1a7cd9e0c96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?=
Date: Mon, 14 May 2018 02:26:05 -0700 Subject: [PATCH] cp: fix symlink checks when overwriting files Ensure this _does_ recreate the symlink Given "path1" and "path2" are on different devices. $ touch "path1/file" $ cd path2/; ln -s path1/file $ cp -sf path1/file . Ensure this does _not_ overwrite file $ touch file $ ln -s file l1 $ cp -sf l1 file * src/copy.c (same_file_ok): Remove device ids from consideration, instead deferring to future EXDEV with --link or allowing the first case above to work. Also ensure that we do not exist this function too early, when the destination file is not a symlink, which protects against the second case. * tests/cp/cross-dev-symlink.sh: Add a test for the first case. * tests/cp/same-file.sh: Add a test for the second case above. * NEWS: Mention the bug fixes. * THANKS.in: Mention the reporters who also analyzed the code. Fixes https://bugs.gnu.org/31364 --- NEWS | 7 +++++++ THANKS.in | 2 ++ src/copy.c | 18 ++++++++---------- tests/cp/cross-dev-symlink.sh | 38 ++++++++++++++++++++++++++++++++++++++ tests/cp/same-file.sh | 15 ++++++++++++++- tests/local.mk | 1 + 6 files changed, 70 insertions(+), 11 deletions(-) create mode 100755 tests/cp/cross-dev-symlink.sh diff --git a/NEWS b/NEWS index de02814..7aa2925 100644 --- a/NEWS +++ b/NEWS @@ -4,6 +4,13 @@ GNU coreutils NEWS -*- outline -*- ** Bug fixes + 'cp --symlink SRC DST' will again correctly validate DST. + If DST is a regular file and SRC is a symlink to DST, + then cp will no longer allow that operation to clobber DST. + Also if DST is a symlink, then it can always be replaced, + even if it points to SRC on a separate device. + [bug introduced with coreutils-8.27] + 'ls -aA' is now equivalent to 'ls -A', since -A now overrides -a. [bug introduced in coreutils-5.3.0] diff --git a/THANKS.in b/THANKS.in index c63cc9b..3951b66 100644 --- a/THANKS.in +++ b/THANKS.in @@ -261,6 +261,7 @@ Ian Kent address@hidden Ian Lance Taylor address@hidden Ian Turner address@hidden Iida Yosiaki address@hidden +Illia Bobyr address@hidden Ilya N. Golubev address@hidden Ingo Saitz address@hidden Ivan Labath address@hidden @@ -285,6 +286,7 @@ Jan-Pawel Wrozstinski address@hidden Jari Aalto address@hidden Jarkko Hietaniemi address@hidden Jarod Wilson address@hidden +Jason Smith address@hidden Jean Charles Delepine address@hidden Jean-Pierre Tosoni address@hidden Jeff Moore address@hidden diff --git a/src/copy.c b/src/copy.c index 4998c83..0407c56 100644 --- a/src/copy.c +++ b/src/copy.c @@ -1627,14 +1627,9 @@ same_file_ok (char const *src_name, struct stat const *src_sb, } } - /* It's ok to remove a destination symlink. But that works only - when creating symbolic links, or when the source and destination - are on the same file system and when creating hard links or when - unlinking before opening the destination. */ - if (x->symbolic_link - || ((x->hard_link || x->unlink_dest_before_opening) - && S_ISLNK (dst_sb_link->st_mode))) - return dst_sb_link->st_dev == src_sb_link->st_dev; + /* It's ok to recreate a destination symlink. */ + if (x->symbolic_link && S_ISLNK (dst_sb_link->st_mode)) + return true; if (x->dereference == DEREF_NEVER) { @@ -1651,10 +1646,13 @@ same_file_ok (char const *src_name, struct stat const *src_sb, if ( ! SAME_INODE (tmp_src_sb, tmp_dst_sb)) return true; - /* FIXME: shouldn't this be testing whether we're making symlinks? */ if (x->hard_link) { - *return_now = true; + /* It's ok to attempt to hardlink the same file, + and return early if not replacing a symlink. + Note we need to return early to avoid a later + unlink() of DST (when SRC is a symlink). */ + *return_now = ! S_ISLNK (dst_sb_link->st_mode); return true; } } diff --git a/tests/cp/cross-dev-symlink.sh b/tests/cp/cross-dev-symlink.sh new file mode 100755 index 0000000..e945b40 --- /dev/null +++ b/tests/cp/cross-dev-symlink.sh @@ -0,0 +1,38 @@ +#!/bin/sh +# Ensure symlinks can be replaced across devices + +# Copyright (C) 2018 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