From owner-freebsd-arch Thu Sep 13 10:28:24 2001 Delivered-To: freebsd-arch@freebsd.org Received: from whale.sunbay.crimea.ua (whale.sunbay.crimea.ua [212.110.138.65]) by hub.freebsd.org (Postfix) with ESMTP id 12E3237B40E; Thu, 13 Sep 2001 10:27:49 -0700 (PDT) Received: (from ru@localhost) by whale.sunbay.crimea.ua (8.11.2/8.11.2) id f8DHRgQ06496; Thu, 13 Sep 2001 20:27:42 +0300 (EEST) (envelope-from ru) Date: Thu, 13 Sep 2001 20:27:42 +0300 From: Ruslan Ermilov To: arch@FreeBSD.org, freebsd-standards@bostonradio.org Cc: Bruce Evans , Garrett Wollman Subject: Fixing chown(8) and friends to handle symlinks properly Message-ID: <20010913202742.C2458@sunbay.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="2fHTh5uZTiUOsy+g" Content-Disposition: inline User-Agent: Mutt/1.2.5i Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG --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