fchmod in do_mkstemp? (patch included)

Assar assar at permabit.com
Sun Apr 11 00:04:00 GMT 2004


Why is do_mkstemp fchmod-ing the temporary file?  I was not able to
figure this out from the CVS logs or my searches in the mail archives.

Currently, do_mkstemp does this (*):

        mkstemp temporary file (which leaves it with 0600)
        fchmod temporary file (final perm & 0700)

And then later it gets renamed to the final name and permissions set
to what they are supposed to be.

(*) rsync only uses mkstemp if it creates files with 0600, otherwise
it does mktemp and open.

What purpose does the fchmod serve here?  Having it in there makes
rsync not be able to copy read-only files on some weird filesystems.
But regardless of that case, I wonder if it adds anything in common
cases.  If you're copying a read-only file, the temporary file would
be writable by the owner for a longer time, but it's not like
protecting against the owner doing something to the file is useful.

So, can anyone tell me any reasons for having the fchmod there?  And
if there are none, would anyone object to removing it?  A simple patch
for that is appended.  Thanks.

/assar

-------------- next part --------------
Index: proto.h
===================================================================
RCS file: /cvsroot/rsync/proto.h,v
retrieving revision 1.185
diff -u -w -r1.185 proto.h
--- proto.h	27 Mar 2004 09:44:34 -0000	1.185
+++ proto.h	10 Apr 2004 23:48:36 -0000
@@ -222,7 +222,7 @@
 int do_rename(char *fname1, char *fname2);
 void trim_trailing_slashes(char *name);
 int do_mkdir(char *fname, mode_t mode);
-int do_mkstemp(char *template, mode_t perms);
+int do_mkstemp(char *template);
 int do_stat(const char *fname, STRUCT_STAT *st);
 int do_lstat(const char *fname, STRUCT_STAT *st);
 int do_fstat(int fd, STRUCT_STAT *st);
Index: receiver.c
===================================================================
RCS file: /cvsroot/rsync/receiver.c,v
retrieving revision 1.75
diff -u -w -r1.75 receiver.c
--- receiver.c	23 Mar 2004 16:50:40 -0000	1.75
+++ receiver.c	10 Apr 2004 23:48:36 -0000
@@ -424,7 +424,7 @@
 		 * the lchown. Thanks to snabb at epipe.fi for pointing
 		 * this out.  We also set it initially without group
 		 * access because of a similar race condition. */
-		fd2 = do_mkstemp(fnametmp, file->mode & INITACCESSPERMS);
+		fd2 = do_mkstemp(fnametmp);
 
 		/* in most cases parent directories will already exist
 		 * because their information should have been previously
@@ -432,7 +432,7 @@
 		if (fd2 == -1 && relative_paths && errno == ENOENT &&
 		    create_directory_path(fnametmp, orig_umask) == 0) {
 			strlcpy(fnametmp, template, sizeof fnametmp);
-			fd2 = do_mkstemp(fnametmp, file->mode & INITACCESSPERMS);
+			fd2 = do_mkstemp(fnametmp);
 		}
 		if (fd2 == -1) {
 			rprintf(FERROR, "mkstemp %s failed: %s\n",
Index: syscall.c
===================================================================
RCS file: /cvsroot/rsync/syscall.c,v
retrieving revision 1.30
diff -u -w -r1.30 syscall.c
--- syscall.c	18 Feb 2004 22:33:21 -0000	1.30
+++ syscall.c	10 Apr 2004 23:48:36 -0000
@@ -146,8 +146,8 @@
 }
 
 
-/* like mkstemp but forces permissions */
-int do_mkstemp(char *template, mode_t perms)
+/* like mkstemp but force 0600 permissions */
+int do_mkstemp(char *template)
 {
 	RETURN_ERROR_IF(dry_run, 0);
 	RETURN_ERROR_IF(read_only, EROFS);
@@ -155,21 +155,12 @@
 #if defined(HAVE_SECURE_MKSTEMP) && defined(HAVE_FCHMOD)
 	{
 		int fd = mkstemp(template);
-		if (fd == -1)
-			return -1;
-		if (fchmod(fd, perms) != 0 && preserve_perms) {
-			int errno_save = errno;
-			close(fd);
-			unlink(template);
-			errno = errno_save;
-			return -1;
-		}
 		return fd;
 	}
 #else
 	if (!mktemp(template))
 		return -1;
-	return do_open(template, O_RDWR|O_EXCL|O_CREAT, perms);
+	return do_open(template, O_RDWR|O_EXCL|O_CREAT, 0600);
 #endif
 }
 


More information about the rsync mailing list