I have found that in the static void process_rename(void) function of sftp-server.c when a file being renamed in Linux (and I assume all types on UN*Xs) is mounted on a fat32 partition the rename fails.This may effect other non-posix filesystem files as well. I tracked the problem down to the link(oldpath, newpath) == -1) failing, since the S_ISREG(sb.st_mode) condition is met with a fat32 mounted filesystem file. I found that if the link() fails falling back on rename() works to remedy the problem, but I wasn't sure if this is safe coding.
link() on Linux returns EPERM in that case, which seems to be indistinguishable from a vanilla permission error. (OpenBSD returns EOPNOTSUPP, but the sftp-server problem probably exists there too for filesystems with non-Unix semantics.) I guess if it link() fails because of permissions then rename() will too, so it's probably as safe as the old code (ie prone to whatever races the old code was).
The semantics do differ: rename() will clobber the destination, whereas link() will not. I'm not sure to what extent that this will matter, but I'm sure there would be consequences. I'm not sure how to fix this, given that the Linux semantics don't give us any useful information. They should really to be fixed to return a "not supported" errno.
Well, http://www.ietf.org/internet-drafts/draft-ietf-secsh-filexfer-05.txt has an extra field (compared to filexfer-02) "flags" for rename: 'flags' is 0 or a combination of: SSH_FXP_RENAME_OVERWRITE 0x00000001 SSH_FXP_RENAME_ATOMIC 0x00000002 SSH_FXP_RENAME_NATIVE 0x00000004
That won't help until both our sftp client and sftp-server support filexfer v5, which is a little tricky if one wants to remain backwards compatible. I have a few reservations about v5 which I'd like to see ironed out. Also the pace of change of the filexfer drafts has been relatively fast, so anything implemented would likely bitrot. Personally, I won't be bothering to implement them until things settle down. (Don't let that stop anyone else though)
My point was that in earlier versions (it was introduced in v5) it's unspecified, so for those versions sftp-server can do whatever the heck it likes :-). I've just been reading the draft and it seems like quite a lot of work to implement, even without the backwards compatibility.
Created attachment 596 [details] Make sftp rename fall back if fs doesn't support links (eg fat) This fixes the problem on OpenBSD by falling back to stat+rename if the filesystem doesn't support links. Not sure what to do about it on Linux, though (see djm's comment #2).
Created attachment 597 [details] Possible implementation for Linux Maybe something like this (with the appropriate bits in configure plus attachment #596 [details]) for Linux? I don't think this would be any worse on Linux for filesystems with Unix semantics. You would only fall into the rename() block if the file exists but you don't have perms. If the file still exists at the time of the stat(), nothing happens. If it doesn't, the rename happens. In particular, you can't have the case where a newly-created file is clobbered, because you can't get into the racy section unless you get the EPERM first. Of course, on FAT, all bets are off.
Created attachment 662 [details] Handle Linux EPERM case Attachment id #596 has been committed (will be in tomorrow's snapshot). This patch adds configure-time handling for Linux. It's kinda ugly, but it seems like the least-bad option. It checks for both EOPNOTSUPP and EPERM, so it will work with other filesystems with the same limitations return EOPNOTSUPP (or if FAT changes to saner behaviour).
Attachment #596 [details] or yours, #662?
#596 has been committed, which solves the problem if rename returns EOPNOTSUPP (which Linux doesn't). #662 extends #596 to work on Linux too, and if it is OK it will be committed as well. If you'd like to test it on Linux you'll need to apply both #596 and #662 to 3.8.1p1, or get tomorrow's snapshot and apply just #662.
Attachment #622 [details] has been committed to OpenSSH Portable too, so this should now be fixed for Linux as well. Please try tomorrow's snapshot (ie 20040629 or later).