On 19/02/18 02:22, Declercq Laurent wrote:
> I think that I did found at least two bugs in cp(1) command when the 
> --no-preserve=mode option is involved and when copying special file. I 
> describe each of them below.
> 
> 1. Mode set on special files seem to be wrong:
> 
> Original file to copy: prw-rw-rw- 1 root staff 0 févr. 18 18:59 spfile
> cp(1) command (run as root user): cp -r --no-preserve=mode spfile 
> spfile_copy
> 
> Current result:
> 
> prwxr-xr-x 1 root staff 0 févr. 18 22:01 spfile_copy
> 
> Expected result (considering UMASK 0022):
> 
> prw-r--r-- 1 root staff 0 févr. 18 22:01 spfile_copy
> 
> The current behavior is due to the fact that mode used is 0777 while 
> 0666 should be used for files.
> 
> Possible fix: Differentiate directories from files in the copy_internal 
> function.

Thanks for the clear details.
The attached should fix this up.

> 2. Non-permission bits are preserved, even when the --no-preserve=mode 
> option is involved.
> 
> Original file to copy: prwSrw-rw- 1 root staff 0 févr. 18 18:59 spfile
> cp(1) command (run as root user): cp -r --no-preserve=mode spfile 
> spfile_copy
> 
> Current result:
> 
> prwsr-xr-x 1 root staff 0 févr. 18 22:05 spfile_copy

I'm not seeing this. I get 'x' rather than 's' here (and '-' with the fix)

> Expected result (considering UMASK 0022 and without the first bug above):
> 
> prw-r--r-- 1 root staff 0 févr. 18 22:05 spfile_copy

thanks!
Pádraig

From 71e05111fa9dd6a4ad29630752d95289cb6d0274 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com>
Date: Mon, 19 Feb 2018 19:10:14 -0800
Subject: [PATCH] cp: set appropriate default permissions for special files

This issue was introduced in commit v8.19-145-g24ebca6

* src/copy.c (copy_internal): When setting default permissions
to use with --no-preserve=mode, only set executable bits for
directories or sockets.
* NEWS: Mention the fix.
* tests/cp/preserve-mode.sh: Add a test case.
Fixes https://bugs.gnu.org/30534
---
 NEWS                      |  5 +++++
 src/copy.c                |  6 ++++--
 tests/cp/preserve-mode.sh | 10 +++++++++-
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/NEWS b/NEWS
index 8a9e09e..5fa6928 100644
--- a/NEWS
+++ b/NEWS
@@ -16,6 +16,11 @@ GNU coreutils NEWS                                    -*- outline -*-
   that caused -u to sometimes override -n.
   [bug introduced with coreutils-7.1]
 
+  'cp -a --no-preserve=mode' now sets appropriate default permissions
+  for non regular files like fifos and character device nodes etc.
+  Previously it would have set executable bits on created special files.
+  [bug introduced with coreutils-8.20]
+
 
 * Noteworthy changes in release 8.29 (2017-12-27) [stable]
 
diff --git a/src/copy.c b/src/copy.c
index e050d41..233b498 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -1378,7 +1378,7 @@ preserve_metadata:
     }
   else if (x->explicit_no_preserve_mode)
     {
-      if (set_acl (dst_name, dest_desc, 0666 & ~cached_umask ()) != 0)
+      if (set_acl (dst_name, dest_desc, MODE_RW_UGO & ~cached_umask ()) != 0)
         return_val = false;
     }
   else if (omitted_permissions)
@@ -2860,7 +2860,9 @@ copy_internal (char const *src_name, char const *dst_name,
     }
   else if (x->explicit_no_preserve_mode)
     {
-      if (set_acl (dst_name, -1, 0777 & ~cached_umask ()) != 0)
+      int default_permissions = S_ISDIR (src_mode) || S_ISSOCK (src_mode)
+                                ? S_IRWXUGO : MODE_RW_UGO;
+      if (set_acl (dst_name, -1, default_permissions & ~cached_umask ()) != 0)
         return false;
     }
   else
diff --git a/tests/cp/preserve-mode.sh b/tests/cp/preserve-mode.sh
index 1cd173a..3b0aca8 100755
--- a/tests/cp/preserve-mode.sh
+++ b/tests/cp/preserve-mode.sh
@@ -19,7 +19,7 @@
 . "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
 print_ver_ cp
 
-get_mode() { ls -ld "$1" | cut -b-10; }
+get_mode() { stat -c%f "$1"; }
 
 rm -f a b c
 umask 0022
@@ -47,4 +47,12 @@ chmod 600 a
 cp --no-preserve=mode --preserve=all a b || fail=1
 test "$(get_mode a)" = "$(get_mode b)" || fail=1
 
+#fifo test
+if mkfifo fifo; then
+  cp -a --no-preserve=mode fifo fifo_copy || fail=1
+  #ensure default perms set appropriate for non regular files
+  #which wasn't done between v8.20 and 8.29 inclusive
+  test "$(get_mode fifo)" = "$(get_mode fifo_copy)" || fail=1
+fi
+
 Exit $fail
-- 
2.9.3

Reply via email to