Bug#447153: /usr/bin/scp: Fails to notice write errors

[Michal Suchanek]

Hello

On 12/11/2007, Colin Watson <cjwatson at debian.org> wrote:

Thanks for looking into this.

> Here's a reduced test program that exhibits the same problem as scp when
> run with a filename on a CIFS mount of a full filesystem:
>
> #include <stdio.h>
> #include <unistd.h>
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <fcntl.h>
> #include <errno.h>
>
> int main(int argc, char **argv)
> {
>     int fd;
>     if (argc <= 1) {
>         fprintf(stderr, "Usage: %s filename\n", argv[0]);
>         return 1;
>     }
>     fd = open(argv[1], O_CREAT | O_WRONLY, 0644);
>     if (fd < 0) {
>         perror("open");
>         return 1;
>     }
>     while (write(fd, "x", 1) < 1) {
>         if (errno == EINTR)
>             continue;
>         perror("write");
>         return 1;
>     }
>     if (ftruncate(fd, 1) < 0) {
>         perror("ftruncate");
>         return 1;
>     }
>     if (close(fd) < 0) {
>         perror("close");
>         return 1;
>     }
>     return 0;
> }
>
> No error, but you end up with a one-byte hole rather than either (a) "x"
> or (b) an error. If you leave out the ftruncate, then close returns
> ENOSPC. In either case, you get "CIFS VFS: Write2 ret -28, written = 0"
> in syslog, but the error code doesn't make it to userspace if there's an
> ftruncate between write and close. Is this a CIFS bug? I can't find
> anything in the ftruncate documentation that suggests it is allowed to
> do this; I think that if write claims to have written all the bytes then
> userspace ought to be able to assume that ftruncate(fd, st_size) is a
> no-op.
>
>
> To openssh-unix-dev: does anyone think this is worth a workaround? The
> ftruncate seems rather unnecessary if we've already written out the
> required number of bytes anyway. I've attached a patch which only does
> it if that isn't the case (although I have some trouble seeing how we
> could ever get to the ftruncate without either writing the required
> number of bytes or encountering a write error). If people think it's a
> good idea I'll file it in Bugzilla.
>

I do not know the scp code so I might be missing something. However,
truncating the file might be necessary when there is already a file,
and it was originally longer.

It looks like a bug either in the kernel or in ftruncate
documentation. It is certainly true that the write error should get
reported at some point if it occurs, and a filesystem that chooses to
not return it on write() should save the errors for close().

However, using ftruncate() on the file does, in some sense,
successfully extend the file to the desired length. It looks like such
mixing of calls was not expected by the fs driver, and may not be well
defined in general. I would suggest closing the file, and if it needs
truncating, reopen and truncate it (or do some voodoo with duplicated
fds if possible).
The file could also be truncated in advance, but that would make
dealing with files that change during transfer even harder. Although
one cannot guarantee any sane results in that case anyway.

Thanks

Michal