Subject: Re: bug#17035: [PATCH] chmod -c -R produces errors with special permissions

From: Pádraig Brady <[email-masked]>
Date: Tue, 18 Mar 2014 18:45:29 +0000

On 03/18/2014 04:06 PM, Dylan Alex Simon wrote:
> chmod sometimes tries to stat files in the wrong directory when reporting
> changes:
> 
>> mkdir -p a/b
>> chmod -R g+s a/b # requires some special permission bit set first
>> chmod -c -R g+w a
> chmod: getting new attributes of 'b': No such file or directory
> 
> A fix is attached.
> 

The fix looks good.  It seems this bug was introduced
with the change to FTS in 5.1.0 (released Dec 2003)
I'll push soon in your name with these changes merged in.

thanks!
Pádraig.

$ git diff HEAD@{1}

diff --git a/NEWS b/NEWS
index 35d48e5..3623674 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,9 @@ GNU coreutils NEWS                                    -*- outline -*-

 ** Bug fixes

+  chmod -R --changes no longer issues erroneous warnings for files with special
+  bits set.  [bug introduced in coreutils-5.1.0]
+
   cp -a, mv, and install --preserve-context, once again set the correct SELinux
   context for existing directories in the destination.  Previously they set
   the context of an existing directory to that of its last copied descendent.
diff --git a/src/chmod.c b/src/chmod.c
index f9debde..ae8b6fb 100644
--- a/src/chmod.c
+++ b/src/chmod.c
@@ -111,7 +111,7 @@ static struct option const long_options[] =
    The old mode was OLD_MODE, but it was changed to NEW_MODE.  */

 static bool
-mode_changed (int dirfd, char const *file, char const *file_full_name,
+mode_changed (int dir_fd, char const *file, char const *file_full_name,
               mode_t old_mode, mode_t new_mode)
 {
   if (new_mode & (S_ISUID | S_ISGID | S_ISVTX))
@@ -121,7 +121,7 @@ mode_changed (int dirfd, char const *file, char const *file_full_name,

       struct stat new_stats;

-      if (fstatat (dirfd, file, &new_stats, 0) != 0)
+      if (fstatat (dir_fd, file, &new_stats, 0) != 0)
         {
           if (! force_silent)
             error (0, errno, _("getting new attributes of %s"), quote (file_full_name));
diff --git a/tests/chmod/c-option.sh b/tests/chmod/c-option.sh
index 1dd9b9e..38b127b 100755
--- a/tests/chmod/c-option.sh
+++ b/tests/chmod/c-option.sh
@@ -37,4 +37,15 @@ case "$(cat out)" in
   *) cat out; fail=1 ;;
 esac

+# From V5.1.0 to 8.22 this would stat the wrong file and
+# give an erroneous ENOENT diagnostic
+mkdir -p a/b || framework_failure_
+# chmod g+s might fail as detailed in setgid.sh
+# but we don't care about those edge cases here
+chmod g+s a/b
+# This should never warn, but in did when special
+# bits are set on b (the common case under test)
+chmod -c -R g+w a 2>err
+test -s err && fail=1
+
 Exit $fail