Author: smh
Date: Wed Apr 29 00:49:00 2015
New Revision: 282208
URL: https://svnweb.freebsd.org/changeset/base/282208

Log:
  Standardise chmod, chflags, chown and chgrp recursive symlink processing
  
  chmod, chflags, chgrp, chmod and chown now affect symlinks in -R mode as
  defined in symlink(7); previously symlinks were silently ignored.
  
  Differential Revision:        https://reviews.freebsd.org/D2316
  Reviewed by:  jilles
  MFC after:    1 month
  Relnotes:     yes
  Sponsored by: Multiplay

Modified:
  head/UPDATING
  head/bin/chflags/chflags.1
  head/bin/chflags/chflags.c
  head/bin/chmod/chmod.1
  head/bin/chmod/chmod.c
  head/usr.sbin/chown/chgrp.1
  head/usr.sbin/chown/chown.8
  head/usr.sbin/chown/chown.c

Modified: head/UPDATING
==============================================================================
--- head/UPDATING       Tue Apr 28 23:54:55 2015        (r282207)
+++ head/UPDATING       Wed Apr 29 00:49:00 2015        (r282208)
@@ -31,6 +31,10 @@ NOTE TO PEOPLE WHO THINK THAT FreeBSD 11
        disable the most expensive debugging functionality run
        "ln -s 'abort:false,junk:false' /etc/malloc.conf".)
 
+20150523:
+       chmod, chflags, chown and chgrp now affect symlinks in -R mode as
+       defined in symlink(7); previously symlinks were silently ignored.
+
 20150415:
        The const qualifier has been removed from iconv(3) to comply with
        POSIX.  The ports tree is aware of this from r384038 onwards.

Modified: head/bin/chflags/chflags.1
==============================================================================
--- head/bin/chflags/chflags.1  Tue Apr 28 23:54:55 2015        (r282207)
+++ head/bin/chflags/chflags.1  Wed Apr 29 00:49:00 2015        (r282208)
@@ -32,7 +32,7 @@
 .\"    @(#)chflags.1   8.4 (Berkeley) 5/2/95
 .\" $FreeBSD$
 .\"
-.Dd April 8, 2013
+.Dd April 20, 2015
 .Dt CHFLAGS 1
 .Os
 .Sh NAME
@@ -66,8 +66,9 @@ nor modify the exit status to reflect su
 .It Fl H
 If the
 .Fl R
-option is specified, symbolic links on the command line are followed.
-(Symbolic links encountered in the tree traversal are not followed.)
+option is specified, symbolic links on the command line are followed
+and hence unaffected by the command.
+(Symbolic links encountered during traversal are not followed.)
 .It Fl h
 If the
 .Ar file
@@ -83,8 +84,12 @@ If the
 option is specified, no symbolic links are followed.
 This is the default.
 .It Fl R
-Change the file flags for the file hierarchies rooted
-in the files instead of just the files themselves.
+Change the file flags of the file hierarchies rooted in the files,
+instead of just the files themselves.
+Beware of unintentionally matching the
+.Dq Pa ".."
+hard link to the parent directory when using wildcards like
+.Dq Li ".*" .
 .It Fl v
 Cause
 .Nm

Modified: head/bin/chflags/chflags.c
==============================================================================
--- head/bin/chflags/chflags.c  Tue Apr 28 23:54:55 2015        (r282207)
+++ head/bin/chflags/chflags.c  Wed Apr 29 00:49:00 2015        (r282208)
@@ -47,6 +47,7 @@ __FBSDID("$FreeBSD$");
 
 #include <err.h>
 #include <errno.h>
+#include <fcntl.h>
 #include <fts.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -65,7 +66,6 @@ main(int argc, char *argv[])
        int Hflag, Lflag, Rflag, fflag, hflag, vflag;
        int ch, fts_options, oct, rval;
        char *flags, *ep;
-       int (*change_flags)(const char *, unsigned long);
 
        Hflag = Lflag = Rflag = fflag = hflag = vflag = 0;
        while ((ch = getopt(argc, argv, "HLPRfhv")) != -1)
@@ -104,20 +104,23 @@ main(int argc, char *argv[])
                usage();
 
        if (Rflag) {
-               fts_options = FTS_PHYSICAL;
                if (hflag)
-                       errx(1, "the -R and -h options "
-                               "may not be specified together");
-               if (Hflag)
-                       fts_options |= FTS_COMFOLLOW;
+                       errx(1, "the -R and -h options may not be "
+                           "specified together.");
                if (Lflag) {
-                       fts_options &= ~FTS_PHYSICAL;
-                       fts_options |= FTS_LOGICAL;
+                       fts_options = FTS_LOGICAL;
+               } else {
+                       fts_options = FTS_PHYSICAL;
+
+                       if (Hflag) {
+                               fts_options |= FTS_COMFOLLOW;
+                       }
                }
-       } else
-               fts_options = hflag ? FTS_PHYSICAL : FTS_LOGICAL;
-
-       change_flags = hflag ? lchflags : chflags;
+       } else if (hflag) {
+               fts_options = FTS_PHYSICAL;
+       } else {
+               fts_options = FTS_LOGICAL;
+       }
 
        flags = *argv;
        if (*flags >= '0' && *flags <= '7') {
@@ -142,12 +145,21 @@ main(int argc, char *argv[])
                err(1, NULL);
 
        for (rval = 0; (p = fts_read(ftsp)) != NULL;) {
+               int atflag;
+
+               if ((fts_options & FTS_LOGICAL) ||
+                   ((fts_options & FTS_COMFOLLOW) &&
+                   p->fts_level == FTS_ROOTLEVEL))
+                       atflag = 0;
+               else
+                       atflag = AT_SYMLINK_NOFOLLOW;
+
                switch (p->fts_info) {
                case FTS_D:     /* Change it at FTS_DP if we're recursive. */
                        if (!Rflag)
                                fts_set(ftsp, p, FTS_SKIP);
                        continue;
-               case FTS_DNR:                   /* Warn, chflag, continue. */
+               case FTS_DNR:                   /* Warn, chflags. */
                        warnx("%s: %s", p->fts_path, strerror(p->fts_errno));
                        rval = 1;
                        break;
@@ -156,16 +168,6 @@ main(int argc, char *argv[])
                        warnx("%s: %s", p->fts_path, strerror(p->fts_errno));
                        rval = 1;
                        continue;
-               case FTS_SL:                    /* Ignore. */
-               case FTS_SLNONE:
-                       /*
-                        * The only symlinks that end up here are ones that
-                        * don't point to anything and ones that we found
-                        * doing a physical walk.
-                        */
-                       if (!hflag)
-                               continue;
-                       /* FALLTHROUGH */
                default:
                        break;
                }
@@ -175,7 +177,8 @@ main(int argc, char *argv[])
                        newflags = (p->fts_statp->st_flags | set) & clear;
                if (newflags == p->fts_statp->st_flags)
                        continue;
-               if ((*change_flags)(p->fts_accpath, newflags) && !fflag) {
+               if (chflagsat(AT_FDCWD, p->fts_accpath, newflags,
+                   atflag) == -1 && !fflag) {
                        warn("%s", p->fts_path);
                        rval = 1;
                } else if (vflag) {

Modified: head/bin/chmod/chmod.1
==============================================================================
--- head/bin/chmod/chmod.1      Tue Apr 28 23:54:55 2015        (r282207)
+++ head/bin/chmod/chmod.1      Wed Apr 29 00:49:00 2015        (r282208)
@@ -32,7 +32,7 @@
 .\"    @(#)chmod.1     8.4 (Berkeley) 3/31/94
 .\" $FreeBSD$
 .\"
-.Dd January 26, 2009
+.Dd April 20, 2015
 .Dt CHMOD 1
 .Os
 .Sh NAME
@@ -63,9 +63,9 @@ nor modify the exit status to reflect su
 .It Fl H
 If the
 .Fl R
-option is specified, symbolic links on the command line are followed.
-(Symbolic links encountered in the tree traversal are not followed by
-default.)
+option is specified, symbolic links on the command line are followed
+and hence unaffected by the command.
+(Symbolic links encountered during tree traversal are not followed.)
 .It Fl h
 If the file is a symbolic link, change the mode of the link itself
 rather than the file that the link points to.
@@ -79,8 +79,12 @@ If the
 option is specified, no symbolic links are followed.
 This is the default.
 .It Fl R
-Change the modes of the file hierarchies rooted in the files
+Change the modes of the file hierarchies rooted in the files,
 instead of just the files themselves.
+Beware of unintentionally matching the
+.Dq Pa ".."
+hard link to the parent directory when using wildcards like
+.Dq Li ".*" .
 .It Fl v
 Cause
 .Nm

Modified: head/bin/chmod/chmod.c
==============================================================================
--- head/bin/chmod/chmod.c      Tue Apr 28 23:54:55 2015        (r282207)
+++ head/bin/chmod/chmod.c      Wed Apr 29 00:49:00 2015        (r282208)
@@ -46,6 +46,7 @@ __FBSDID("$FreeBSD$");
 
 #include <err.h>
 #include <errno.h>
+#include <fcntl.h>
 #include <fts.h>
 #include <limits.h>
 #include <stdio.h>
@@ -62,7 +63,7 @@ main(int argc, char *argv[])
        FTS *ftsp;
        FTSENT *p;
        mode_t *set;
-       int Hflag, Lflag, Rflag, ch, error, fflag, fts_options, hflag, rval;
+       int Hflag, Lflag, Rflag, ch, fflag, fts_options, hflag, rval;
        int vflag;
        char *mode;
        mode_t newmode;
@@ -126,18 +127,23 @@ done:     argv += optind;
                usage();
 
        if (Rflag) {
-               fts_options = FTS_PHYSICAL;
                if (hflag)
-                       errx(1,
-               "the -R and -h options may not be specified together.");
-               if (Hflag)
-                       fts_options |= FTS_COMFOLLOW;
+                       errx(1, "the -R and -h options may not be "
+                           "specified together.");
                if (Lflag) {
-                       fts_options &= ~FTS_PHYSICAL;
-                       fts_options |= FTS_LOGICAL;
+                       fts_options = FTS_LOGICAL;
+               } else {
+                       fts_options = FTS_PHYSICAL;
+
+                       if (Hflag) {
+                               fts_options |= FTS_COMFOLLOW;
+                       }
                }
-       } else
-               fts_options = hflag ? FTS_PHYSICAL : FTS_LOGICAL;
+       } else if (hflag) {
+               fts_options = FTS_PHYSICAL;
+       } else {
+               fts_options = FTS_LOGICAL;
+       }
 
        mode = *argv;
        if ((set = setmode(mode)) == NULL)
@@ -146,12 +152,21 @@ done:     argv += optind;
        if ((ftsp = fts_open(++argv, fts_options, 0)) == NULL)
                err(1, "fts_open");
        for (rval = 0; (p = fts_read(ftsp)) != NULL;) {
+               int atflag;
+
+               if ((fts_options & FTS_LOGICAL) ||
+                   ((fts_options & FTS_COMFOLLOW) &&
+                   p->fts_level == FTS_ROOTLEVEL))
+                       atflag = 0;
+               else
+                       atflag = AT_SYMLINK_NOFOLLOW;
+
                switch (p->fts_info) {
                case FTS_D:                     /* Change it at FTS_DP. */
                        if (!Rflag)
                                fts_set(ftsp, p, FTS_SKIP);
                        continue;
-               case FTS_DNR:                   /* Warn, chmod, continue. */
+               case FTS_DNR:                   /* Warn, chmod. */
                        warnx("%s: %s", p->fts_path, strerror(p->fts_errno));
                        rval = 1;
                        break;
@@ -160,16 +175,6 @@ done:      argv += optind;
                        warnx("%s: %s", p->fts_path, strerror(p->fts_errno));
                        rval = 1;
                        continue;
-               case FTS_SL:                    /* Ignore. */
-               case FTS_SLNONE:
-                       /*
-                        * The only symlinks that end up here are ones that
-                        * don't point to anything and ones that we found
-                        * doing a physical walk.
-                        */
-                       if (!hflag)
-                               continue;
-                       /* FALLTHROUGH */
                default:
                        break;
                }
@@ -182,32 +187,25 @@ done:     argv += optind;
                if (may_have_nfs4acl(p, hflag) == 0 &&
                    (newmode & ALLPERMS) == (p->fts_statp->st_mode & ALLPERMS))
                                continue;
-               if (hflag)
-                       error = lchmod(p->fts_accpath, newmode);
-               else
-                       error = chmod(p->fts_accpath, newmode);
-               if (error) {
-                       if (!fflag) {
-                               warn("%s", p->fts_path);
-                               rval = 1;
-                       }
-               } else {
-                       if (vflag) {
-                               (void)printf("%s", p->fts_path);
+               if (fchmodat(AT_FDCWD, p->fts_accpath, newmode, atflag) == -1
+                   && !fflag) {
+                       warn("%s", p->fts_path);
+                       rval = 1;
+               } else if (vflag) {
+                       (void)printf("%s", p->fts_path);
 
-                               if (vflag > 1) {
-                                       char m1[12], m2[12];
+                       if (vflag > 1) {
+                               char m1[12], m2[12];
 
-                                       strmode(p->fts_statp->st_mode, m1);
-                                       strmode((p->fts_statp->st_mode &
-                                           S_IFMT) | newmode, m2);
-                                       (void)printf(": 0%o [%s] -> 0%o [%s]",
-                                           p->fts_statp->st_mode, m1,
-                                           (p->fts_statp->st_mode & S_IFMT) |
-                                           newmode, m2);
-                               }
-                               (void)printf("\n");
+                               strmode(p->fts_statp->st_mode, m1);
+                               strmode((p->fts_statp->st_mode &
+                                   S_IFMT) | newmode, m2);
+                               (void)printf(": 0%o [%s] -> 0%o [%s]",
+                                   p->fts_statp->st_mode, m1,
+                                   (p->fts_statp->st_mode & S_IFMT) |
+                                   newmode, m2);
                        }
+                       (void)printf("\n");
                }
        }
        if (errno)

Modified: head/usr.sbin/chown/chgrp.1
==============================================================================
--- head/usr.sbin/chown/chgrp.1 Tue Apr 28 23:54:55 2015        (r282207)
+++ head/usr.sbin/chown/chgrp.1 Wed Apr 29 00:49:00 2015        (r282208)
@@ -31,7 +31,7 @@
 .\"     @(#)chgrp.1    8.3 (Berkeley) 3/31/94
 .\" $FreeBSD$
 .\"
-.Dd February 21, 2010
+.Dd April 20, 2015
 .Dt CHGRP 1
 .Os
 .Sh NAME
@@ -60,8 +60,9 @@ The following options are available:
 .It Fl H
 If the
 .Fl R
-option is specified, symbolic links on the command line are followed.
-(Symbolic links encountered in the tree traversal are not followed.)
+option is specified, symbolic links on the command line are followed
+and hence unaffected by the command.
+(Symbolic links encountered during traversal are not followed.)
 .It Fl L
 If the
 .Fl R
@@ -72,8 +73,12 @@ If the
 option is specified, no symbolic links are followed.
 This is the default.
 .It Fl R
-Change the group ID for the file hierarchies rooted
-in the files instead of just the files themselves.
+Change the group ID of the file hierarchies rooted in the files,
+instead of just the files themselves.
+Beware of unintentionally matching the
+.Dq Pa ".."
+hard link to the parent directory when using wildcards like
+.Dq Li ".*" .
 .It Fl f
 The force option ignores errors, except for usage errors and does not
 query about strange modes (unless the user does not have proper permissions).

Modified: head/usr.sbin/chown/chown.8
==============================================================================
--- head/usr.sbin/chown/chown.8 Tue Apr 28 23:54:55 2015        (r282207)
+++ head/usr.sbin/chown/chown.8 Wed Apr 29 00:49:00 2015        (r282208)
@@ -28,7 +28,7 @@
 .\"     @(#)chown.8    8.3 (Berkeley) 3/31/94
 .\" $FreeBSD$
 .\"
-.Dd February 21, 2010
+.Dd April 20, 2015
 .Dt CHOWN 8
 .Os
 .Sh NAME
@@ -64,8 +64,9 @@ The options are as follows:
 .It Fl H
 If the
 .Fl R
-option is specified, symbolic links on the command line are followed.
-(Symbolic links encountered in the tree traversal are not followed.)
+option is specified, symbolic links on the command line are followed
+and hence unaffected by the command.
+(Symbolic links encountered during traversal are not followed.)
 .It Fl L
 If the
 .Fl R
@@ -76,8 +77,8 @@ If the
 option is specified, no symbolic links are followed.
 This is the default.
 .It Fl R
-Change the user ID and/or the group ID of the specified directory trees
-(recursively, including their contents) and files.
+Change the user ID and/or the group ID of the file hierarchies rooted
+in the files, instead of just the files themselves.
 Beware of unintentionally matching the
 .Dq Pa ".."
 hard link to the parent directory when using wildcards like

Modified: head/usr.sbin/chown/chown.c
==============================================================================
--- head/usr.sbin/chown/chown.c Tue Apr 28 23:54:55 2015        (r282207)
+++ head/usr.sbin/chown/chown.c Wed Apr 29 00:49:00 2015        (r282208)
@@ -47,6 +47,7 @@ __FBSDID("$FreeBSD$");
 
 #include <err.h>
 #include <errno.h>
+#include <fcntl.h>
 #include <fts.h>
 #include <grp.h>
 #include <libgen.h>
@@ -119,18 +120,24 @@ main(int argc, char **argv)
                usage();
 
        if (Rflag) {
-               fts_options = FTS_PHYSICAL;
                if (hflag && (Hflag || Lflag))
                        errx(1, "the -R%c and -h options may not be "
                            "specified together", Hflag ? 'H' : 'L');
-               if (Hflag)
-                       fts_options |= FTS_COMFOLLOW;
-               else if (Lflag) {
-                       fts_options &= ~FTS_PHYSICAL;
-                       fts_options |= FTS_LOGICAL;
+               if (Lflag) {
+                       fts_options = FTS_LOGICAL;
+               } else {
+                       fts_options = FTS_PHYSICAL;
+
+                       if (Hflag) {
+                               fts_options |= FTS_COMFOLLOW;
+                       }
                }
-       } else
-               fts_options = hflag ? FTS_PHYSICAL : FTS_LOGICAL;
+       } else if (hflag) {
+               fts_options = FTS_PHYSICAL;
+       } else {
+               fts_options = FTS_LOGICAL;
+       }
+
        if (xflag)
                fts_options |= FTS_XDEV;
 
@@ -156,6 +163,15 @@ main(int argc, char **argv)
                err(1, NULL);
 
        for (rval = 0; (p = fts_read(ftsp)) != NULL;) {
+               int atflag;
+
+               if ((fts_options & FTS_LOGICAL) ||
+                   ((fts_options & FTS_COMFOLLOW) &&
+                   p->fts_level == FTS_ROOTLEVEL))
+                       atflag = 0;
+               else
+                       atflag = AT_SYMLINK_NOFOLLOW;
+
                switch (p->fts_info) {
                case FTS_D:                     /* Change it at FTS_DP. */
                        if (!Rflag)
@@ -170,58 +186,44 @@ main(int argc, char **argv)
                        warnx("%s: %s", p->fts_path, strerror(p->fts_errno));
                        rval = 1;
                        continue;
-               case FTS_SL:
-               case FTS_SLNONE:
-                       /*
-                        * The only symlinks that end up here are ones that
-                        * don't point to anything and ones that we found
-                        * doing a physical walk.
-                        */
-                       if (hflag)
-                               break;
-                       else
-                               continue;
                default:
                        break;
                }
                if ((uid == (uid_t)-1 || uid == p->fts_statp->st_uid) &&
                    (gid == (gid_t)-1 || gid == p->fts_statp->st_gid))
                        continue;
-               if ((hflag ? lchown : chown)(p->fts_accpath, uid, gid) == -1) {
-                       if (!fflag) {
-                               chownerr(p->fts_path);
-                               rval = 1;
-                       }
-               } else {
-                       if (vflag) {
-                               printf("%s", p->fts_path);
-                               if (vflag > 1) {
-                                       if (ischown) {
-                                               printf(": %ju:%ju -> %ju:%ju",
-                                                   (uintmax_t)
-                                                   p->fts_statp->st_uid, 
-                                                   (uintmax_t)
-                                                   p->fts_statp->st_gid,
-                                                   (uid == (uid_t)-1) ? 
-                                                   (uintmax_t)
-                                                   p->fts_statp->st_uid : 
-                                                   (uintmax_t)uid,
-                                                   (gid == (gid_t)-1) ? 
-                                                   (uintmax_t)
-                                                   p->fts_statp->st_gid :
-                                                   (uintmax_t)gid);
-                                       } else {
-                                               printf(": %ju -> %ju",
-                                                   (uintmax_t)
-                                                   p->fts_statp->st_gid,
-                                                   (gid == (gid_t)-1) ? 
-                                                   (uintmax_t)
-                                                   p->fts_statp->st_gid : 
-                                                   (uintmax_t)gid);
-                                       }
+               if (fchownat(AT_FDCWD, p->fts_accpath, uid, gid, atflag)
+                   == -1 && !fflag) {
+                       chownerr(p->fts_path);
+                       rval = 1;
+               } else if (vflag) {
+                       printf("%s", p->fts_path);
+                       if (vflag > 1) {
+                               if (ischown) {
+                                       printf(": %ju:%ju -> %ju:%ju",
+                                           (uintmax_t)
+                                           p->fts_statp->st_uid, 
+                                           (uintmax_t)
+                                           p->fts_statp->st_gid,
+                                           (uid == (uid_t)-1) ? 
+                                           (uintmax_t)
+                                           p->fts_statp->st_uid : 
+                                           (uintmax_t)uid,
+                                           (gid == (gid_t)-1) ? 
+                                           (uintmax_t)
+                                           p->fts_statp->st_gid :
+                                           (uintmax_t)gid);
+                               } else {
+                                       printf(": %ju -> %ju",
+                                           (uintmax_t)
+                                           p->fts_statp->st_gid,
+                                           (gid == (gid_t)-1) ? 
+                                           (uintmax_t)
+                                           p->fts_statp->st_gid : 
+                                           (uintmax_t)gid);
                                }
-                               printf("\n");
                        }
+                       printf("\n");
                }
        }
        if (errno)
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "[email protected]"

Reply via email to