On 02/06/17 09:07, Jim Minter wrote:
> Hello,
> 
> A behaviour that surprised me with mv, moving files to an NFS-mounted 
> filesystem with all_squash set (in a Vagrant box running Fedora 25, 
> coreutils 8.25).
> 
> (/vagrant is NFS-mounted as described in the example below).
> 
> ==== 8< ====
> [vagrant@default ~]$ sudo -i
> [root@default ~]# ln -s thing symlink
> [root@default /]# mv symlink /vagrant
> mv: failed to preserve ownership for /vagrant/symlink: Operation not 
> permitted
> [root@default /]# echo $?
> 1
> [root@default /]# ls . /vagrant
> .:
> symlink
> 
> /vagrant:
> symlink
> ==== 8< ====
> 
> The surprise to me is that mv exits early in this case (after copying 
> the symlink across volumes, before removing the source symlink).
> 
> Doing the same operation as root but with a regular file, I get the same 
> warning message, but the move completes as expected, and mv returns exit 
> status 0.  I had expected the same behaviour when moving a symlink.
> 
> (As non-root, mv works as expected both for a file and for a symlink - 
> object moved, chown syscall fails, no warning message).
> 
> The knock-on effect I've seen which is particularly problematic, is that 
> this issue causes mv to fail when moving an entire directory tree as 
> root to an NFS-mounted filesystem, if that directory tree contains a 
> symlink anywhere within it.  Can the exit(1) in this case be considered 
> a bug?

That indeed looks like a bug that has been present since the initial 
implementation!
The attached should fix it up.

thanks!
Pádraig

>From f1de9740e663181ef6a303118364545ced043c13 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email protected]>
Date: Fri, 2 Jun 2017 21:50:04 -0700
Subject: [PATCH] copy: don't fail when unable to chown symlinks

* src/copy.c (copy_internal): Honor the x->require_preserve flag
for symlinks as we do for ordinary files, so we don't exit with
failure upon failure to chown a symbolic link.
* NEWS: Mention the bug fix.
---
 NEWS       | 5 +++++
 src/copy.c | 3 ++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/NEWS b/NEWS
index 7c4429d..c811dbc 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,11 @@ GNU coreutils NEWS                                    -*- outline -*-
 
 ** Bug fixes
 
+  cp and mv now only warn about failure to preserve ownership of symlinks.
+  cp (without -p) will no longer exit with a failure status, and mv will
+  also no longer leave such symlinks remaining in the source file system.
+  [the bug dates back to the initial implementation]
+
   date and touch no longer overwrite the heap with large
   user specified TZ values (CVE-2017-7476).
   [bug introduced in coreutils-8.27]
diff --git a/src/copy.c b/src/copy.c
index e96ecdd..965e819 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -2714,7 +2714,8 @@ copy_internal (char const *src_name, char const *dst_name,
             {
               error (0, errno, _("failed to preserve ownership for %s"),
                      dst_name);
-              goto un_backup;
+              if (x->require_preserve)
+                goto un_backup;
             }
           else
             {
-- 
2.9.3

Reply via email to