Date: Thu, 13 Sep 2001 20:27:42 +0300 From: Ruslan Ermilov <ru@FreeBSD.org> To: arch@FreeBSD.org, freebsd-standards@bostonradio.org Cc: Bruce Evans <bde@FreeBSD.org>, Garrett Wollman <wollman@FreeBSD.org> Subject: Fixing chown(8) and friends to handle symlinks properly Message-ID: <20010913202742.C2458@sunbay.com>
next in thread | raw e-mail | index | archive | help
--2fHTh5uZTiUOsy+g
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
[Bcc'ed to -current in a hope for a more wider auditory]
The attached patch fixes the handling of symlinks in chown(8)
and chgrp(1). It is currently broken in many different ways.
Basically, it is broken because -h is not supported with -RH
or -RL. The actual problem lies in the depths of the fts(3)
implementation, in particular, how the `fts_stat' is getting
initialized. The trick in supporting roughly any combination
of command line flags ([-h] [-R [-H | -L | -P]]) was to avoid
looking into the `fts_stat' (as we don't know whether it is
from lstat(2) or stat(2)), and just call chown(2) or lchown(2)
as appropriate. Although it may seem as a big performance
degradation, in fact it isn't, as kernel doesn't modify the
inode if nothing is to be changed.
This work is based on the latest POSIX drafts, and the
superior NetBSD implementation (which I had to fix in a
number of cases anyway, in particular, handling of dead
symlinks in the -h case).
The new algorithm of handling a symlink encountered during
a file tree traversal works as follows:
+-------------+-------------------------------------------------+
| Options | Action |
+-------------+-------------------------------------------------+
| -- | chown |
| -h | lchown |
| | Notes: only FTS_SL symlinks may end up here |
+-------------+-------------------------------------------------+
| -RP | SKIP |
| -RP -h | lchown |
| | Notes: only FTS_SL symlinks may end here |
+-------------+-------------------------------------------------+
| -RH | SKIP |
| -RH -h | lchown |
| | Notes: both FTS_SL and FTS_SLNONE symlinks |
| | may end up here. FTS_SLNONE's are deadlinks |
| | specified as command line arguments |
+-------------+-------------------------------------------------+
| -RL | SKIP |
| -RL -h | lchown |
| | Notes: only FTS_SLNONE symlinks may end up here |
+-------------+-------------------------------------------------+
Or, in a more compact form:
: if symlink {
: if -R
: -h ? lchmod : SKIP
: else
: -h ? lchmod : chmod
: } else
: chmod
For the testing purposes, I'd suggest creating the following
structure, which should be self-explaining:
afile
alink -> afile
deadlink -> nofile
dir
adir -> dir
dir/afile
dir/alink -> afile
dir/deadlink -> nofile
PLEASE TEST THIS PATCH THROUGHLY!
NOTE: This implementation is still not quite POSIX compliant, as
the latter says to follow a symlink (in the -R case) only if it
points to an object of type directory. Our fts(3) routines are
unable of doing this.
If this patch is accepted, I'm going to revisit and fix the rest
of the fts(3) utilities that are listed on the symlink(7) manpage.
Cheers,
--
Ruslan Ermilov Oracle Developer/DBA,
ru@sunbay.com Sunbay Software AG,
ru@FreeBSD.org FreeBSD committer,
+380.652.512.251 Simferopol, Ukraine
http://www.FreeBSD.org The Power To Serve
http://www.oracle.com Enabling The Information Age
--2fHTh5uZTiUOsy+g
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=p
Index: chown.c
===================================================================
RCS file: /home/ncvs/src/usr.sbin/chown/chown.c,v
retrieving revision 1.19
diff -u -p -r1.19 chown.c
--- chown.c 2001/09/13 14:55:59 1.19
+++ chown.c 2001/09/13 16:54:28
@@ -80,6 +80,7 @@ main(argc, argv)
int Hflag, Lflag, Rflag, fflag, hflag, vflag;
int ch, fts_options, rval;
char *cp;
+ int (*change_owner) __P((const char *, uid_t, gid_t));
cp = strrchr(argv[0], '/');
cp = (cp != NULL) ? cp + 1 : argv[0];
@@ -121,19 +122,15 @@ main(argc, argv)
if (argc < 2)
usage();
+ fts_options = FTS_PHYSICAL;
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;
}
- } else
- fts_options = hflag ? FTS_PHYSICAL : FTS_LOGICAL;
+ }
uid = (uid_t)-1;
gid = (gid_t)-1;
@@ -156,6 +153,7 @@ main(argc, argv)
err(1, NULL);
for (rval = 0; (p = fts_read(ftsp)) != NULL;) {
+ change_owner = chown;
switch (p->fts_info) {
case FTS_D: /* Change it at FTS_DP. */
if (!Rflag)
@@ -170,31 +168,48 @@ main(argc, 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.
+ * don't point to anything. Note that if we are
+ * doing a physical walk, we never reach here unless
+ * we asked to follow explicitly.
*/
- if (hflag)
- break;
- else
+ /* FALLTHROUGH */
+ case FTS_SL:
+ /*
+ * All symlinks we found while doing a physical
+ * walk end up here.
+ */
+ if (Rflag && !hflag)
continue;
+ /*
+ * Note that if we follow a symlink, fts_info is
+ * not FTS_SL but FTS_F or whatever. And we should
+ * use lchown(2) only for FTS_SL and should use
+ * chown(2) for others.
+ */
+ if (hflag)
+ change_owner = lchown;
+ break;
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 ((*change_owner)(p->fts_accpath, uid, gid) == -1) {
if (!fflag) {
chownerr(p->fts_path);
rval = 1;
}
} else {
if (vflag)
+#define DEBUG
+#ifdef DEBUG
+ printf("%s %s\n",
+ (change_owner == chown) ? "chown" : "lchown",
+ p->fts_path);
+#else
printf("%s\n", p->fts_path);
+#endif
}
}
if (errno)
--2fHTh5uZTiUOsy+g--
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-arch" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20010913202742.C2458>