Fix two issues related to the use of SIGINFO in fetch(1) to display

progress information.  The first is that fetch_read() (used in the HTTP
code but not the FTP code) can enter an infinite loop if it has previously
been interrupted by a signal.  The second is that when it is interrupted,
fetch_read() will discard any data it may have read up to that point.
Luckily, both bugs are extremely timing-sensitive and therefore difficult
to trigger.

PR:		bin/153240
Submitted by:	Mark <markjdb@gmail.com>
MFC after:	3 weeks

3 files changed, 75 insertions, 7 deletions

diff --git a/lib/libfetch/common.c b/lib/libfetch/common.c
index 0d5f2d6..5dfdcc4 100644
--- a/lib/libfetch/common.c
+++ b/lib/libfetch/common.c
@@ -404,6 +404,34 @@ fetch_ssl_read(SSL *ssl, char *buf, size_t len)
 }
 #endif
 
+/*
+ * Cache some data that was read from a socket but cannot be immediately
+ * returned because of an interrupted system call.
+ */
+static int
+fetch_cache_data(conn_t *conn, char *src, size_t nbytes)
+{
+	char *tmp;
+
+	if (conn->cache.size < nbytes) {
+		tmp = realloc(conn->cache.buf, nbytes);
+		if (tmp == NULL) {
+			errno = ENOMEM;
+			fetch_syserr();
+			return (-1);
+		}
+		conn->cache.buf = tmp;
+		conn->cache.size = nbytes;
+	}
+
+	memcpy(conn->cache.buf, src, nbytes);
+	conn->cache.len = nbytes;
+	conn->cache.pos = 0;
+
+	return (0);
+}
+
+
 static ssize_t
 fetch_socket_read(int sd, char *buf, size_t len)
 {
@@ -429,6 +457,7 @@ fetch_read(conn_t *conn, char *buf, size_t len)
 	fd_set readfds;
 	ssize_t rlen, total;
 	int r;
+	char *start;
 
 	if (fetchTimeout) {
 		FD_ZERO(&readfds);
@@ -437,6 +466,24 @@ fetch_read(conn_t *conn, char *buf, size_t len)
 	}
 
 	total = 0;
+	start = buf;
+
+	if (conn->cache.len > 0) {
+		/*
+		 * The last invocation of fetch_read was interrupted by a
+		 * signal after some data had been read from the socket. Copy
+		 * the cached data into the supplied buffer before trying to
+		 * read from the socket again.
+		 */
+		total = (conn->cache.len < len) ? conn->cache.len : len;
+		memcpy(buf, conn->cache.buf, total);
+
+		conn->cache.len -= total;
+		conn->cache.pos += total;
+		len -= total;
+		buf+= total;
+	}
+
 	while (len > 0) {
 		/*
 		 * The socket is non-blocking.  Instead of the canonical
@@ -472,6 +519,8 @@ fetch_read(conn_t *conn, char *buf, size_t len)
 			total += rlen;
 			continue;
 		} else if (rlen == FETCH_READ_ERROR) {
+			if (errno == EINTR)
+				fetch_cache_data(conn, start, total);
 			return (-1);
 		}
 		// assert(rlen == FETCH_READ_WAIT);
@@ -492,8 +541,12 @@ fetch_read(conn_t *conn, char *buf, size_t len)
 			errno = 0;
 			r = select(conn->sd + 1, &readfds, NULL, NULL, &delta);
 			if (r == -1) {
-				if (errno == EINTR && fetchRestartCalls)
-					continue;
+				if (errno == EINTR) {
+					if (fetchRestartCalls)
+						continue;
+					/* Save anything that was read. */
+					fetch_cache_data(conn, start, total);
+				}
 				fetch_syserr();
 				return (-1);
 			}
@@ -677,6 +730,7 @@ fetch_close(conn_t *conn)
 	if (--conn->ref > 0)
 		return (0);
 	ret = close(conn->sd);
+	free(conn->cache.buf);
 	free(conn->buf);
 	free(conn);
 	return (ret);
diff --git a/lib/libfetch/common.h b/lib/libfetch/common.h
index 35e0dfd..241dbbf 100644
--- a/lib/libfetch/common.h
+++ b/lib/libfetch/common.h
@@ -52,6 +52,13 @@ struct fetchconn {
 	size_t		 bufsize;	/* buffer size */
 	size_t		 buflen;	/* length of buffer contents */
 	int		 err;		/* last protocol reply code */
+	struct {			/* data cached after an interrupted
+					   read */
+		char	*buf;
+		size_t	 size;
+		size_t	 pos;
+		size_t	 len;
+	} cache;
 #ifdef WITH_SSL
 	SSL		*ssl;		/* SSL handle */
 	SSL_CTX		*ssl_ctx;	/* SSL context */
diff --git a/lib/libfetch/http.c b/lib/libfetch/http.c
index b33d6f1..557ff26 100644
--- a/lib/libfetch/http.c
+++ b/lib/libfetch/http.c
@@ -196,6 +196,8 @@ http_growbuf(struct httpio *io, size_t len)
 static int
 http_fillbuf(struct httpio *io, size_t len)
 {
+	ssize_t nbytes;
+
 	if (io->error)
 		return (-1);
 	if (io->eof)
@@ -204,10 +206,11 @@ http_fillbuf(struct httpio *io, size_t len)
 	if (io->chunked == 0) {
 		if (http_growbuf(io, len) == -1)
 			return (-1);
-		if ((io->buflen = fetch_read(io->conn, io->buf, len)) == -1) {
-			io->error = 1;
+		if ((nbytes = fetch_read(io->conn, io->buf, len)) == -1) {
+			io->error = errno;
 			return (-1);
 		}
+		io->buflen = nbytes;
 		io->bufpos = 0;
 		return (io->buflen);
 	}
@@ -227,10 +230,11 @@ http_fillbuf(struct httpio *io, size_t len)
 		len = io->chunksize;
 	if (http_growbuf(io, len) == -1)
 		return (-1);
-	if ((io->buflen = fetch_read(io->conn, io->buf, len)) == -1) {
-		io->error = 1;
+	if ((nbytes = fetch_read(io->conn, io->buf, len)) == -1) {
+		io->error = errno;
 		return (-1);
 	}
+	io->buflen = nbytes;
 	io->chunksize -= io->buflen;
 
 	if (io->chunksize == 0) {
@@ -272,8 +276,11 @@ http_readfn(void *v, char *buf, int len)
 		io->bufpos += l;
 	}
 
-	if (!pos && io->error)
+	if (!pos && io->error) {
+		if (io->error == EINTR)
+			io->error = 0;
 		return (-1);
+	}
 	return (pos);
 }