cp,mv,install: avoid opening non directory destination

commit v9.0-66-ge2daa8f79 introduced an issue, for example
where cp could hang when overwriting a destination fifo,
when it would try to open() the fifo on systems
like Solaris 10 that didn't support the O_DIRECTORY flag.

This is still racy on such systems, but only in the
case where a directory is replaced by a fifo in
the small window between stat() and open().

* src/system.h (target_directory_operand): On systems without
O_DIRECTORY, ensure the file is a directory before attempting to open().
* tests/cp/special-f.sh: Protect cp with timeout(1),
as cp was seen to hang when trying to overwrite an existing fifo.
* NEWS: Mention the bug fix.

diff --git a/NEWS b/NEWS
index 85fd5be122f9678b9937c633077848dcbc529319..69feffcbc13727ec2820669cec0c220768f12a63 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -21,6 +21,11 @@ GNU coreutils NEWS                                    -*- outline -*-
   and B is in some other file system.
   [bug introduced in coreutils-9.0]
 
+  cp, mv, and install, on older systems like Solaris 10 without O_DIRECTORY
+  support, avoid opening non directory destination operands.  Opening such
+  files can have side effects, like hanging with a fifo for example.
+  [bug introduced in coreutils-9.0]
+
   On macOS, fmt no longer corrupts multi-byte characters
   by misdetecting their component bytes as spaces.
   [This bug was present in "the beginning".]

diff --git a/src/system.h b/src/system.h
index 09498a172bcff805355245cc497fd04b9aeaa309..c24cb4dc393031b820611f2c1b19b083e0eb939f 100644 (file)
--- a/src/system.h
+++ b/src/system.h
@@ -136,13 +136,26 @@ target_directory_operand (char const *file)
   if (must_be_working_directory (file))
     return AT_FDCWD;
 
-  int fd = open (file, O_PATHSEARCH | O_DIRECTORY);
+  int fd = -1;
+  bool is_a_dir = false;
+  struct stat st;
+
+  /* On old systems like Solaris 10, check with stat first
+     lest we try to open a fifo for example and hang.  */
+  if (!O_DIRECTORY && stat (file, &st) == 0)
+    {
+      is_a_dir = !!S_ISDIR (st.st_mode);
+      if (! is_a_dir)
+        errno = ENOTDIR;
+    }
+
+  if (O_DIRECTORY || is_a_dir)
+    fd = open (file, O_PATHSEARCH | O_DIRECTORY);
 
   if (!O_DIRECTORY && 0 <= fd)
     {
-      /* On old systems like Solaris 10 that do not support O_DIRECTORY,
-         check by hand whether FILE is a directory.  */
-      struct stat st;
+      /* On old systems like Solaris 10 double check type,
+         to ensure we've opened a directory.  */
       int err;
       if (fstat (fd, &st) != 0 ? (err = errno, true)
           : !S_ISDIR (st.st_mode) && (err = ENOTDIR, true))

diff --git a/tests/cp/special-f.sh b/tests/cp/special-f.sh
index a3e7842fad73711424648e1467fcf4ac415e9cc9..2c7bf5ea728a691f02666ad961ebe1af029183f1 100755 (executable)
--- a/tests/cp/special-f.sh
+++ b/tests/cp/special-f.sh
@@ -24,12 +24,10 @@ mkfifo_or_skip_ fifo
 
 touch e || framework-failure
 
-
-# Without -f, expect it to fail.
-cp -R fifo e || fail=1
-
-# With -f, it must succeed.
-cp -Rf fifo e || fail=1
-test -p fifo || fail=1
+for force in '' '-f'; do
+  # Second time through will need to unlink fifo e
+  timeout 10 cp -R $force fifo e || fail=1
+  test -p fifo || fail=1
+done
 
 Exit $fail