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

From: [email-masked] (GNU bug Tracking System)
Date: Tue, 18 Mar 2014 18:46:02 +0000

Your bug report

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

which was filed against the coreutils package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
17035: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=17035
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#17035: [PATCH] chmod -c -R produces errors with special permissions
From: Pádraig Brady <[email-masked]>
To: Dylan Alex Simon <[email-masked]>
Cc: [email-masked]
Date: Tue, 18 Mar 2014 18:45:29 +0000
List-Id: <debbugs-submit.debbugs.gnu.org>

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

Subject: [PATCH] chmod -c -R produces errors with special permissions
From: Dylan Alex Simon <[email-masked]>
To: [email-masked]
Date: Tue, 18 Mar 2014 12:06:31 -0400
List-Id: <debbugs-submit.debbugs.gnu.org>

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.
>From 8e4f39ea022c9305d43ebd2823cf07b5daf682a9 Mon Sep 17 00:00:00 2001
From: Dylan Simon <[email-masked]>
Date: Tue, 18 Mar 2014 11:50:29 -0400
Subject: [PATCH] chmod: fix mode_changed check

* src/chmod.c (mode_changed): Use fts->fts_cwd_fd with fstatat rather
than stat. All callers changed.
---
 src/chmod.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/chmod.c b/src/chmod.c
index 81bf4b2..f9debde 100644
--- a/src/chmod.c
+++ b/src/chmod.c
@@ -111,7 +111,8 @@ static struct option const long_options[] =
    The old mode was OLD_MODE, but it was changed to NEW_MODE.  */
 
 static bool
-mode_changed (char const *file, mode_t old_mode, mode_t new_mode)
+mode_changed (int dirfd, 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))
     {
@@ -120,10 +121,10 @@ mode_changed (char const *file, mode_t old_mode, mode_t new_mode)
 
       struct stat new_stats;
 
-      if (stat (file, &new_stats) != 0)
+      if (fstatat (dirfd, file, &new_stats, 0) != 0)
         {
           if (! force_silent)
-            error (0, errno, _("getting new attributes of %s"), quote (file));
+            error (0, errno, _("getting new attributes of %s"), quote (file_full_name));
           return false;
         }
 
@@ -283,7 +284,8 @@ process_file (FTS *fts, FTSENT *ent)
   if (verbosity != V_off)
     {
       bool changed = (chmod_succeeded
-                      && mode_changed (file, old_mode, new_mode));
+                      && mode_changed (fts->fts_cwd_fd, file, file_full_name,
+                                       old_mode, new_mode));
 
       if (changed || verbosity == V_high)
         {
-- 
1.9.0