Tim Robbins
2002-Dec-03 02:37 UTC
scp "Bad address" errors with strange filesystem block sizes
When copying from a remote host to a local filesystem with a strange block size, allocbuf() in scp.c seems to calculate an incorrect buffer size, causing the copy loop in sink() to write past the end of the buffer. For example, with smbfs, the optimal block size is negotiated when the client connects to the server, and is rarely a power of two. In my case it is 64560. This loop in sink() keeps reading into the buffer until it has read bp->count bytes of data: for (count = i = 0; i < size; i += 4096) { amt = 4096; if (i + amt > size) amt = size - i; count += amt; do { j = read(remin, cp, amt); if (j == -1 && (errno == EINTR || errno == EAGAIN)) { continue; } else if (j <= 0) { run_err("%s", j ? strerror(errno) : "dropped connection"); exit(1); } amt -= j; cp += j; statbytes += j; } while (amt > 0); if (count == bp->cnt) { /* Keep reading so we stay sync'd up. */ if (wrerr == NO) { j = atomicio(write, ofd, bp->buf, count); The problem here is that it requests the data in chunks of 4096 bytes, but the block size is not a multiple of 4096. One of the read() calls eventually fails with EFAULT when "cp" is past the break. This patch makes allocbuf() use roundup() like Berkeley rcp does, instead of the wacky calculation it uses now. The calculated buffer sizes are probably still suboptimal when the file system's optimal block size is not a power of two. Index: scp.c ==================================================================RCS file: /x/freebsd/src/crypto/openssh/scp.c,v retrieving revision 1.1.1.9 diff -u -r1.1.1.9 scp.c --- scp.c 27 Jun 2002 22:31:12 -0000 1.1.1.9 +++ scp.c 3 Dec 2002 01:54:49 -0000 @@ -1030,6 +1030,9 @@ { size_t size; #ifdef HAVE_STRUCT_STAT_ST_BLKSIZE +#ifndef roundup +#define roundup(x, y) ((((x)+((y)-1))/(y))*(y)) /* to any y */ +#endif struct stat stb; if (fstat(fd, &stb) < 0) { @@ -1039,8 +1042,7 @@ if (stb.st_blksize == 0) size = blksize; else - size = blksize + (stb.st_blksize - blksize % stb.st_blksize) % - stb.st_blksize; + size = roundup(stb.st_blksize, blksize); #else /* HAVE_STRUCT_STAT_ST_BLKSIZE */ size = blksize; #endif /* HAVE_STRUCT_STAT_ST_BLKSIZE */ Tim
Markus Friedl
2002-Dec-05 09:46 UTC
scp "Bad address" errors with strange filesystem block sizes
On Tue, Dec 03, 2002 at 01:37:51PM +1100, Tim Robbins wrote:> +#ifndef roundup > +#define roundup(x, y) ((((x)+((y)-1))/(y))*(y)) /* to any y */ > +#endifno need to define roundup here, openssh uses roundup() in other places, too. -m