bug-coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH 1/2] chcon, chgrp, chmod and chown now diagnose a directory cycle


From: Jim Meyering
Subject: [PATCH 1/2] chcon, chgrp, chmod and chown now diagnose a directory cycle
Date: Sat, 07 Nov 2009 08:46:10 +0100

FYI, now chcon, chgrp, chmod and chown diagnose directory cycles, too:
The other fts clients, du and rm, did already.

>From d9dbbb9a455f6bfc4e09d9f5f6c6c633f1b03c52 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Sat, 7 Nov 2009 08:09:12 +0100
Subject: [PATCH 1/2] 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.
---
 NEWS             |    5 +++--
 lib/xfts.c       |   19 +++++++++++++++++++
 lib/xfts.h       |    4 ++++
 src/chcon.c      |    8 ++++++++
 src/chmod.c      |    9 +++++++++
 src/chown-core.c |    8 ++++++++
 6 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index 24b2afa..b13a5f2 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 5994a5f..9c46f6a 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 27ddb5d..fc3ba90 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 2badefb..5e58cac 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 da35003..1a0dafa 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 e7dacf6..eaebe60 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;
     }
--
1.6.5.2.303.g13162


>From 9a8d8f46a541d333f98dca26d053d1f5bd4424bb Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Sat, 7 Nov 2009 08:17:28 +0100
Subject: [PATCH 2/2] maint: make du's cycle-detection code consistent

* src/du.c (process_file): Revert the du.c-changing part of
commit 8ba5d1a7. Use cycle_warning_required instead.
---
 src/du.c |   17 ++++++-----------
 1 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/src/du.c b/src/du.c
index c33bbb7..bee006d 100644
--- a/src/du.c
+++ b/src/du.c
@@ -65,10 +65,6 @@ extern bool fts_debug;
 /* Initial size of the hash table.  */
 #define INITIAL_TABLE_SIZE 103

-/* Select one of the three FTS_ options that control if/when
-     to follow a symlink.  */
-static int symlink_deref_bits = FTS_PHYSICAL;
-
 /* Hash structure for inode and device numbers.  The separate entry
    structure makes it easier to rehash "in place".  */
 struct entry
@@ -498,14 +494,9 @@ process_file (FTS *fts, FTSENT *ent)
       break;

     case FTS_DC:               /* directory that causes cycles */
-      /* 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. */
-      if (symlink_deref_bits == FTS_PHYSICAL
-          || (symlink_deref_bits == (FTS_COMFOLLOW | FTS_PHYSICAL)
-              && ent->fts_level != FTS_ROOTLEVEL))
+      if (cycle_warning_required (fts, ent))
         {
-          emit_cycle_warning (ent->fts_path);
+          emit_cycle_warning (file);
           return false;
         }
       ok = true;
@@ -677,6 +668,10 @@ main (int argc, char **argv)
   /* Bit flags that control how fts works.  */
   int bit_flags = FTS_TIGHT_CYCLE_CHECK | FTS_DEFER_STAT;

+  /* Select one of the three FTS_ options that control if/when
+     to follow a symlink.  */
+  int symlink_deref_bits = FTS_PHYSICAL;
+
   /* If true, display only a total for each argument. */
   bool opt_summarize_only = false;

--
1.6.5.2.303.g13162




reply via email to

[Prev in Thread] Current Thread [Next in Thread]