From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Paul Eggert Newsgroups: gmane.emacs.bugs Subject: bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem Date: Mon, 23 Dec 2013 15:58:29 -0800 Organization: UCLA Computer Science Department Message-ID: <52B8CE25.9080803@cs.ucla.edu> References: <52B62BC3.4050508@cs.ucla.edu> <83ob498s3q.fsf@gnu.org> <52B66414.1090709@cs.ucla.edu> <87y53czxz5.fsf@wizzle.ran.pp.ru> <52B7377C.3070004@cs.ucla.edu> <8738lkhcen.fsf@wizzle.ran.pp.ru> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------020404090304010004010701" X-Trace: ger.gmane.org 1387843154 25332 80.91.229.3 (23 Dec 2013 23:59:14 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Mon, 23 Dec 2013 23:59:14 +0000 (UTC) Cc: 16133@debbugs.gnu.org To: Artem Chuprina Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Tue Dec 24 00:59:18 2013 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1VvFP3-0003Sw-Lj for geb-bug-gnu-emacs@m.gmane.org; Tue, 24 Dec 2013 00:59:17 +0100 Original-Received: from localhost ([::1]:36011 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VvFP3-0007Qp-9A for geb-bug-gnu-emacs@m.gmane.org; Mon, 23 Dec 2013 18:59:17 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:35023) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VvFOv-0007H7-2l for bug-gnu-emacs@gnu.org; Mon, 23 Dec 2013 18:59:13 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VvFOq-0000Wn-7H for bug-gnu-emacs@gnu.org; Mon, 23 Dec 2013 18:59:08 -0500 Original-Received: from debbugs.gnu.org ([140.186.70.43]:51200) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VvFOq-0000Wi-3g for bug-gnu-emacs@gnu.org; Mon, 23 Dec 2013 18:59:04 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.80) (envelope-from ) id 1VvFOo-0003Lx-VV for bug-gnu-emacs@gnu.org; Mon, 23 Dec 2013 18:59:03 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Paul Eggert Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 23 Dec 2013 23:59:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 16133 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: confirmed Original-Received: via spool by 16133-submit@debbugs.gnu.org id=B16133.138784311812851 (code B ref 16133); Mon, 23 Dec 2013 23:59:02 +0000 Original-Received: (at 16133) by debbugs.gnu.org; 23 Dec 2013 23:58:38 +0000 Original-Received: from localhost ([127.0.0.1]:36986 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1VvFOP-0003L9-17 for submit@debbugs.gnu.org; Mon, 23 Dec 2013 18:58:38 -0500 Original-Received: from smtp.cs.ucla.edu ([131.179.128.62]:34351) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1VvFOK-0003Kx-KA for 16133@debbugs.gnu.org; Mon, 23 Dec 2013 18:58:35 -0500 Original-Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp.cs.ucla.edu (Postfix) with ESMTP id 4540F39E8116; Mon, 23 Dec 2013 15:58:31 -0800 (PST) X-Virus-Scanned: amavisd-new at smtp.cs.ucla.edu Original-Received: from smtp.cs.ucla.edu ([127.0.0.1]) by localhost (smtp.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id spHQ6L4lwwzO; Mon, 23 Dec 2013 15:58:29 -0800 (PST) Original-Received: from [192.168.1.9] (pool-108-0-233-62.lsanca.fios.verizon.net [108.0.233.62]) by smtp.cs.ucla.edu (Postfix) with ESMTPSA id 5D82139E8108; Mon, 23 Dec 2013 15:58:29 -0800 (PST) User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0 In-Reply-To: <8738lkhcen.fsf@wizzle.ran.pp.ru> X-Enigmail-Version: 1.6 X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 140.186.70.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:82476 Archived-At: This is a multi-part message in MIME format. --------------020404090304010004010701 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Artem Chuprina wrote: > As you appeal to GNU cp, see > its default behavior: BY DEFAULT it TRIES to save permissions and > owner/group No, by default GNU cp does not try to copy either owner/group or permissions to an existing destination. It does not invoke chmod or chown unless you use something like 'cp -p'. > I can create files there, but I cannot change > their metainfo, because they are not mine. This is an unusual setup, at least for me. If I create a file, I should be able to change its metainformation. I expect this setup will cause problems with other applications, not just Emacs. GNU tar would be one example. That being said, we should be able to work around the problem by having copy-file behave more like 'cp'. That is, copy-file should not invoke chmod by default; it should invoke chmod only if it's told to preserve permissions (or preserve ownership, since that often involves temporarily revoking permissions for security reasons). That way, plain copy-file should work with your setup, although you'll still have trouble with copy-file with the last arg t (which asks to copy permissions). Attached is a proposed patch to do that, against trunk bzr 115721. Does it solve your problem? --------------020404090304010004010701 Content-Type: text/x-patch; name="copy-file.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="copy-file.diff" === modified file 'doc/lispref/ChangeLog' --- doc/lispref/ChangeLog 2013-12-23 11:27:29 +0000 +++ doc/lispref/ChangeLog 2013-12-23 23:53:26 +0000 @@ -1,3 +1,8 @@ +2013-12-23 Paul Eggert + + Plain copy-file no longer chmods an existing destination (Bug#16133). + * files.texi (Changing Files): Document this. + 2013-12-23 Xue Fuqiao * eval.texi (Special Forms): Document `special-form-p'. === modified file 'doc/lispref/files.texi' --- doc/lispref/files.texi 2013-12-23 08:50:31 +0000 +++ doc/lispref/files.texi 2013-12-23 23:53:26 +0000 @@ -1563,8 +1563,6 @@ interactive call, a prefix argument specifies a non-@code{nil} value for @var{time}. -This function copies the file modes, too. - If argument @var{preserve-uid-gid} is @code{nil}, we let the operating system decide the user and group ownership of the new file (this is usually set to the user running Emacs). If @var{preserve-uid-gid} is @@ -1572,10 +1570,11 @@ file. This works only on some operating systems, and only if you have the correct permissions to do so. -If the optional argument @var{preserve-extended-attributes} is -non-@code{nil}, and Emacs has been built with the appropriate support, -this function attempts to copy the file's extended attributes, such as -its SELinux context and ACL entries (@pxref{File Attributes}). +If the optional argument @var{preserve-permissions} is non-@code{nil}, +this function copies the file's permissions, such as its file modes, +its SELinux context, and ACL entries (@pxref{File Attributes}). +Otherwise, if the destination is created its file permission bits are +those of the source, masked by the default file permissions. @end deffn @deffn Command make-symbolic-link filename newname &optional ok-if-exists === modified file 'etc/ChangeLog' --- etc/ChangeLog 2013-12-23 22:27:49 +0000 +++ etc/ChangeLog 2013-12-23 23:53:26 +0000 @@ -1,3 +1,8 @@ +2013-12-23 Paul Eggert + + Plain copy-file no longer chmods an existing destination (Bug#16133). + * NEWS: Document this. + 2013-12-23 Teodor Zlatanov * NEWS: Updated for `gnutls-verify-error', cfengine-mode, and === modified file 'etc/NEWS' --- etc/NEWS 2013-12-23 13:05:27 +0000 +++ etc/NEWS 2013-12-23 23:53:26 +0000 @@ -889,6 +889,12 @@ `file-extended-attributes'. The attributes can be applied to another file using `set-file-extended-attributes'. +** By default `copy-file' no longer copies file permission bits to an +existing destination; and it sets the file permission bits of a newly +created destination to those of the source, masked by the default file +permissions. To copy the file permission bits, pass t as the +PRESERVE-PERMISSIONS argument of `copy-file'. + ** `visited-file-modtime' now returns -1 for nonexistent files. Formerly it returned a list (-1 LOW USEC PSEC), but this was ambiguous in the presence of files with negative time stamps. @@ -1030,8 +1036,8 @@ +++ *** The 6th argument to `copy-file' has been renamed to -PRESERVE-EXTENDED-ATTRIBUTES as it now handles both SELinux context -and ACL entries. +PRESERVE-PERMISSIONS as it now handles ACL entries and the traditional +Unix file permission bits as well as SELinux context. +++ *** The function `file-ownership-preserved-p' now has an optional === modified file 'src/ChangeLog' --- src/ChangeLog 2013-12-23 19:24:25 +0000 +++ src/ChangeLog 2013-12-23 23:53:26 +0000 @@ -1,3 +1,20 @@ +2013-12-23 Paul Eggert + + Plain copy-file no longer chmods an existing destination (Bug#16133). + * fileio.c (realmask): Now a static var, not a local. + (barf_or_query_if_file_exists): New arg KNOWN_TO_EXIST. + Remove arg STATPTR. All uses changed. + (Fcopy_file): Do not alter permissions of existing destinations, + unless PRESERVE-PERMISSIONS (renamed from + PRESERVE-EXTENDED-ATTRIBUTES) is non-nil. + Avoid race when testing for existing destinations and for + when input and output files are the same. + If changing the group fails, adjust both default and + preserved permissions so that access is not granted to the + wrong group. + (Fset_default_file_modes, init_fileio): Update realmask. + (Fdefault_file_modes): Use realmask instead of calling umask. + 2013-12-23 Eli Zaretskii * xdisp.c (tool_bar_height): Use WINDOW_PIXEL_WIDTH to set up the === modified file 'src/fileio.c' --- src/fileio.c 2013-12-17 17:46:31 +0000 +++ src/fileio.c 2013-12-23 23:53:26 +0000 @@ -95,6 +95,9 @@ /* True during writing of auto-save files. */ static bool auto_saving; +/* Emacs's real umask. */ +static mode_t realmask; + /* Nonzero umask during creation of auto-save directories. */ static mode_t auto_saving_dir_umask; @@ -1858,20 +1861,16 @@ } /* Signal an error if the file ABSNAME already exists. + If KNOWN_TO_EXIST, the file is known to exist. + QUERYSTRING is a name for the action that is being considered + to alter the file. If INTERACTIVE, ask the user whether to proceed, and bypass the error if the user says to go ahead. - QUERYSTRING is a name for the action that is being considered - to alter the file. - - *STATPTR is used to store the stat information if the file exists. - If the file does not exist, STATPTR->st_mode is set to 0. - If STATPTR is null, we don't store into it. - If QUICK, ask for y or n, not yes or no. */ static void -barf_or_query_if_file_exists (Lisp_Object absname, const char *querystring, - bool interactive, struct stat *statptr, +barf_or_query_if_file_exists (Lisp_Object absname, bool known_to_exist, + const char *querystring, bool interactive, bool quick) { Lisp_Object tem, encoded_filename; @@ -1880,14 +1879,16 @@ encoded_filename = ENCODE_FILE (absname); - /* `stat' is a good way to tell whether the file exists, - regardless of what access permissions it has. */ - if (lstat (SSDATA (encoded_filename), &statbuf) >= 0) + if (! known_to_exist && lstat (SSDATA (encoded_filename), &statbuf) == 0) { if (S_ISDIR (statbuf.st_mode)) xsignal2 (Qfile_error, build_string ("File is a directory"), absname); + known_to_exist = true; + } + if (known_to_exist) + { if (! interactive) xsignal2 (Qfile_already_exists, build_string ("File already exists"), absname); @@ -1902,15 +1903,7 @@ if (NILP (tem)) xsignal2 (Qfile_already_exists, build_string ("File already exists"), absname); - if (statptr) - *statptr = statbuf; - } - else - { - if (statptr) - statptr->st_mode = 0; - } - return; + } } DEFUN ("copy-file", Fcopy_file, Scopy_file, 2, 6, @@ -1937,16 +1930,21 @@ If PRESERVE-UID-GID is non-nil, we try to transfer the uid and gid of FILE to NEWNAME. -If PRESERVE-EXTENDED-ATTRIBUTES is non-nil, we try to copy additional -attributes of FILE to NEWNAME, such as its SELinux context and ACL -entries (depending on how Emacs was built). */) - (Lisp_Object file, Lisp_Object newname, Lisp_Object ok_if_already_exists, Lisp_Object keep_time, Lisp_Object preserve_uid_gid, Lisp_Object preserve_extended_attributes) +If PRESERVE-PERMISSIONS is non-nil, copy permissions of FILE to NEWNAME; +this includes the file modes, along with ACL entries and SELinux +context if present. Otherwise, if NEWNAME is created its file +permission bits are those of FILE, masked by the default file +permissions. */) + (Lisp_Object file, Lisp_Object newname, Lisp_Object ok_if_already_exists, + Lisp_Object keep_time, Lisp_Object preserve_uid_gid, + Lisp_Object preserve_permissions) { - struct stat out_st; Lisp_Object handler; struct gcpro gcpro1, gcpro2, gcpro3, gcpro4; ptrdiff_t count = SPECPDL_INDEX (); Lisp_Object encoded_file, encoded_newname; + bool already_exists = false; + mode_t new_mask; #if HAVE_LIBSELINUX security_context_t con; int conlength = 0; @@ -1981,22 +1979,20 @@ if (!NILP (handler)) RETURN_UNGCPRO (call7 (handler, Qcopy_file, file, newname, ok_if_already_exists, keep_time, preserve_uid_gid, - preserve_extended_attributes)); + preserve_permissions)); encoded_file = ENCODE_FILE (file); encoded_newname = ENCODE_FILE (newname); +#ifdef WINDOWSNT if (NILP (ok_if_already_exists) || INTEGERP (ok_if_already_exists)) - barf_or_query_if_file_exists (newname, "copy to it", - INTEGERP (ok_if_already_exists), &out_st, 0); - else if (stat (SSDATA (encoded_newname), &out_st) < 0) - out_st.st_mode = 0; + barf_or_query_if_file_exists (newname, false, "copy to it", + INTEGERP (ok_if_already_exists), false); -#ifdef WINDOWSNT result = w32_copy_file (SSDATA (encoded_file), SSDATA (encoded_newname), !NILP (keep_time), !NILP (preserve_uid_gid), - !NILP (preserve_extended_attributes)); + !NILP (preserve_permissions)); switch (result) { case -1: @@ -2022,7 +2018,7 @@ if (fstat (ifd, &st) != 0) report_file_error ("Input file status", file); - if (!NILP (preserve_extended_attributes)) + if (!NILP (preserve_permissions)) { #if HAVE_LIBSELINUX if (is_selinux_enabled ()) @@ -2034,32 +2030,46 @@ #endif } - if (out_st.st_mode != 0 - && st.st_dev == out_st.st_dev && st.st_ino == out_st.st_ino) - report_file_errno ("Input and output files are the same", - list2 (file, newname), 0); - /* We can copy only regular files. */ if (!S_ISREG (st.st_mode)) report_file_errno ("Non-regular file", file, S_ISDIR (st.st_mode) ? EISDIR : EINVAL); - { #ifndef MSDOS - int new_mask = st.st_mode & (!NILP (preserve_uid_gid) ? 0600 : 0666); + new_mask = st.st_mode & 0777; + if (!NILP (preserve_uid_gid) || !NILP (preserve_permissions)) + new_mask &= 0700; #else - int new_mask = S_IREAD | S_IWRITE; + new_mask = S_IREAD | S_IWRITE; #endif - ofd = emacs_open (SSDATA (encoded_newname), - (O_WRONLY | O_TRUNC | O_CREAT - | (NILP (ok_if_already_exists) ? O_EXCL : 0)), - new_mask); - } + + ofd = emacs_open (SSDATA (encoded_newname), O_WRONLY | O_CREAT | O_EXCL, + new_mask); + if (ofd < 0 && errno == EEXIST) + { + if (NILP (ok_if_already_exists) || INTEGERP (ok_if_already_exists)) + barf_or_query_if_file_exists (newname, true, "copy to it", + INTEGERP (ok_if_already_exists), false); + already_exists = true; + ofd = emacs_open (SSDATA (encoded_newname), O_WRONLY, 0); + } if (ofd < 0) report_file_error ("Opening output file", newname); record_unwind_protect_int (close_file_unwind, ofd); + if (already_exists) + { + struct stat out_st; + if (fstat (ofd, &out_st) != 0) + report_file_error ("Output file status", newname); + if (st.st_dev == out_st.st_dev && st.st_ino == out_st.st_ino) + report_file_errno ("Input and output files are the same", + list2 (file, newname), 0); + if (ftruncate (ofd, 0) != 0) + report_file_error ("Truncating output file", newname); + } + immediate_quit = 1; QUIT; while ((n = emacs_read (ifd, buf, sizeof buf)) > 0) @@ -2071,26 +2081,41 @@ /* Preserve the original file permissions, and if requested, also its owner and group. */ { - mode_t mode_mask = 07777; + mode_t preserved_permissions = st.st_mode & 07777; + mode_t default_permissions = st.st_mode & 0777 & ~realmask; if (!NILP (preserve_uid_gid)) { /* Attempt to change owner and group. If that doesn't work attempt to change just the group, as that is sometimes allowed. Adjust the mode mask to eliminate setuid or setgid bits - that are inappropriate if the owner and group are wrong. */ + or group permissions bits that are inappropriate if the + owner or group are wrong. */ if (fchown (ofd, st.st_uid, st.st_gid) != 0) { - mode_mask &= ~06000; if (fchown (ofd, -1, st.st_gid) == 0) - mode_mask |= 02000; + preserved_permissions &= ~04000; + else + { + preserved_permissions &= ~06000; + + /* Copy the other bits to the group bits, since the + group is wrong. */ + preserved_permissions &= ~070; + preserved_permissions |= (preserved_permissions & 7) << 3; + default_permissions &= ~070; + default_permissions |= (default_permissions & 7) << 3; + } } } - switch (!NILP (preserve_extended_attributes) + switch (!NILP (preserve_permissions) ? qcopy_acl (SSDATA (encoded_file), ifd, SSDATA (encoded_newname), ofd, - st.st_mode & mode_mask) - : fchmod (ofd, st.st_mode & mode_mask)) + preserved_permissions) + : (already_exists + || (new_mask & ~realmask) == default_permissions) + ? 0 + : fchmod (ofd, default_permissions)) { case -2: report_file_error ("Copying permissions from", file); case -1: report_file_error ("Copying permissions to", newname); @@ -2307,8 +2332,8 @@ #endif if (NILP (ok_if_already_exists) || INTEGERP (ok_if_already_exists)) - barf_or_query_if_file_exists (newname, "rename to it", - INTEGERP (ok_if_already_exists), 0, 0); + barf_or_query_if_file_exists (newname, false, "rename to it", + INTEGERP (ok_if_already_exists), false); if (rename (SSDATA (encoded_file), SSDATA (encoded_newname)) < 0) { int rename_errno = errno; @@ -2387,8 +2412,8 @@ if (NILP (ok_if_already_exists) || INTEGERP (ok_if_already_exists)) - barf_or_query_if_file_exists (newname, "make it a new name", - INTEGERP (ok_if_already_exists), 0, 0); + barf_or_query_if_file_exists (newname, false, "make it a new name", + INTEGERP (ok_if_already_exists), false); unlink (SSDATA (newname)); if (link (SSDATA (encoded_file), SSDATA (encoded_newname)) < 0) @@ -2449,8 +2474,8 @@ if (NILP (ok_if_already_exists) || INTEGERP (ok_if_already_exists)) - barf_or_query_if_file_exists (linkname, "make it a link", - INTEGERP (ok_if_already_exists), 0, 0); + barf_or_query_if_file_exists (linkname, false, "make it a link", + INTEGERP (ok_if_already_exists), false); if (symlink (SSDATA (encoded_filename), SSDATA (encoded_linkname)) < 0) { /* If we didn't complain already, silently delete existing file. */ @@ -3137,10 +3162,17 @@ This setting is inherited by subprocesses. */) (Lisp_Object mode) { + mode_t oldrealmask, oldumask, newumask; CHECK_NUMBER (mode); - - umask ((~ XINT (mode)) & 0777); - + oldrealmask = realmask; + newumask = ~ XINT (mode) & 0777; + + block_input (); + realmask = newumask; + oldumask = umask (newumask); + unblock_input (); + + eassert (oldumask == oldrealmask); return Qnil; } @@ -3149,14 +3181,7 @@ The value is an integer. */) (void) { - mode_t realmask; Lisp_Object value; - - block_input (); - realmask = umask (0); - umask (realmask); - unblock_input (); - XSETINT (value, (~ realmask) & 0777); return value; } @@ -4697,7 +4722,7 @@ filename = Fexpand_file_name (filename, Qnil); if (!NILP (mustbenew) && !EQ (mustbenew, Qexcl)) - barf_or_query_if_file_exists (filename, "overwrite", 1, 0, 1); + barf_or_query_if_file_exists (filename, false, "overwrite", true, true); if (STRINGP (visit)) visit_file = Fexpand_file_name (visit, Qnil); @@ -5765,6 +5790,9 @@ void init_fileio (void) { + realmask = umask (0); + umask (realmask); + valid_timestamp_file_system = 0; /* fsync can be a significant performance hit. Often it doesn't --------------020404090304010004010701--