* "Preserve owner and group" on MSDOS/Windows @ 2005-06-24 9:08 Juanma Barranquero 2005-06-24 11:45 ` Eli Zaretskii 2005-06-24 15:07 ` Race-condition ? (was: "Preserve owner and group" on MSDOS/Windows) Gaëtan LEURENT 0 siblings, 2 replies; 20+ messages in thread From: Juanma Barranquero @ 2005-06-24 9:08 UTC (permalink / raw) This change fails on Windows (no chown function): 2005-06-23 Richard M. Stallman <rms@gnu.org> * fileio.c (Frename_file): Preserve owner and group, if possible, when copying. I've wrapped it with #ifndef DOS_NT. I don't think any other action is needed. -- /L/e/k/t/u ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: "Preserve owner and group" on MSDOS/Windows 2005-06-24 9:08 "Preserve owner and group" on MSDOS/Windows Juanma Barranquero @ 2005-06-24 11:45 ` Eli Zaretskii 2005-06-24 11:02 ` Juanma Barranquero 2005-06-24 15:07 ` Race-condition ? (was: "Preserve owner and group" on MSDOS/Windows) Gaëtan LEURENT 1 sibling, 1 reply; 20+ messages in thread From: Eli Zaretskii @ 2005-06-24 11:45 UTC (permalink / raw) Cc: emacs-devel > Date: Fri, 24 Jun 2005 11:08:20 +0200 > From: Juanma Barranquero <lekktu@gmail.com> > > This change fails on Windows (no chown function): > > 2005-06-23 Richard M. Stallman <rms@gnu.org> > > * fileio.c (Frename_file): Preserve owner and group, if possible, > when copying. > > I've wrapped it with #ifndef DOS_NT. I don't think any other action is needed. I don't think this is the best solution. It is IMHO much better to add to w32.c a trivial `chown' that does nothing; then the ugly ifdef can go away. (The DOS port doesn't need the ifdef anyway, since the DJGPP library already has such a no-op implementation of `chown'.) I think system-dependcent ifdef's should be the last resort in such situations. I've just installed a change along these lines (and undid yours). (A more meaningful implementation of `chown' for w32, that uses GetNamedSecurityInfo and SetNamedSecurityInfo, is left as an exercise to the reader ;-) ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: "Preserve owner and group" on MSDOS/Windows 2005-06-24 11:45 ` Eli Zaretskii @ 2005-06-24 11:02 ` Juanma Barranquero 0 siblings, 0 replies; 20+ messages in thread From: Juanma Barranquero @ 2005-06-24 11:02 UTC (permalink / raw) Cc: emacs-devel > I've just installed a change along these lines (and undid yours). Cool, thanks. Mine was mainly a fix to allow compilation to proceed. > (A more meaningful implementation of `chown' for w32, that uses > GetNamedSecurityInfo and SetNamedSecurityInfo, is left as an exercise > to the reader ;-) I feared someone was going to say that... -- /L/e/k/t/u ^ permalink raw reply [flat|nested] 20+ messages in thread
* Race-condition ? (was: "Preserve owner and group" on MSDOS/Windows) 2005-06-24 9:08 "Preserve owner and group" on MSDOS/Windows Juanma Barranquero 2005-06-24 11:45 ` Eli Zaretskii @ 2005-06-24 15:07 ` Gaëtan LEURENT 2005-06-24 20:07 ` Eli Zaretskii ` (2 more replies) 1 sibling, 3 replies; 20+ messages in thread From: Gaëtan LEURENT @ 2005-06-24 15:07 UTC (permalink / raw) Juanma Barranquero wrote on 24 Jun 2005 11:08:20 +0200: > 2005-06-23 Richard M. Stallman <rms@gnu.org> > > * fileio.c (Frename_file): Preserve owner and group, if possible, > when copying. This is done with a call to chown, and I think this is a source of race-conditions, like the one that was recently discovered in bzip2 (someone could have replaced the file by a link to another file between Fcopy_file and chown). I believe we should use fchown instead. In fileio.c there is also a call to chmod in copy-file which seem to suffer the same problem. This one is also in emacs 21.4. I think lisp calls to set-file-modes should also be checked carefully. -- Gaëtan LEURENT ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Race-condition ? (was: "Preserve owner and group" on MSDOS/Windows) 2005-06-24 15:07 ` Race-condition ? (was: "Preserve owner and group" on MSDOS/Windows) Gaëtan LEURENT @ 2005-06-24 20:07 ` Eli Zaretskii 2005-06-24 20:46 ` Race-condition ? Gaëtan LEURENT 2005-06-24 21:01 ` David Kastrup 2005-06-26 4:46 ` Race-condition ? (was: "Preserve owner and group" on MSDOS/Windows) Richard M. Stallman 2005-06-26 4:46 ` Race-condition ? (was: "Preserve owner and group" on MSDOS/Windows) Richard M. Stallman 2 siblings, 2 replies; 20+ messages in thread From: Eli Zaretskii @ 2005-06-24 20:07 UTC (permalink / raw) > From: gaetan.leurent@ens.fr (=?iso-8859-1?Q?Ga=EBtan?= LEURENT) > Date: Fri, 24 Jun 2005 17:07:57 +0200 > > > * fileio.c (Frename_file): Preserve owner and group, if possible, > > when copying. > > This is done with a call to chown, and I think this is a source of > race-conditions, like the one that was recently discovered in bzip2 > (someone could have replaced the file by a link to another file between > Fcopy_file and chown). So? What problems would that cause, except defeating the call to chown itself? Previous versions of Emacs didn't call chown at all, so how is the current version worse? It's possible that this race condition is harmful in the context of bzip2, but that doesn't necessarily mean it's as harmful in Emacs. > I believe we should use fchown instead. Only if the danger is real, IMHO: fchown requires that we open the file, which is expensive. If we go that way, we might as well check if we are root, and only open the file and call fchown if we are: no need to punish mere mortals if we know in advance the call will fail for them anyway. > In fileio.c there is also a call to chmod in copy-file which seem to > suffer the same problem. This one is also in emacs 21.4. Yeah, and many versions before that. Anyway, how portable are fchown and fchmod? If not all platforms support them, we shouldn't introduce them without an Autoconf test. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Race-condition ? 2005-06-24 20:07 ` Eli Zaretskii @ 2005-06-24 20:46 ` Gaëtan LEURENT 2005-06-25 9:27 ` Eli Zaretskii 2005-06-24 21:01 ` David Kastrup 1 sibling, 1 reply; 20+ messages in thread From: Gaëtan LEURENT @ 2005-06-24 20:46 UTC (permalink / raw) Cc: emacs-devel Eli Zaretskii wrote on 24 Jun 2005 22:07:35 +0200: > So? What problems would that cause, except defeating the call to > chown itself? Previous versions of Emacs didn't call chown at all, so > how is the current version worse? > > It's possible that this race condition is harmful in the context of > bzip2, but that doesn't necessarily mean it's as harmful in Emacs. > >> I believe we should use fchown instead. > > Only if the danger is real, IMHO: fchown requires that we open the > file, which is expensive. If we go that way, we might as well check > if we are root, and only open the file and call fchown if we are: no > need to punish mere mortals if we know in advance the call will fail > for them anyway. I guess I haven't been clear enough. The scenario is: - Suppose the users have two directory in two different drives. For instance in /home which is a small drive with frequent backups, and in /large which is a large drive without backups. - Now, root is using emacs. He decides to move the file /home/joe/somefile to /large/joe/somefile because it takes too much place on the small drive. - Since /home and /large are on different filesystems, emacs will copy the content of /home/joe/somefile to /large/joe/somefile and will then call chown on /large/joe/somefile. - But joe is a bad guy, and while emacs is copying the file, he removes /large/joe/somefile and replaces it with a hardlink to /large/root/importantfile (possibly /etc/passwd if it's on the same drive). - When emacs finishes to copy the file, it call chown on /large/joe/somefile, and joe now owns /large/root/importantfile. The problem is that chown takes a path as argument, and paths are not safe in a Unix environment. What we must do is use fchown which takes a filedescriptor as argument, and give it the fd we got *when we opened the new file in the first place*. We must not close the file before we call fchown, and we must not open it second time. This induce no additional cost and it is the only way to be sure we are dealing with the same file. You can build the same kind of attack with chmod. The attack is not very likely because it requires that the target directory is writable by the victim and by the attacker, which can happen it root is playing with someone else's file or if the victim is doing things in a world writable directory, but it the same as the one that was found in bzip2. > Anyway, how portable are fchown and fchmod? If not all platforms > support them, we shouldn't introduce them without an Autoconf test. According to Linux's man files: # The fchmod call conforms to 4.4BSD and SVr4. SVr4 documents additional # EINTR and ENOLINK error conditions. POSIX requires the fchmod function # if at least one of _POSIX_MAPPED_FILES and _POSIX_SHARED_MEMORY_OBJECTS # is defined, and documents additional ENOSYS and EINVAL error condi- # tions, but does not document EIO. If a platform does not support them, I think we should not try to change access permissions and/or file owner at all on these platforms. -- Gaëtan LEURENT ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Race-condition ? 2005-06-24 20:46 ` Race-condition ? Gaëtan LEURENT @ 2005-06-25 9:27 ` Eli Zaretskii 0 siblings, 0 replies; 20+ messages in thread From: Eli Zaretskii @ 2005-06-25 9:27 UTC (permalink / raw) > Cc: emacs-devel@gnu.org > From: gaetan.leurent@ens.fr (=?iso-8859-1?Q?Ga=EBtan?= LEURENT) > Date: Fri, 24 Jun 2005 22:46:59 +0200 > > I guess I haven't been clear enough. The scenario is: No, you were very clear. I still am not sure whether the danger is real, but then I'm not an expert on security. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Race-condition ? 2005-06-24 20:07 ` Eli Zaretskii 2005-06-24 20:46 ` Race-condition ? Gaëtan LEURENT @ 2005-06-24 21:01 ` David Kastrup 2005-06-25 22:25 ` Gaëtan LEURENT 1 sibling, 1 reply; 20+ messages in thread From: David Kastrup @ 2005-06-24 21:01 UTC (permalink / raw) Cc: emacs-devel Eli Zaretskii <eliz@gnu.org> writes: >> From: gaetan.leurent@ens.fr (=?iso-8859-1?Q?Ga=EBtan?= LEURENT) >> Date: Fri, 24 Jun 2005 17:07:57 +0200 >> >> > * fileio.c (Frename_file): Preserve owner and group, if possible, >> > when copying. >> >> This is done with a call to chown, and I think this is a source of >> race-conditions, like the one that was recently discovered in bzip2 >> (someone could have replaced the file by a link to another file between >> Fcopy_file and chown). > > So? What problems would that cause, except defeating the call to > chown itself? Previous versions of Emacs didn't call chown at all, so > how is the current version worse? > > It's possible that this race condition is harmful in the context of > bzip2, but that doesn't necessarily mean it's as harmful in Emacs. > >> I believe we should use fchown instead. > > Only if the danger is real, IMHO: fchown requires that we open the > file, which is expensive. If we go that way, we might as well check > if we are root, and only open the file and call fchown if we are: no > need to punish mere mortals if we know in advance the call will fail > for them anyway. > >> In fileio.c there is also a call to chmod in copy-file which seem to >> suffer the same problem. This one is also in emacs 21.4. > > Yeah, and many versions before that. > > Anyway, how portable are fchown and fchmod? If not all platforms > support them, we shouldn't introduce them without an Autoconf test. I fail to see the advantage of using chown, or using fopen and fchown. In both cases the file name can be changed to refer to something else before the operation starts. The only situation where fchown offers any advantage is where you _already_ have a file open, like when you write the file after fopen, and then change its permissions. That is, the owner change must be accomplished in something like write-region, or it is pointless. As an isolated operation, fopen/fchown offers no advantage whatsoever. You could conceivable to fopen, followed by fstat, check that you are not talking about a symbolic link, and only then to fchown, and this would be safe against symlink attacks. But apart from that, I see no specific advantage. -- David Kastrup, Kriemhildstr. 15, 44793 Bochum ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Race-condition ? 2005-06-24 21:01 ` David Kastrup @ 2005-06-25 22:25 ` Gaëtan LEURENT 2005-06-26 18:48 ` David Kastrup 0 siblings, 1 reply; 20+ messages in thread From: Gaëtan LEURENT @ 2005-06-25 22:25 UTC (permalink / raw) Cc: Eli Zaretskii, emacs-devel David Kastrup wrote on 24 Jun 2005 23:01:50 +0200: > I fail to see the advantage of using chown, or using fopen and > fchown. In both cases the file name can be changed to refer to > something else before the operation starts. > > The only situation where fchown offers any advantage is where you > _already_ have a file open, like when you write the file after fopen, > and then change its permissions. Yes, that is true. But we are exactly in the situation were fchown is useful. > That is, the owner change must be accomplished in something like > write-region, or it is pointless. As an isolated operation, > fopen/fchown offers no advantage whatsoever. Yes, fopen/fchown would be the same as calling chown. But here we have just created a new file, and we want to change it's owner, so this should be done with fchown. The code is: | Fcopy_file (file, newname, | /* We have already prompted if it was an integer, | so don't have copy-file prompt again. */ | NILP (ok_if_already_exists) ? Qnil : Qt, | Qt, Qnil); | | /* Preserve owner and group, if possible (if we are root). */ | if (stat (SDATA (encoded_file), &data) >= 0) | chown (SDATA (encoded_file), data.st_uid, data.st_gid); | | Fdelete_file (file); So, we first open a file and write something in it inside Fcopy_file, then we call chown on the same path. But the file that is at this path could be something else that what the we created in Fcopy_file. We should keep that file open, and call fchown on its filedescriptor, instead of calling chown on it's path. This will need some change in Fcopy_file, to be able to do something with the filedescriptor before the file is closed; the most easy way seems to make a subroutine that does the job of Fcopy_file and returns the fd before closing it, and use this subroutine both in Fcopy_file and here in Frename_file. We also have the same kind of thing inside Fcopy_file (and this one is also in stable emacs) where we call chmod on the file path just after closing the file. [I have replaced some part of the code with //... to make it shorter] | #ifdef VMS | // ... | #else | #ifdef MSDOS | // ... | #else /* not MSDOS */ | ofd = emacs_open (SDATA (encoded_newname), | O_WRONLY | O_TRUNC | O_CREAT | | (EQ (mustbenew, Qexcl) ? O_EXCL : 0), | 0666); | #endif /* not MSDOS */ | #endif /* VMS */ | if (ofd < 0) | report_file_error ("Opening output file", Fcons (newname, Qnil)); | | record_unwind_protect (close_file_unwind, make_number (ofd)); | | immediate_quit = 1; | QUIT; | while ((n = emacs_read (ifd, buf, sizeof buf)) > 0) | if (emacs_write (ofd, buf, n) != n) | report_file_error ("I/O error", Fcons (newname, Qnil)); | immediate_quit = 0; | | /* Closing the output clobbers the file times on some systems. */ | if (emacs_close (ofd) < 0) | report_file_error ("I/O error", Fcons (newname, Qnil)); | | if (input_file_statable_p) | { | if (!NILP (keep_time)) | { | // ... | } | #ifndef MSDOS | chmod (SDATA (encoded_newname), st.st_mode & 07777); | #else /* MSDOS */ | // ... | #endif /* MSDOS */ | } Here the change is very easy: we should just do fchmod(ofd,...) before emacs_close(ofd). -- Gaëtan LEURENT ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Race-condition ? 2005-06-25 22:25 ` Gaëtan LEURENT @ 2005-06-26 18:48 ` David Kastrup 0 siblings, 0 replies; 20+ messages in thread From: David Kastrup @ 2005-06-26 18:48 UTC (permalink / raw) Cc: Eli Zaretskii gaetan.leurent@ens.fr (Gaëtan LEURENT) writes: > David Kastrup wrote on 24 Jun 2005 23:01:50 +0200: > >> I fail to see the advantage of using chown, or using fopen and >> fchown. In both cases the file name can be changed to refer to >> something else before the operation starts. >> >> The only situation where fchown offers any advantage is where you >> _already_ have a file open, like when you write the file after fopen, >> and then change its permissions. > > Yes, that is true. But we are exactly in the situation were fchown is > useful. I saw the detailed description afterwards. You are right. fchown would be desirable to use where available. -- David Kastrup, Kriemhildstr. 15, 44793 Bochum ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Race-condition ? (was: "Preserve owner and group" on MSDOS/Windows) 2005-06-24 15:07 ` Race-condition ? (was: "Preserve owner and group" on MSDOS/Windows) Gaëtan LEURENT 2005-06-24 20:07 ` Eli Zaretskii @ 2005-06-26 4:46 ` Richard M. Stallman 2005-06-26 19:52 ` Race-condition ? Gaëtan LEURENT 2005-06-26 4:46 ` Race-condition ? (was: "Preserve owner and group" on MSDOS/Windows) Richard M. Stallman 2 siblings, 1 reply; 20+ messages in thread From: Richard M. Stallman @ 2005-06-26 4:46 UTC (permalink / raw) Cc: emacs-devel I think lisp calls to set-file-modes should also be checked carefully. Would you like to check some of them? The crucial question is, does a call to set-file-modes introduce a worse problem than what existed anyway. For instance, if someone could put a hardlink where you will chmod it, could he also put a hardlink where you will write the contents of the file? In cases where the latter problem exists, adding the former problem to it does not really make things any worse. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Race-condition ? 2005-06-26 4:46 ` Race-condition ? (was: "Preserve owner and group" on MSDOS/Windows) Richard M. Stallman @ 2005-06-26 19:52 ` Gaëtan LEURENT 2005-06-27 5:38 ` Richard M. Stallman 0 siblings, 1 reply; 20+ messages in thread From: Gaëtan LEURENT @ 2005-06-26 19:52 UTC (permalink / raw) Cc: emacs-devel Richard M. Stallman wrote on 26 Jun 2005 06:46:13 +0200: > Would you like to check some of them? I'll look at it. > The crucial question is, does a call to set-file-modes introduce a > worse problem than what existed anyway. For instance, if someone > could put a hardlink where you will chmod it, could he also put a > hardlink where you will write the contents of the file? Yes, that needs to be checked in each case. In the case of copy_file, someone could put a hardlink when you write the file, but then emacs should warn you that the file already exist (you should never overwrite files in public writable directories because it is unsafe -- you must first delete the file). In fact, by looking again at the code of Fcopy_file, I see that the way emacs does it is also vulnerable to race-conditions attack: we first check if the file exist with barf_or_query_if_file_exists, and then we open the file (it is also done in a few other functions in fileio.c). If the file was created in-between, it will be overwritten without any warning. The correct way to do this is to open the file with O_CREAT|O_EXCL and ask the user what to do if it fails with EEXIST. As far as I know, it's the only way to check for the existence of a file and create it atomically. I don't know how we should fix this one, maybe by replacing barf_or_query_if_file_exists with some open_and_barf_or_query_if_file_exists ? -- Gaëtan LEURENT ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Race-condition ? 2005-06-26 19:52 ` Race-condition ? Gaëtan LEURENT @ 2005-06-27 5:38 ` Richard M. Stallman 2005-06-28 22:57 ` Gaëtan LEURENT 0 siblings, 1 reply; 20+ messages in thread From: Richard M. Stallman @ 2005-06-27 5:38 UTC (permalink / raw) Cc: emacs-devel I don't know how we should fix this one, maybe by replacing barf_or_query_if_file_exists with some open_and_barf_or_query_if_file_exists ? I think it may be too difficult to fix all the places that do this, but would you like to try fixing one or two as a first step? ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Race-condition ? 2005-06-27 5:38 ` Richard M. Stallman @ 2005-06-28 22:57 ` Gaëtan LEURENT 2005-06-28 23:50 ` Lennart Borgman 2005-07-03 15:49 ` Richard M. Stallman 0 siblings, 2 replies; 20+ messages in thread From: Gaëtan LEURENT @ 2005-06-28 22:57 UTC (permalink / raw) Cc: emacs-devel Richard M. Stallman wrote on 27 Jun 2005 07:38:04 +0200: > I think it may be too difficult to fix all the places > that do this, but would you like to try fixing one or two > as a first step? In fact, doing the right thing is not so obvious as I thought. I've looked at various implementations of mv and cp (from GNU, OpenSolaris, OpenBSD and FreeBSD) and at the Single Unix Specification, and almost each one uses a subtly different way, and they all allow some kind of race-condition. Some (including GNU mv and GNU cp) do even use chown and chmod in the same unsafe way that we did previously, so I will report that to them. Now, as for as I can tell, there is no way to move a file asking the user what to do if needed and avoid every race-conditions in a Unix-like OS. I will try to come with a solution that avoids the worse problems. An other question is to decide what to do when the destination file exists: we can either overwrite it or remove it and create a new one. Those two ways will have different effects if the file we overwrite/remove has a link count > 1. Current emacs implementation use "overwrite mode" in copy and "remove mode" in rename if it is in the same filesystem, but cross-filesystem rename use the "overwrite mode". We should maybe do something more consistent. FWIW, the Single Unix specification asks that mv removes the file, and cp overwrites it (but some implementation don't follow strictly this rule). In the case of copy, I think we can do it without race-conditions using open in O_EXCL|O_CREAT mode, and if it fails, using unlink and trying again. That would mean to use the "remove mode". In fact the "overwrite mode" is unsafe as soon as someone can replace the file with a hardlink to something else. -- Gaëtan LEURENT ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Race-condition ? 2005-06-28 22:57 ` Gaëtan LEURENT @ 2005-06-28 23:50 ` Lennart Borgman 2005-06-30 5:42 ` David Kastrup 2005-07-03 15:49 ` Richard M. Stallman 1 sibling, 1 reply; 20+ messages in thread From: Lennart Borgman @ 2005-06-28 23:50 UTC (permalink / raw) Gaëtan LEURENT wrote: >Now, as for as I can tell, there is no way to move a file asking the >user what to do if needed and avoid every race-conditions in a Unix-like >OS. I will try to come with a solution that avoids the worse problems. > > That reminds me of that I long ago read someone who found that the most interesting piece of programming he had ever done was for implementing file move. Unfortunately I do not remember who or where I read this - of course... ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Race-condition ? 2005-06-28 23:50 ` Lennart Borgman @ 2005-06-30 5:42 ` David Kastrup 0 siblings, 0 replies; 20+ messages in thread From: David Kastrup @ 2005-06-30 5:42 UTC (permalink / raw) Cc: emacs-devel Lennart Borgman <lennart.borgman.073@student.lu.se> writes: > Gaëtan LEURENT wrote: > >>Now, as for as I can tell, there is no way to move a file asking the >>user what to do if needed and avoid every race-conditions in a Unix-like >>OS. I will try to come with a solution that avoids the worse problems. >> >> > That reminds me of that I long ago read someone who found that the > most interesting piece of programming he had ever done was for > implementing file move. Unfortunately I do not remember who or where I > read this - of course... movemail in Emacs? -- David Kastrup, Kriemhildstr. 15, 44793 Bochum ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Race-condition ? 2005-06-28 22:57 ` Gaëtan LEURENT 2005-06-28 23:50 ` Lennart Borgman @ 2005-07-03 15:49 ` Richard M. Stallman 1 sibling, 0 replies; 20+ messages in thread From: Richard M. Stallman @ 2005-07-03 15:49 UTC (permalink / raw) Cc: emacs-devel Some (including GNU mv and GNU cp) do even use chown and chmod in the same unsafe way that we did previously, so I will report that to them. Thank you. Now, as for as I can tell, there is no way to move a file asking the user what to do if needed and avoid every race-conditions in a Unix-like OS. I will try to come with a solution that avoids the worse problems. That is good. Those two ways will have different effects if the file we overwrite/remove has a link count > 1. Current emacs implementation use "overwrite mode" in copy and "remove mode" in rename if it is in the same filesystem, Those are clearly the right things. copy-file is defined to overwrite an existing file; it would be wrong to do anything else (at least in the default case). Rename, in the case where it is implemented by the rename system call, will replace the existing file. but cross-filesystem rename use the "overwrite mode". In principle, that ought to replace the existing file, to be consistent with other cases of renaming. The easiest way might be to create an option (or reuse an existing option) in copy-file, so that rename-file can continue to handle this case by calling copy-file. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Race-condition ? (was: "Preserve owner and group" on MSDOS/Windows) 2005-06-24 15:07 ` Race-condition ? (was: "Preserve owner and group" on MSDOS/Windows) Gaëtan LEURENT 2005-06-24 20:07 ` Eli Zaretskii 2005-06-26 4:46 ` Race-condition ? (was: "Preserve owner and group" on MSDOS/Windows) Richard M. Stallman @ 2005-06-26 4:46 ` Richard M. Stallman 2005-06-26 17:12 ` Race-condition ? Gaëtan LEURENT 2005-06-26 18:59 ` Race-condition ? (was: "Preserve owner and group" on MSDOS/Windows) Eli Zaretskii 2 siblings, 2 replies; 20+ messages in thread From: Richard M. Stallman @ 2005-06-26 4:46 UTC (permalink / raw) Cc: emacs-devel How about this fix? *** fileio.c 24 Jun 2005 15:43:20 -0400 1.547 --- fileio.c 25 Jun 2005 16:56:28 -0400 *************** *** 2406,2412 **** return; } ! DEFUN ("copy-file", Fcopy_file, Scopy_file, 2, 5, "fCopy file: \nGCopy %s to file: \np\nP", doc: /* Copy FILE to NEWNAME. Both args must be strings. If NEWNAME names a directory, copy FILE there. --- 2406,2412 ---- return; } ! DEFUN ("copy-file", Fcopy_file, Scopy_file, 2, 6, "fCopy file: \nGCopy %s to file: \np\nP", doc: /* Copy FILE to NEWNAME. Both args must be strings. If NEWNAME names a directory, copy FILE there. *************** *** 2426,2434 **** that means to get an error if the file already exists; never overwrite. If MUSTBENEW is neither nil nor `excl', that means ask for confirmation before overwriting, but do go ahead and overwrite the file ! if the user confirms. */) ! (file, newname, ok_if_already_exists, keep_time, mustbenew) Lisp_Object file, newname, ok_if_already_exists, keep_time, mustbenew; { int ifd, ofd, n; char buf[16 * 1024]; --- 2426,2438 ---- that means to get an error if the file already exists; never overwrite. If MUSTBENEW is neither nil nor `excl', that means ask for confirmation before overwriting, but do go ahead and overwrite the file ! if the user confirms. ! ! If PRESERVE-UID-GID is non-nil, we try to transfer the ! uid and gid of FILE to NEWNAME. */) ! (file, newname, ok_if_already_exists, keep_time, mustbenew, preserve_uid_gid) Lisp_Object file, newname, ok_if_already_exists, keep_time, mustbenew; + Lisp_Object preserve_uid_gid; { int ifd, ofd, n; char buf[16 * 1024]; *************** *** 2570,2575 **** --- 2574,2599 ---- report_file_error ("I/O error", Fcons (newname, Qnil)); immediate_quit = 0; + /* Preserve the owner and group, if requested. */ + if (input_file_statable_p && ! NILP (preserve_uid_gid)) + fchown (ofd, st.st_uid, st.st_gid); + + if (input_file_statable_p) + { + #ifndef MSDOS + fchmod (ofd, st.st_mode & 07777); + #else /* MSDOS */ + #if defined (__DJGPP__) && __DJGPP__ > 1 + /* In DJGPP v2.0 and later, fstat usually returns true file mode bits, + and if it can't, it tells so. Otherwise, under MSDOS we usually + get only the READ bit, which will make the copied file read-only, + so it's better not to chmod at all. */ + if ((_djstat_flags & _STFAIL_WRITEBIT) == 0) + chmod (SDATA (encoded_newname), st.st_mode & 07777); + #endif /* DJGPP version 2 or newer */ + #endif /* MSDOS */ + } + /* Closing the output clobbers the file times on some systems. */ if (emacs_close (ofd) < 0) report_file_error ("I/O error", Fcons (newname, Qnil)); *************** *** 2587,2604 **** Fcons (build_string ("Cannot set file date"), Fcons (newname, Qnil))); } - #ifndef MSDOS - chmod (SDATA (encoded_newname), st.st_mode & 07777); - #else /* MSDOS */ - #if defined (__DJGPP__) && __DJGPP__ > 1 - /* In DJGPP v2.0 and later, fstat usually returns true file mode bits, - and if it can't, it tells so. Otherwise, under MSDOS we usually - get only the READ bit, which will make the copied file read-only, - so it's better not to chmod at all. */ - if ((_djstat_flags & _STFAIL_WRITEBIT) == 0) - chmod (SDATA (encoded_newname), st.st_mode & 07777); - #endif /* DJGPP version 2 or newer */ - #endif /* MSDOS */ } emacs_close (ifd); --- 2611,2616 ---- *************** *** 2775,2781 **** { if (errno == EXDEV) { - struct stat data; #ifdef S_IFLNK symlink_target = Ffile_symlink_p (file); if (! NILP (symlink_target)) --- 2787,2792 ---- *************** *** 2783,2797 **** NILP (ok_if_already_exists) ? Qnil : Qt); else #endif ! Fcopy_file (file, newname, ! /* We have already prompted if it was an integer, ! so don't have copy-file prompt again. */ ! NILP (ok_if_already_exists) ? Qnil : Qt, ! Qt, Qnil); ! ! /* Preserve owner and group, if possible (if we are root). */ ! if (stat (SDATA (encoded_file), &data) >= 0) ! chown (SDATA (encoded_file), data.st_uid, data.st_gid); Fdelete_file (file); } --- 2794,2804 ---- NILP (ok_if_already_exists) ? Qnil : Qt); else #endif ! Fcopy_file (file, newname, ! /* We have already prompted if it was an integer, ! so don't have copy-file prompt again. */ ! NILP (ok_if_already_exists) ? Qnil : Qt, ! Qt, Qnil, Qt); Fdelete_file (file); } ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Race-condition ? 2005-06-26 4:46 ` Race-condition ? (was: "Preserve owner and group" on MSDOS/Windows) Richard M. Stallman @ 2005-06-26 17:12 ` Gaëtan LEURENT 2005-06-26 18:59 ` Race-condition ? (was: "Preserve owner and group" on MSDOS/Windows) Eli Zaretskii 1 sibling, 0 replies; 20+ messages in thread From: Gaëtan LEURENT @ 2005-06-26 17:12 UTC (permalink / raw) Cc: emacs-devel Richard M. Stallman wrote on 26 Jun 2005 06:46:54 +0200: > How about this fix? It seems fine to me. -- Gaëtan LEURENT ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Race-condition ? (was: "Preserve owner and group" on MSDOS/Windows) 2005-06-26 4:46 ` Race-condition ? (was: "Preserve owner and group" on MSDOS/Windows) Richard M. Stallman 2005-06-26 17:12 ` Race-condition ? Gaëtan LEURENT @ 2005-06-26 18:59 ` Eli Zaretskii 1 sibling, 0 replies; 20+ messages in thread From: Eli Zaretskii @ 2005-06-26 18:59 UTC (permalink / raw) Cc: gaetan.leurent, emacs-devel > From: "Richard M. Stallman" <rms@gnu.org> > Date: Sun, 26 Jun 2005 00:46:54 -0400 > Cc: emacs-devel@gnu.org > > How about this fix? > > *** fileio.c 24 Jun 2005 15:43:20 -0400 1.547 > --- fileio.c 25 Jun 2005 16:56:28 -0400 This change will do Bad Things on MSDOS, because it calls chmod before closing the file. And fchown needs a similar trick on w32 as did chown. I will fix these problems unless someone beats me to it. ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2005-07-03 15:49 UTC | newest] Thread overview: 20+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2005-06-24 9:08 "Preserve owner and group" on MSDOS/Windows Juanma Barranquero 2005-06-24 11:45 ` Eli Zaretskii 2005-06-24 11:02 ` Juanma Barranquero 2005-06-24 15:07 ` Race-condition ? (was: "Preserve owner and group" on MSDOS/Windows) Gaëtan LEURENT 2005-06-24 20:07 ` Eli Zaretskii 2005-06-24 20:46 ` Race-condition ? Gaëtan LEURENT 2005-06-25 9:27 ` Eli Zaretskii 2005-06-24 21:01 ` David Kastrup 2005-06-25 22:25 ` Gaëtan LEURENT 2005-06-26 18:48 ` David Kastrup 2005-06-26 4:46 ` Race-condition ? (was: "Preserve owner and group" on MSDOS/Windows) Richard M. Stallman 2005-06-26 19:52 ` Race-condition ? Gaëtan LEURENT 2005-06-27 5:38 ` Richard M. Stallman 2005-06-28 22:57 ` Gaëtan LEURENT 2005-06-28 23:50 ` Lennart Borgman 2005-06-30 5:42 ` David Kastrup 2005-07-03 15:49 ` Richard M. Stallman 2005-06-26 4:46 ` Race-condition ? (was: "Preserve owner and group" on MSDOS/Windows) Richard M. Stallman 2005-06-26 17:12 ` Race-condition ? Gaëtan LEURENT 2005-06-26 18:59 ` Race-condition ? (was: "Preserve owner and group" on MSDOS/Windows) Eli Zaretskii
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).