Date: Mon, 7 May 2007 15:13:24 -0600 From: John E Hein <jhein@timing.com> To: FreeBSD-gnats-submit@FreeBSD.org, freebsd-bugs@FreeBSD.org Subject: Re: bin/112336: install -S (safe copy) with -C or -p is not so safe Message-ID: <17983.38516.966371.23214@gromit.timing.com> In-Reply-To: <200705020050.l420o3qI065567@freefall.freebsd.org> References: <200705020011.l420BC2t051835@gromit.timing.com> <200705020050.l420o3qI065567@freefall.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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,
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?17983.38516.966371.23214>