From 6a8f6c0734e4d671111eb687f7111936bd0bd9e3 Mon Sep 17 00:00:00 2001 From: Gerhard Rieger Date: Sun, 9 Oct 2011 09:18:31 +0200 Subject: [PATCH] handle partial write()'s without data loss --- CHANGES | 8 ++++++++ sysutils.c | 34 ++++++++++++++++++++++++++++++++++ sysutils.h | 4 +++- test.sh | 45 +++++++++++++++++++++++++++++++++++++++++++++ xio-proxy.c | 24 +++++++++++------------- xio-readline.c | 6 ++---- xio-socks.c | 7 ++----- xiolockfile.c | 7 +++++-- xiowrite.c | 26 ++++---------------------- 9 files changed, 114 insertions(+), 47 deletions(-) diff --git a/CHANGES b/CHANGES index 8d524a7..5dfbc05 100644 --- a/CHANGES +++ b/CHANGES @@ -30,6 +30,14 @@ corrections: 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. + ####################### V 1.7.1.3: security: diff --git a/sysutils.c b/sysutils.c index 8815900..139d551 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 b5146e9..171b65a 100644 --- a/sysutils.h +++ b/sysutils.h @@ -1,5 +1,5 @@ /* source: sysutils.h */ -/* Copyright Gerhard Rieger 2001-2010 */ +/* Copyright Gerhard Rieger 2001-2011 */ /* Published under the GNU General Public License V.2, see file COPYING */ #ifndef __sysutils_h_included @@ -40,6 +40,8 @@ struct xiorange { } ; #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); #endif diff --git a/test.sh b/test.sh index 68dc452..87b532e 100755 --- a/test.sh +++ b/test.sh @@ -10439,6 +10439,51 @@ 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)) + + ############################################################################### # here come tests that might affect your systems integrity. Put normal tests # before this paragraph. diff --git a/xio-proxy.c b/xio-proxy.c index 622e2c5..fb08d28 100644 --- a/xio-proxy.c +++ b/xio-proxy.c @@ -1,5 +1,5 @@ /* source: xio-proxy.c */ -/* Copyright Gerhard Rieger 2002-2008 */ +/* Copyright Gerhard Rieger 2002-2011 */ /* Published under the GNU General Public License V.2, see file COPYING */ /* this file contains the source for opening addresses of HTTP proxy CONNECT @@ -293,10 +293,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->fd, request, strlen(request)); - } while (sresult < 0 && errno == EINTR); - if (sresult < 0) { + if (writefull(xfd->fd, request, strlen(request)) < 0) { Msg4(level, "write(%d, %p, "F_Zu"): %s", xfd->fd, request, strlen(request), strerror(errno)); if (Close(xfd->fd) < 0) { @@ -326,10 +323,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->fd, header, strlen(header)); - } while (sresult < 0 && errno == EINTR); - if (sresult < 0) { + if (writefull(xfd->fd, header, strlen(header)) < 0) { Msg4(level, "write(%d, %p, "F_Zu"): %s", xfd->fd, header, strlen(header), strerror(errno)); if (Close(xfd->fd) < 0) { @@ -342,10 +336,14 @@ int _xioopen_proxy_connect(struct single *xfd, } Info("sending \"\\r\\n\""); - do { - sresult = Write(xfd->fd, "\r\n", 2); - } while (sresult < 0 && errno == EINTR); - /*! */ + if (writefull(xfd->fd, "\r\n", 2) < 0) { + Msg2(level, "write(%d, %p, "F_Zu"): %s", + xfd->fd, strerror(errno)); + if (Close(xfd->fd) < 0) { + Info2("close(%d): %s", xfd->fd, 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 3f66e95..5ffd8ed 100644 --- a/xio-readline.c +++ b/xio-readline.c @@ -1,5 +1,5 @@ /* source: xio-readline.c */ -/* Copyright Gerhard Rieger 2002-2009 */ +/* Copyright Gerhard Rieger 2002-2011 */ /* Published under the GNU General Public License V.2, see file COPYING */ /* this file contains the source for opening the readline address */ @@ -177,9 +177,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->fd, "\r", 1); - } while (writt < 0 && errno == EINTR); + writt = writefull(pipe->fd, "\r", 1); if (writt < 0) { Warn2("write(%d, \"\\r\", 1): %s", pipe->fd, strerror(errno)); diff --git a/xio-socks.c b/xio-socks.c index 9912a89..8597e77 100644 --- a/xio-socks.c +++ b/xio-socks.c @@ -1,5 +1,5 @@ /* source: xio-socks.c */ -/* Copyright Gerhard Rieger 2001-2009 */ +/* Copyright Gerhard Rieger 2001-2011 */ /* Published under the GNU General Public License V.2, see file COPYING */ /* this file contains the source for opening addresses of socks4 type */ @@ -328,10 +328,7 @@ int _xioopen_socks4_connect(struct single *xfd, } } #endif /* WITH_MSGLEVEL <= E_DEBUG */ - do { - result = Write(xfd->fd, sockhead, headlen); - } while (result < 0 && errno == EINTR); - if (result < 0) { + if (writefull(xfd->fd, sockhead, headlen) < 0) { Msg4(level, "write(%d, %p, "F_Zu"): %s", xfd->fd, sockhead, headlen, strerror(errno)); if (Close(xfd->fd) < 0) { diff --git a/xiolockfile.c b/xiolockfile.c index 0ad5269..8de62af 100644 --- a/xiolockfile.c +++ b/xiolockfile.c @@ -1,5 +1,5 @@ /* source: xiolockfile.c */ -/* Copyright Gerhard Rieger 2005-2006 */ +/* Copyright Gerhard Rieger 2005-2011 */ /* 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 2bc9379..569aae7 100644 --- a/xiowrite.c +++ b/xiowrite.c @@ -1,5 +1,5 @@ /* source: xiowrite.c */ -/* Copyright Gerhard Rieger 2001-2008 */ +/* Copyright Gerhard Rieger 2001-2011 */ /* Published under the GNU General Public License V.2, see file COPYING */ /* this is the source of the extended write function */ @@ -49,9 +49,7 @@ ssize_t xiowrite(xiofile_t *file, const void *buff, size_t bytes) { switch (pipe->dtype & XIODATA_WRITEMASK) { case XIOWRITE_STREAM: - do { - writt = Write(pipe->fd, buff, bytes); - } while (writt < 0 && errno == EINTR); + writt = writefull(pipe->fd, buff, bytes); if (writt < 0) { _errno = errno; switch (_errno) { @@ -70,10 +68,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 @@ -120,9 +114,7 @@ ssize_t xiowrite(xiofile_t *file, const void *buff, size_t bytes) { #endif /* _WITH_SOCKET */ case XIOWRITE_PIPE: - do { - writt = Write(pipe->para.bipipe.fdout, buff, bytes); - } while (writt < 0 && errno == EINTR); + writt = Write(pipe->para.bipipe.fdout, buff, bytes); _errno = errno; if (writt < 0) { Error4("write(%d, %p, "F_Zu"): %s", @@ -130,16 +122,10 @@ 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; case XIOWRITE_2PIPE: - do { - writt = Write(pipe->para.exec.fdout, buff, bytes); - } while (writt < 0 && errno == EINTR); + writt = Write(pipe->para.exec.fdout, buff, bytes); _errno = errno; if (writt < 0) { Error4("write(%d, %p, "F_Zu"): %s", @@ -147,10 +133,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 processed "F_Zu" of "F_Zu" bytes", - writt, bytes); - } break; #if WITH_OPENSSL