chcon, chgrp, chmod and chown now diagnose a directory cycle

* lib/xfts.c (cycle_warning_required): New function.
* lib/xfts.h: Declare it.
* src/chown-core.c (change_file_owner): Diagnose a cycle.
* src/chmod.c (process_file): Likewise.
* src/chcon.c (process_file): Likewise.
* NEWS (Bug fixes): Mention this.

6 files changed, 51 insertions, 2 deletions

diff --git a/NEWS b/NEWS
index 24b2afa13..b13a5f21a 100644
--- a/NEWS
+++ b/NEWS
@@ -8,8 +8,9 @@ GNU coreutils NEWS                                    -*- outline -*-
   Even then, chcon may still be useful.
   [bug introduced in coreutils-8.0]
 
-  du now diagnoses an ostensible directory cycle and arranges to exit nonzero.
-  Before, it would silently ignore the offending directory and all "contents."
+  chcon, chgrp, chmod, chown and du now diagnose an ostensible directory cycle
+  and arrange to exit nonzero.  Before, they would silently ignore the
+  offending directory and all "contents."
 
   env -u A=B now fails, rather than silently adding A to the
   environment.  Likewise, printenv A=B silently ignores the invalid
diff --git a/lib/xfts.c b/lib/xfts.c
index 5994a5fef..9c46f6af5 100644
--- a/lib/xfts.c
+++ b/lib/xfts.c
@@ -61,3 +61,22 @@ xfts_open (char * const *argv, int options,
 
   return fts;
 }
+
+/* When fts_read returns FTS_DC to indicate a directory cycle,
+   it may or may not indicate a real problem.  When a program like
+   chgrp performs a recursive traversal that requires traversing
+   symbolic links, it is *not* a problem.  However, when invoked
+   with "-P -R", it deserves a warning.  The fts_options member
+   records the options that control this aspect of fts's behavior,
+   so test that.  */
+bool
+cycle_warning_required (FTS const *fts, FTSENT const *ent)
+{
+#define ISSET(Fts,Opt) ((Fts)->fts_options & (Opt))
+  /* When dereferencing no symlinks, or when dereferencing only
+     those listed on the command line and we're not processing
+     a command-line argument, then a cycle is a serious problem. */
+  return ((ISSET (fts, FTS_PHYSICAL) && !ISSET (fts, FTS_COMFOLLOW))
+          || (ISSET (fts, FTS_PHYSICAL) && ISSET (fts, FTS_COMFOLLOW)
+              && ent->fts_level != FTS_ROOTLEVEL));
+}
diff --git a/lib/xfts.h b/lib/xfts.h
index 27ddb5d48..fc3ba9010 100644
--- a/lib/xfts.h
+++ b/lib/xfts.h
@@ -1,5 +1,9 @@
+#include <stdbool.h>
 #include "fts_.h"
 
 FTS *
 xfts_open (char * const *, int options,
            int (*) (const FTSENT **, const FTSENT **));
+
+bool
+cycle_warning_required (FTS const *fts, FTSENT const *ent);
diff --git a/src/chcon.c b/src/chcon.c
index 2badefbbd..5e58cacbf 100644
--- a/src/chcon.c
+++ b/src/chcon.c
@@ -267,6 +267,14 @@ process_file (FTS *fts, FTSENT *ent)
       ok = false;
       break;
 
+    case FTS_DC:		/* directory that causes cycles */
+      if (cycle_warning_required (fts, ent))
+        {
+          emit_cycle_warning (file_full_name);
+          return false;
+        }
+      break;
+
     default:
       break;
     }
diff --git a/src/chmod.c b/src/chmod.c
index da350032f..1a0dafa70 100644
--- a/src/chmod.c
+++ b/src/chmod.c
@@ -228,6 +228,15 @@ process_file (FTS *fts, FTSENT *ent)
         error (0, 0, _("cannot operate on dangling symlink %s"),
                quote (file_full_name));
       ok = false;
+      break;
+
+    case FTS_DC:		/* directory that causes cycles */
+      if (cycle_warning_required (fts, ent))
+        {
+          emit_cycle_warning (file_full_name);
+          return false;
+        }
+      break;
 
     default:
       break;
diff --git a/src/chown-core.c b/src/chown-core.c
index e7dacf68c..eaebe60f9 100644
--- a/src/chown-core.c
+++ b/src/chown-core.c
@@ -316,6 +316,14 @@ change_file_owner (FTS *fts, FTSENT *ent,
       ok = false;
       break;
 
+    case FTS_DC:		/* directory that causes cycles */
+      if (cycle_warning_required (fts, ent))
+        {
+          emit_cycle_warning (file_full_name);
+          return false;
+        }
+      break;
+
     default:
       break;
     }