bin/112336: install -S (safe copy) with -C or -p is not so safe

[John E Hein]

Here is a patch that implements 2 and was simpler than I thought it
would be.

It creates the temp file and does a rename, not only if the contents
of the "from" & "to" files don't match, but also if perms/ownership
don't match.

One optimization could be to try to do the chown/chmod first and if
that succceeds and the files match contents, don't bother with the
temp file.  But that seems to be needlessly complex for the relatively
miniscule number of cases where we could gain from the added pedantry.
And there may be edge case failures associated with an early
chmod/chown.

Any committers willing to take this one?


Index: src/usr.bin/xinstall/xinstall.c
===================================================================
RCS file: /base/FreeBSD-CVS/src/usr.bin/xinstall/xinstall.c,v
retrieving revision 1.67
diff -u -p -r1.67 xinstall.c
--- src/usr.bin/xinstall/xinstall.c	6 Mar 2006 21:52:59 -0000	1.67
+++ src/usr.bin/xinstall/xinstall.c	7 May 2007 20:52:18 -0000
@@ -317,7 +317,19 @@ install(const char *from_name, const cha
 	if (docompare && !dostrip && target) {
 		if ((to_fd = open(to_name, O_RDONLY, 0)) < 0)
 			err(EX_OSERR, "%s", to_name);
-		if (devnull)
+		/*
+		 * Even if the contents are the same, we want to rename
+		 * temp files when doing a "safe" copy if the
+		 * permissions and ownership need to change.  We may
+		 * not have permission to chown/chmod the "to" file
+		 * directly.
+		 */
+		if (tempcopy &&
+		    ((gid != (gid_t)-1 && gid != to_sb.st_gid) ||
+		    (uid != (uid_t)-1 && uid != to_sb.st_uid) ||
+		    (mode != (to_sb.st_mode & ALLPERMS))))
+		    files_match = 0;
+		else if (devnull)
 			files_match = to_sb.st_size == 0;
 		else
 			files_match = !(compare(from_fd, from_name,