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

[Bernhard Voelker]

On 03/18/2014 07:45 PM, Pádraig Brady wrote:
> The fix looks good.  It seems this bug was introduced
> with the change to FTS in 5.1.0 (released Dec 2003)

Wasn't it commit v5.2.1-690-gbc580f6?

> I'll push soon in your name with these changes merged in.

> $ git diff address@hidden
> 
> 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]
> +

v5.3.0? See above.

>    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

s/ in / it /

> +# 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

Thanks.

One question: I did not dig into this deeper yet, but what exactly
is the connection between "directory with special permissions"
vs. "stat()ing the wrong file"?
I'm asking because incidentally yesterday I saw the same warning
from "chmod -R -c" when playing with recursive bind-mounts, i.e.,
there were no files or directories with special bits set.

Thanks & have a nice day,
Berny