Re: "chmod a+rwx A B" doesn't chmod A before chmoding B

[Jim Meyering]

Paul Eggert <address@hidden> wrote:
> While testing the new mkdir -p stuff I noticed a bug in chmod:
> it doesn't operate left-to-right as tradition and POSIX require:
>
>   $ mkdir d d/e
>   $ chmod 0 d/e d
>   $ chmod a+rwx d d/e
>   chmod: cannot access `d/e': Permission denied
>
> The last chmod should succeed, but it fails.

Hi Paul!

Thanks for finding and patching that!

> I installed the following patch to fix this and test for the
> bug.
>
> I suppose another option would be to modify fts so that it
> has a new flag saying "please do everything left-to-right",
> but I suspect that'd be more complicated.

No need to modify fts.
That is the default already, when fts_open's comparison function
is NULL.  The problem is that fts_open traverses the list of file
names once initially, trying to stat each of them.  So when an
initially unstat'able file becomes stat'able, as in this case,
we just have to be sure not to trust the stale FTS_NS indicator.

I've reverted your changes to chmod.c and chown-core.c, and then
made the less-invasive change below.

Regarding your conceptually separate change that adds the check
for fts_close failure, you're welcome to add it back, especially
if you can come up with a test case that triggers it.  Of course,
you know that it will be triggered only on a system with neither
openat nor /proc support.

Jim

2006-09-20  Jim Meyering  <address@hidden>

        Fix the 2006-09-18 bug differently.
        * src/chmod.c: (process_file): Upon FTS_NS for a top-level file,
        tell fts_read to stat the file again, in case it has become
        accessible since the initial fts_open call.
        * src/chown-core.c (change_file_owner): Likewise.

        * src/chmod.c: Revert last change.  There is a better way.
        * src/chown-core.c: Likewise.

Index: src/chmod.c
===================================================================
RCS file: /fetish/cu/src/chmod.c,v
retrieving revision 1.120
diff -u -r1.120 chmod.c
--- src/chmod.c 20 Sep 2006 11:26:18 -0000      1.120
+++ src/chmod.c 20 Sep 2006 11:27:01 -0000
@@ -193,6 +193,19 @@
       return true;

     case FTS_NS:
+      /* For a top-level file or directory, this FTS_NS (stat failed)
+        indicator is determined at the time of the initial fts_open call.
+        With programs like chmod, chown, and chgrp, that modify
+        permissions, it is possible that the file in question is
+        accessible when control reaches this point.  So, if this is
+        the first time we've seen the FTS_NS for this file, tell
+        fts_read to stat it "again".  */
+      if (ent->fts_level == 0 && ent->fts_number == 0)
+       {
+         ent->fts_number = 1;
+         fts_set (fts, ent, FTS_AGAIN);
+         return true;
+       }
       error (0, ent->fts_errno, _("cannot access %s"), quote (file_full_name));
       ok = false;
       break;
Index: src/chown-core.c
===================================================================
RCS file: /fetish/cu/src/chown-core.c,v
retrieving revision 1.42
diff -u -r1.42 chown-core.c
--- src/chown-core.c    20 Sep 2006 11:26:18 -0000      1.42
+++ src/chown-core.c    20 Sep 2006 11:27:16 -0000
@@ -267,6 +267,19 @@
       break;

     case FTS_NS:
+      /* For a top-level file or directory, this FTS_NS (stat failed)
+        indicator is determined at the time of the initial fts_open call.
+        With programs like chmod, chown, and chgrp, that modify
+        permissions, it is possible that the file in question is
+        accessible when control reaches this point.  So, if this is
+        the first time we've seen the FTS_NS for this file, tell
+        fts_read to stat it "again".  */
+      if (ent->fts_level == 0 && ent->fts_number == 0)
+       {
+         ent->fts_number = 1;
+         fts_set (fts, ent, FTS_AGAIN);
+         return true;
+       }
       error (0, ent->fts_errno, _("cannot access %s"), quote (file_full_name));
       ok = false;
       break;