Subject: Re: bug#18499: Possible mv race for hardlinks (rhbz #1141368 )

From: Pádraig Brady <[email-masked]>
Date: Sat, 22 Nov 2014 03:49:33 +0000

I just realised that this change avoids a more problematic issue.
The loss of files on case insensitive file systems supporting hardlinks.
Consider hfs...

  $ truncate -s1G hfs.img
  $ mkfs.hfsplus hfs.img
  $ mkdir hfs
  $ sudo mount hfs.img hfs
  $ sudo su
  # cd hfs
  # touch foo
  # ln foo blah
  # mv foo Foo  # foo is removed!
  # ls -l
  -rw-r--r--. 1 root root 0 Nov 22 02:42 blah

In the attached patch I've added a test case,
removed an old test case that pondered the issue in 5.0.91?,
updated NEWS and mentioned the issue in copy.c

cheers,
Pádraig.
From a78819af2873ea7b104366bf72af7c3e5b30782b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email-masked]>
Date: Sat, 22 Nov 2014 03:41:55 +0000
Subject: [PATCH] tests: add a case verifying mv on case insensitive file
 systems

* NEWS: Update the recent entry to also mention the avoidance
of incorrectly unlinking a multi-hardlinked "source" file when
presented with source and dest that only differ in case.
* src/copy.c (same_file_ok): Mention the case issue with same_name().
* tests/mv/hardlink-case.sh: Test the issue on HFS+.
* tests/local.mk: Reference the new test case.
* tests/mv/vfat: Remove an old related but unused test case.
---
 NEWS                      |  9 ++++---
 src/copy.c                |  1 +
 tests/local.mk            |  1 +
 tests/mv/hardlink-case.sh | 37 +++++++++++++++++++++++++++
 tests/mv/vfat             | 64 -----------------------------------------------
 5 files changed, 44 insertions(+), 68 deletions(-)
 create mode 100755 tests/mv/hardlink-case.sh
 delete mode 100644 tests/mv/vfat

diff --git a/NEWS b/NEWS
index 6a84c48..5d3bc58 100644
--- a/NEWS
+++ b/NEWS
@@ -32,10 +32,11 @@ GNU coreutils NEWS                                    -*- outline -*-
 
 ** Changes in behavior
 
-   mv no longer supports moving a file to a hardlink, as currently that
-   is only supported through unlinking the source file.  That's racy
-   in the presence of multiple mv instances, which could result in both
-   hardlinks being deleted.  This feature was added in coreutils-5.0.1.
+  mv no longer supports moving a file to a hardlink, instead issuing an error.
+  The implementation was susceptible to races in the presence of multiple mv
+  instances, which could result in both hardlinks being deleted.  Also on case
+  insensitive file systems like HFS, mv would just remove a hardlinked 'file'
+  if called like `mv file File`.  The feature was added in coreutils-5.0.1.
 
 ** Improvements
 
diff --git a/src/copy.c b/src/copy.c
index 01cee32..f316f05 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -1526,6 +1526,7 @@ same_file_ok (char const *src_name, struct stat const *src_sb,
           return true;
         }
 
+      /* FIXME: What about case insensitive file systems ?  */
       return ! same_name (src_name, dst_name);
     }
 
diff --git a/tests/local.mk b/tests/local.mk
index 22e8b86..653c984 100644
--- a/tests/local.mk
+++ b/tests/local.mk
@@ -127,6 +127,7 @@ all_root_tests =				\
   tests/misc/truncate-owned-by-other.sh		\
   tests/mkdir/writable-under-readonly.sh	\
   tests/mkdir/smack-root.sh			\
+  tests/mv/hardlink-case.sh			\
   tests/mv/sticky-to-xpart.sh			\
   tests/rm/fail-2eperm.sh			\
   tests/rm/no-give-up.sh			\
diff --git a/tests/mv/hardlink-case.sh b/tests/mv/hardlink-case.sh
new file mode 100755
index 0000000..5ab8c95
--- /dev/null
+++ b/tests/mv/hardlink-case.sh
@@ -0,0 +1,37 @@
+#!/bin/sh
+# Ensure multi-hardlinked files are not lost on case insensitive file systems
+
+# Copyright (C) 2014 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 <http://www.gnu.org/licenses/>.
+
+. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
+print_ver_ mv
+require_root_
+
+cwd=$(pwd)
+cleanup_() { cd /; umount "$cwd/mnt"; }
+
+truncate -s100M hfs.img || framework_failure_
+mkfs -t hfsplus hfs.img || skip_ 'failed to create hfs file system'
+mkdir mnt               || framework_failure_
+mount hfs.img mnt       || skip_ 'failed to mount hfs file system'
+
+cd mnt
+touch foo
+ln foo whatever
+mv foo Foo && fail=1
+test -r foo || fail=1
+
+Exit $fail
diff --git a/tests/mv/vfat b/tests/mv/vfat
deleted file mode 100644
index cd2f8b4..0000000
--- a/tests/mv/vfat
+++ /dev/null
@@ -1,64 +0,0 @@
-#!/bin/sh
-# This is just for the record.
-# This test is not run.
-
-# Copyright (C) 2003-2014 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 <http://www.gnu.org/licenses/>.
-
-exit 0
-
-cat <<\EOF
-
-Prior to 5.0.91, ...
-
-The problem:
-  On a VFAT file system with coreutils-5.0.90, 'mv FOO foo' removes the
-  sole copy of the file named by both the source and destination arguments.
-
-Demonstrate the problem, as root:
-
-cd /tmp                                    \
-  && dd if=/dev/zero of=1 bs=8192 count=50 \
-  && mkdir mnt && mkfs -t vfat 1           \
-  && mount -oloop 1 mnt && cd mnt          \
-  && printf something important > foo      \
-  && mv foo FOO
-test -f FOO && echo PASS-1 || echo FAIL-1
-ln foo bar
-mv foo FOO
-test -f FOO && echo PASS-2 || echo FAIL-2
-
-And in case you actually do the above, you can do this to clean up:
-
-  cd /tmp && umount /tmp/mnt && rm -r 1 mnt
-
-Hey!  Can't create hard links on vfat.
-The above 'ln' evokes an 'operation not permitted' failure.
-
-This demonstrates the same thing with file system type 'umsdos'
-No hard links:
-
-cd /tmp                                    \
-  && dd if=/dev/zero of=1 bs=8192 count=50 \
-  && mkdir mnt && mkfs -t msdos 1          \
-  && mount -t umsdos -oloop 1 mnt && cd mnt \
-  && printf something important > foo      \
-  && mv foo FOO
-test -f FOO && echo PASS-1 || echo FAIL-1
-ln foo bar
-mv foo FOO
-test -f FOO && echo PASS-2 || echo FAIL-2
-
-EOF
-- 
2.1.0