diff --git a/CHANGES b/CHANGES index 85a925b..fbeb126 100644 --- a/CHANGES +++ b/CHANGES @@ -56,6 +56,14 @@ corrections: Marcus Meissner provided a patch that fixes invalid output and possible process crash when socat prints info about an unnamed unix domain socket + + Michal Soltys reported the following problem and provided an initial + patch: when socat was interrupted, e.g. by SIGSTOP, and resumed during + data transfer only parts of the data might have been written. + + Option o-nonblock in combination with large transfer block sizes + may result in partial writes and/or EAGAIN errors that were not handled + properly but resulted in data loss or process termination. docu mentions option so-bindtodev but correct name is so-bindtodevice. Thanks to Jim Zimmerman for reporting. diff --git a/sysutils.c b/sysutils.c index 87c0079..1adfd9e 100644 --- a/sysutils.c +++ b/sysutils.c @@ -16,6 +16,40 @@ #include "utils.h" #include "sysutils.h" +/* Substitute for Write(): + Try to write all bytes before returning; this handles EINTR, + EAGAIN/EWOULDBLOCK, and partial write situations. The drawback is that this + function might block even with O_NONBLOCK option. + Returns <0 on unhandled error, errno valid + Will only return <0 or bytes +*/ +ssize_t writefull(int fd, const void *buff, size_t bytes) { + size_t writt = 0; + ssize_t chk; + while (1) { + chk = Write(fd, (const char *)buff + writt, bytes - writt); + if (chk < 0) { + switch (errno) { + case EINTR: + case EAGAIN: +#if EAGAIN != EWOULDBLOCK + case EWOULDBLOCK: +#endif + Warn4("write(%d, %p, "F_Zu"): %s", fd, (const char *)buff+writt, bytes-writt, strerror(errno)); + Sleep(1); continue; + default: return -1; + } + } else if (writt+chk < bytes) { + Warn4("write(%d, %p, "F_Zu"): only wrote "F_Zu" bytes, trying to continue ", + fd, (const char *)buff+writt, bytes-writt, chk); + writt += chk; + } else { + writt = bytes; + break; + } + } + return writt; +} #if WITH_UNIX void socket_un_init(struct sockaddr_un *sa) { diff --git a/sysutils.h b/sysutils.h index 380552e..dc1dd44 100644 --- a/sysutils.h +++ b/sysutils.h @@ -40,6 +40,8 @@ struct xiorange { union sockaddr_union netmask; } ; #endif /* _WITH_SOCKET */ + +extern ssize_t writefull(int fd, const void *buff, size_t bytes); #if _WITH_SOCKET extern socklen_t socket_init(int af, union sockaddr_union *sa); diff --git a/test.sh b/test.sh index b388b70..a82f9db 100755 --- a/test.sh +++ b/test.sh @@ -10867,6 +10867,50 @@ fi # NUMCOND esac N=$((N+1)) + +# incomplete writes were reported but led to data loss +NAME=INCOMPLETE_WRITE +case "$TESTS" in +*%functions%*|*%bugs%*|*%$NAME%*) +TEST="$NAME: check if incomplete writes are handled properly" +# write to a nonblocking fd a block that is too large for atomic write +# and check if all data arrives +if ! eval $NUMCOND; then :; else +tf="$td/test$N.stdout" +te="$td/test$N.stderr" +tp="$td/test$N.pipe" +tw="$td/test$N.wc-c" +# this is the size we write() in one call; data is never stored on disk, so +# make it large enough to exceed any atomic write size; but higher number might +# take much time +bytes=100000 # for Linux 2.6.? this must be >65536 +CMD0="$SOCAT $opts -u PIPE:$tp STDOUT" +CMD1="$SOCAT $opts -u -b $bytes OPEN:/dev/zero,readbytes=$bytes FILE:$tp,o-nonblock" +printf "test $F_n $TEST... " $N +$CMD0 2>"${te}0" |wc -c >"$tw" & +pid=$! +waitfile "$tp" +$CMD1 2>"${te}1" >"${tf}1" +rc1=$? +wait +if [ $rc1 -ne 0 ]; then + $PRINTF "$NO_RESULT\n" + numCANT=$((numCANT+1)) +elif [ ! -e "$tw" ]; then + $PRINTF "$NO_RESULT\n" + numCANT=$((numCANT+1)) +elif [ "$bytes" -eq $(cat "$tw") ]; then + $PRINTF "$OK\n" + numOK=$((numOK+1)) +else + $PRINTF "$FAILED\n" + echo "transferred only $(cat $tw) of $bytes bytes" >&2 + numFAIL=$((numFAIL+1)) +fi +fi # NUMCOND + ;; +esac +N=$((N+1)) # socat up to 1.7.2.0 and 2.0.0-b4 had a bug in xioscan_readline() that could diff --git a/xio-proxy.c b/xio-proxy.c index f32b69c..7e86d6f 100644 --- a/xio-proxy.c +++ b/xio-proxy.c @@ -371,10 +371,7 @@ int _xioopen_proxy_connect(struct single *xfd, * xiosanitize(request, strlen(request), textbuff) = '\0'; Info1("sending \"%s\"", textbuff); /* write errors are assumed to always be hard errors, no retry */ - do { - sresult = Write(xfd->wfd, request, strlen(request)); - } while (sresult < 0 && errno == EINTR); - if (sresult < 0) { + if (writefull(xfd->wfd, request, strlen(request)) < 0) { Msg4(level, "write(%d, %p, "F_Zu"): %s", xfd->wfd, request, strlen(request), strerror(errno)); if (Close(xfd->wfd) < 0) { @@ -404,10 +401,7 @@ int _xioopen_proxy_connect(struct single *xfd, *next = '\0'; Info1("sending \"%s\\r\\n\"", header); *next++ = '\r'; *next++ = '\n'; *next++ = '\0'; - do { - sresult = Write(xfd->wfd, header, strlen(header)); - } while (sresult < 0 && errno == EINTR); - if (sresult < 0) { + if (writefull(xfd->wfd, header, strlen(header)) < 0) { Msg4(level, "write(%d, %p, "F_Zu"): %s", xfd->wfd, header, strlen(header), strerror(errno)); if (Close(xfd->wfd/*!*/) < 0) { @@ -420,10 +414,14 @@ int _xioopen_proxy_connect(struct single *xfd, } Info("sending \"\\r\\n\""); - do { - sresult = Write(xfd->wfd, "\r\n", 2); - } while (sresult < 0 && errno == EINTR); - /*! */ + if (writefull(xfd->wfd, "\r\n", 2) < 0) { + Msg2(level, "write(%d, %p, "F_Zu"): %s", + xfd->wfd, strerror(errno)); + if (Close(xfd->wfd) < 0) { + Info2("close(%d): %s", xfd->wfd, strerror(errno)); + } + return STAT_RETRYLATER; + } /* request is kept for later error messages */ *strstr(request, " HTTP") = '\0'; diff --git a/xio-readline.c b/xio-readline.c index 1b9b65b..29351c1 100644 --- a/xio-readline.c +++ b/xio-readline.c @@ -185,9 +185,7 @@ ssize_t xioread_readline(struct single *pipe, void *buff, size_t bufsiz) { /* we must carriage return, because readline will first print the prompt */ ssize_t writt; - do { - writt = Write(pipe->rfd, "\r", 1); - } while (writt < 0 && errno == EINTR); + writt = writefull(pipe->rfd, "\r", 1); if (writt < 0) { Warn2("write(%d, \"\\r\", 1): %s", pipe->rfd, strerror(errno)); diff --git a/xio-socks.c b/xio-socks.c index ebc7b40..eb22c66 100644 --- a/xio-socks.c +++ b/xio-socks.c @@ -377,10 +377,7 @@ int _xioopen_socks4_connect(struct single *xfd, } } #endif /* WITH_MSGLEVEL <= E_DEBUG */ - do { - result = Write(wfd, sockhead, headlen); - } while (result < 0 && errno == EINTR); - if (result < 0) { + if (writefull(wfd, sockhead, headlen) < 0) { Msg4(level, "write(%d, %p, "F_Zu"): %s", wfd, sockhead, headlen, strerror(errno)); if (Close(wfd) < 0) { diff --git a/xiolockfile.c b/xiolockfile.c index 0ad5269..93982f3 100644 --- a/xiolockfile.c +++ b/xiolockfile.c @@ -1,5 +1,5 @@ /* source: xiolockfile.c */ -/* Copyright Gerhard Rieger 2005-2006 */ +/* Copyright Gerhard Rieger 2005-2012 */ /* Published under the GNU General Public License V.2, see file COPYING */ /* this file contains socats explicit locking mechanisms */ @@ -52,7 +52,10 @@ int xiogetlock(const char *lockfile) { pid = Getpid(); bytes = sprintf(pidbuf, F_pid, pid); - Write(fd, pidbuf, bytes); + if (writefull(fd, pidbuf, bytes) < 0) { + Error4("write(%d, %p, "F_Zu"): %s", fd, pidbuf, bytes, strerror(errno)); + return -1; + } Close(fd); /* Chmod(lockfile, 0600); */ diff --git a/xiowrite.c b/xiowrite.c index 206e201..fc95b09 100644 --- a/xiowrite.c +++ b/xiowrite.c @@ -1,5 +1,5 @@ /* source: xiowrite.c */ -/* Copyright Gerhard Rieger 2001-2008 */ +/* Copyright Gerhard Rieger 2001-2012 */ /* Published under the GNU General Public License V.2, see file COPYING */ /* this is the source of the extended write function */ @@ -53,9 +53,7 @@ ssize_t xiowrite(xiofile_t *file, const void *buff, size_t bytes) { switch (pipe->dtype & XIODATA_WRITEMASK) { case XIOWRITE_STREAM: - do { - writt = Write(fd, buff, bytes); - } while (writt < 0 && errno == EINTR); + writt = writefull(pipe->wfd, buff, bytes); if (writt < 0) { _errno = errno; switch (_errno) { @@ -74,10 +72,6 @@ ssize_t xiowrite(xiofile_t *file, const void *buff, size_t bytes) { errno = _errno; return -1; } - if ((size_t)writt < bytes) { - Warn2("write() only wrote "F_Zu" of "F_Zu" bytes", - writt, bytes); - } break; #if _WITH_SOCKET @@ -125,9 +119,7 @@ ssize_t xiowrite(xiofile_t *file, const void *buff, size_t bytes) { case XIOWRITE_PIPE: case XIOWRITE_2PIPE: - do { - writt = Write(fd, buff, bytes); - } while (writt < 0 && errno == EINTR); + writt = Write(pipe->wfd, buff, bytes); _errno = errno; if (writt < 0) { Error4("write(%d, %p, "F_Zu"): %s", @@ -135,10 +127,6 @@ ssize_t xiowrite(xiofile_t *file, const void *buff, size_t bytes) { errno = _errno; return -1; } - if ((size_t)writt < bytes) { - Warn2("write() only wrote "F_Zu" of "F_Zu" bytes", - writt, bytes); - } break; #if WITH_TEST