From d1b0155d805ce51d8f155e648d1e9ad2edb95397 Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Thu, 5 Jan 2012 11:45:50 +0100 Subject: [PATCH] mv: allow moving symlink onto same-inode dest with >= 2 hard links Normally, mv detects a few subtle cases in which proceeding with a same-file rename would, with very high probability, cause data loss. Here, we have found a corner case in which one of these same-inode tests makes mv refuse to perform a useful operation. Permit that corner case. * src/copy.c (same_file_ok): Detect/exempt this case. * tests/mv/symlink-onto-hardlink: New test. * tests/Makefile.am (TESTS): Add it. * NEWS (Bug fixes): Mention it. Initially reported by: Matt McCutchen in http://bugs.gnu.org/6960. Raised again by Anders Kaseorg due to http://bugs.debian.org/654596. Improved-by: Paul Eggert. --- NEWS | 9 +++++++++ THANKS.in | 2 ++ src/copy.c | 38 ++++++++++++++++++++++++++++++++++++++ tests/Makefile.am | 1 + tests/mv/symlink-onto-hardlink | 41 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 91 insertions(+) create mode 100755 tests/mv/symlink-onto-hardlink diff --git a/NEWS b/NEWS index 2b0926f..9eebbf6 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,15 @@ GNU coreutils NEWS -*- outline -*- * Noteworthy changes in release ?.? (????-??-??) [?] +** Bug fixes + + mv now lets you move a symlink onto a same-inode destination file that + has two or more hard links. Before, it would reject that, saying that + they are the same, implicitly warning you that the move would result in + data loss. In this unusual case, when not moving the symlink onto its + referent, there is no risk of data loss, since the symlink will + typically still point to one of the hard links. + * Noteworthy changes in release 8.15 (2012-01-06) [stable] diff --git a/THANKS.in b/THANKS.in index 13c8817..904bb3e 100644 --- a/THANKS.in +++ b/THANKS.in @@ -39,6 +39,7 @@ Alexey Vyskubov alexey@pippuri.mawhrin.net Alfred M. Szmidt ams@kemisten.nu Ambrose Feinstein ambrose@google.com Amr Ali amr.ali.cc@gmail.com +Anders Kaseorg andersk@mit.edu Andi Kleen freitag@alancoxonachip.com Andre Novaes Cunha Andre.Cunha@br.global-one.net Andreas Frische andreasfrische@gmail.com @@ -384,6 +385,7 @@ Mate Wierdl mw@moni.msci.memphis.edu Matej Vela mvela@public.srce.hr Matias A. Fonzo selk@dragora.org Matt Kraai kraai@ftbfs.org +Matt McCutchen matt@mattmccutchen.net Matt Perry matt@primefactor.com Matt Pham mattvpham@gmail.com Matt Schalit mschalit@pacbell.net diff --git a/src/copy.c b/src/copy.c index 51f51be..4810de8 100644 --- a/src/copy.c +++ b/src/copy.c @@ -34,6 +34,7 @@ #include "acl.h" #include "backupfile.h" #include "buffer-lcm.h" +#include "canonicalize.h" #include "copy.h" #include "cp-hash.h" #include "extent-scan.h" @@ -1349,6 +1350,39 @@ same_file_ok (char const *src_name, struct stat const *src_sb, } } + /* At this point, it is normally an error (data loss) to move a symlink + onto its referent, but in at least one narrow case, it is not: + In move mode, when + 1) src is a symlink, + 2) dest has a link count of 2 or more and + 3) dest and the referent of src are not the same directory entry, + then it's ok, since while we'll lose one of those hard links, + src will still point to a remaining link. + Note that technically, condition #3 obviates condition #2, but we + retain the 1 < st_nlink condition because that means fewer invocations + of the more expensive #3. + + Given this, + $ touch f && ln f l && ln -s f s + $ ls -og f l s + -rw-------. 2 0 Jan 4 22:46 f + -rw-------. 2 0 Jan 4 22:46 l + lrwxrwxrwx. 1 1 Jan 4 22:46 s -> f + this must fail: mv s f + this must succeed: mv s l */ + if (x->move_mode + && S_ISLNK (src_sb->st_mode) + && 1 < dst_sb_link->st_nlink) + { + char *abs_src = canonicalize_file_name (src_name); + if (abs_src) + { + bool result = ! same_name (abs_src, dst_name); + free (abs_src); + return result; + } + } + /* It's ok to remove a destination symlink. But that works only when we unlink before opening the destination and when the source and destination files are on the same partition. */ @@ -1837,6 +1871,10 @@ copy_internal (char const *src_name, char const *dst_name, to use fts, so using alloca here will be less of a problem. */ ASSIGN_STRDUPA (dst_backup, tmp_backup); free (tmp_backup); + /* In move mode, when src_name and dst_name are on the + same partition (FIXME, and when they are non-directories), + make the operation atomic: link dest + to backup, then rename src to dest. */ if (rename (dst_name, dst_backup) != 0) { if (errno != ENOENT) diff --git a/tests/Makefile.am b/tests/Makefile.am index 8b670fc..a94aaa2 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -491,6 +491,7 @@ TESTS = \ mv/part-symlink \ mv/partition-perm \ mv/perm-1 \ + mv/symlink-onto-hardlink \ mv/to-symlink \ mv/trailing-slash \ mv/update \ diff --git a/tests/mv/symlink-onto-hardlink b/tests/mv/symlink-onto-hardlink new file mode 100755 index 0000000..2dac484 --- /dev/null +++ b/tests/mv/symlink-onto-hardlink @@ -0,0 +1,41 @@ +#!/bin/sh +# Ensure that mv works with a few symlink-onto-hard-link cases. + +# Copyright (C) 2012 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=.}/init.sh"; path_prepend_ ../src +print_ver_ mv + +touch f || framework_failure_ +ln f h || framework_failure_ +ln -s f s || framework_failure_ + +# Given two links f and h to some important content, and a symlink s to f, +# "mv s f" must fail because it might then be hard to find the link, h. +# "mv s l" may succeed because then, s (now "l") still points to f. +# Of course, if the symlink were being moved into a different destination +# directory, things would be very different, and, I suspect, implausible. + +echo "mv: 's' and 'f' are the same file" > exp || framework_failure_ +mv s f > out 2> err && fail=1 +compare /dev/null out || fail=1 +compare exp err || fail=1 + +mv s l > out 2> err || fail=1 +compare /dev/null out || fail=1 +compare /dev/null err || fail=1 + +Exit $fail -- 2.7.4