Is there an alternative to using link(2) to rename files in sftp-server? Some users use sftp to upload files to a vfat partition on an sftp-server, and then renaming doesn't work. This breaks konqueror, for example (from KDE, which u), which upload files first with a ".part" extension and then renames them removing this extension.
Andreas wrote:> Is there an alternative to using link(2) to rename files in sftp-server?There's an open bug for this: http://bugzilla.mindrot.org/show_bug.cgi?id=823 According to the CVS log, the link shuffle is used to "fix races in rename/symlink" (revs 1.46 and 1.44). -- Darren Tucker (dtucker at zip.com.au) GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4 37C9 C982 80C7 8FF4 FA69 Good judgement comes with experience. Unfortunately, the experience usually comes from bad judgement.
--On Tuesday, April 06, 2004 09:24:31 +1000 Darren Tucker <dtucker at zip.com.au> wrote:> According to the CVS log, the link shuffle is used to "fix races in > rename/symlink" (revs 1.46 and 1.44).If rename() has a race condition, the OS is broken. Plain and simple. -- Carson
Carson Gaspar wrote:> --On Tuesday, April 06, 2004 09:24:31 +1000 Darren Tucker > >> According to the CVS log, the link shuffle is used to "fix races in >> rename/symlink" (revs 1.46 and 1.44). > > If rename() has a race condition, the OS is broken. Plain and simple.The original code for rename looked like the following: if (stat(newpath, &st) == -1) { ret = rename(oldpath, newpath); status = (ret == -1) ? errno_to_portable(errno) : SSH2_FX_OK; } The idea is obviously to not clobber existing files, but the implementation is racy (hence the change, I guess). I have no idea how to implement that portably for filesystems without Unix semantics without the race. -- Darren Tucker (dtucker at zip.com.au) GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4 37C9 C982 80C7 8FF4 FA69 Good judgement comes with experience. Unfortunately, the experience usually comes from bad judgement.
On Mon, Apr 05, 2004 at 09:45:05PM -0400, Carson Gaspar wrote:> --On Tuesday, April 06, 2004 09:24:31 +1000 Darren Tucker > <dtucker at zip.com.au> wrote: > > >According to the CVS log, the link shuffle is used to "fix races in > >rename/symlink" (revs 1.46 and 1.44). > > If rename() has a race condition, the OS is broken. Plain and simple.not rename, but stat+rename.
> The original code for rename looked like the following: > if (stat(newpath, &st) == -1) { > ret = rename(oldpath, newpath); > status = (ret == -1) ? errno_to_portable(errno) : SSH2_FX_OK; > } > > The idea is obviously to not clobber existing files, but the > implementation is racy (hence the change, I guess). > > I have no idea how to implement that portably for filesystems without > Unix semantics without the race.i think we should 1) break the specification and just do rename(), or 2) add a different message type, that just does rename().
Andreas wrote:> Is there an alternative to using link(2) to rename files in sftp-server? > > Some users use sftp to upload files to a vfat partition on an sftp-server, > and then renaming doesn't work. This breaks konqueror, for example (from KDE, > which u), which upload files first with a ".part" extension and then renames > them removing this extension.See the discussion and patches in http://bugzilla.mindrot.org/show_bug.cgi?id=823 The Linux behaviour of returning EPERM is just broken IMO. It should be something like EOPNOTSUPP. -d
On Tue, 6 Apr 2004, Darren Tucker wrote:> Carson Gaspar wrote: > > > --On Tuesday, April 06, 2004 09:24:31 +1000 Darren Tucker > > > >> According to the CVS log, the link shuffle is used to "fix races in > >> rename/symlink" (revs 1.46 and 1.44). > > > > If rename() has a race condition, the OS is broken. Plain and simple. > > The original code for rename looked like the following: > if (stat(newpath, &st) == -1) { > ret = rename(oldpath, newpath); > status = (ret == -1) ? errno_to_portable(errno) : SSH2_FX_OK; > } > > The idea is obviously to not clobber existing files, but the > implementation is racy (hence the change, I guess). > > I have no idea how to implement that portably for filesystems without > Unix semantics without the race. >May be the following code could be used: if ((ret = open(newpath, O_WRONLY|O_CREAT|O_EXCL, S_IRUSR)) != -1) { close(ret); ret = rename(oldpath, newpath); status = (ret == -1) ? errno_to_portable(errno) : SSH2_FX_OK; } Of course, someone could modify temporary file mode and write something into it between calls to open() and rename() are made, but does somebody really care about that case? -- Sincerely Your, Dan.
In a single word.. "yes". Your code is no better than stat()+rename(). Someone can STILL replace the file from under you. Which is what the race condition we are trying to avoid. - Ben On Tue, 6 Apr 2004, Dan Yefimov wrote:> On Tue, 6 Apr 2004, Darren Tucker wrote: > > > Carson Gaspar wrote: > > > > > --On Tuesday, April 06, 2004 09:24:31 +1000 Darren Tucker > > > > > >> According to the CVS log, the link shuffle is used to "fix races in > > >> rename/symlink" (revs 1.46 and 1.44). > > > > > > If rename() has a race condition, the OS is broken. Plain and simple. > > > > The original code for rename looked like the following: > > if (stat(newpath, &st) == -1) { > > ret = rename(oldpath, newpath); > > status = (ret == -1) ? errno_to_portable(errno) : SSH2_FX_OK; > > } > > > > The idea is obviously to not clobber existing files, but the > > implementation is racy (hence the change, I guess). > > > > I have no idea how to implement that portably for filesystems without > > Unix semantics without the race. > > > May be the following code could be used: > > if ((ret = open(newpath, O_WRONLY|O_CREAT|O_EXCL, S_IRUSR)) != -1) { > close(ret); > ret = rename(oldpath, newpath); > status = (ret == -1) ? errno_to_portable(errno) : SSH2_FX_OK; > } > > Of course, someone could modify temporary file mode and write something into it > between calls to open() and rename() are made, but does somebody really care > about that case? > -- > > Sincerely Your, Dan. > > _______________________________________________ > openssh-unix-dev mailing list > openssh-unix-dev at mindrot.org > http://www.mindrot.org/mailman/listinfo/openssh-unix-dev >
On Tue, Apr 06, 2004 at 05:16:54PM +0400, Dan Yefimov wrote:> May be the following code could be used: > > if ((ret = open(newpath, O_WRONLY|O_CREAT|O_EXCL, S_IRUSR)) != -1) { > close(ret); > ret = rename(oldpath, newpath); > status = (ret == -1) ? errno_to_portable(errno) : SSH2_FX_OK; > } > > Of course, someone could modify temporary file mode and write something into it > between calls to open() and rename() are made, but does somebody really care > about that case?this contains the same race as the original code.