Truncated output when writing to a pipe using sendfile

Truncated output when writing to a pipe using sendfile

[Filips R]

Hello!

I've been investigating an issue on my Alpine linux server running
cgit 1.2.3-r2.
When I call the CGI program and redirect to a regular file (or just
output to tty), the output is correct, however if the standard output
is a pipe, the output is truncated at 65523 bytes.

Here is the end of the diff between strace outputs:

 open("/var/cache/cgit/a1000000", O_RDONLY|O_LARGEFILE) = 3
 fstat(3, {st_mode=S_IFREG|0600, st_size=157003, ...}) = 0
 read(3, "st/tree/st.c\0Content-Type: text/"..., 4096) = 4096
-sendfile(1, 3, [13] => [157003], 156990) = 156990
+sendfile(1, 3, [13] => [65536], 156990) = 65523
 close(3)                                = 0
 exit_group(0)                           = ?
 +++ exited with 0 +++

I have managed to work around the issue by wrapping cgit and
redirecting to a temporary file.

Looks like the sendfile call comes from cache.c:95 -
https://git.zx2c4.com/cgit/tree/cache.c?h=v1.2.3&id=55fa25adb097d2681607d8b0f51a0c393cc9af1a#n95

senfile(2) says that a successful call may write fewer bytes than
requested; the caller should be prepared to retry the call if there
were unsent bytes

Re: Truncated output when writing to a pipe using sendfile

[Gianni Ceccarelli]

On 2022-05-07 Filips R <frfilips@gmail.com> wrote:
> Looks like the sendfile call comes from cache.c:95 -
> https://git.zx2c4.com/cgit/tree/cache.c?h=v1.2.3&id=55fa25adb097d2681607d8b0f51a0c393cc9af1a#n95
> 
> senfile(2) says that a successful call may write fewer bytes than
> requested; the caller should be prepared to retry the call if there
> were unsent bytes

Looks like the same issue as
https://www.mail-archive.com/cgit@lists.zx2c4.com/msg03158.html

-- 
	Dakkar - <Mobilis in mobile>
	GPG public key fingerprint = A071 E618 DD2C 5901 9574
	                             6FE2 40EA 9883 7519 3F88
	                    key id = 0x75193F88

Re: Truncated output when writing to a pipe using sendfile

[Hristo Venev]

Oops, I forgot about this...

Sorry about the duplicate emails. I'd made an error in one of the
addresses.

[PATCH RESEND v2] cache: Tolerate short writes in print_slot

[Hristo Venev]

sendfile() can return after a short read/write, so we may need to call
it more than once. As suggested in the manual page, we fall back to
read/write if sendfile fails with EINVAL or ENOSYS.

On the read/write path, use write_in_full which deals with short writes.

Signed-off-by: Hristo Venev <hristo@venev.name>
---
 cache.c | 45 +++++++++++++++++++++++++--------------------
 1 file changed, 25 insertions(+), 20 deletions(-)

diff --git a/cache.c b/cache.c
index 55199e8..1c843ba 100644
--- a/cache.c
+++ b/cache.c
@@ -85,40 +85,45 @@ static int close_slot(struct cache_slot *slot)
 /* Print the content of the active cache slot (but skip the key). */
 static int print_slot(struct cache_slot *slot)
 {
+	off_t off;
 #ifdef HAVE_LINUX_SENDFILE
-	off_t start_off;
-	int ret;
+	off_t size;
+#endif
+
+	off = slot->keylen + 1;
 
-	start_off = slot->keylen + 1;
+#ifdef HAVE_LINUX_SENDFILE
+	size = slot->cache_st.st_size;
 
 	do {
-		ret = sendfile(STDOUT_FILENO, slot->cache_fd, &start_off,
-				slot->cache_st.st_size - start_off);
+		ssize_t ret;
+		ret = sendfile(STDOUT_FILENO, slot->cache_fd, &off, size - off);
 		if (ret < 0) {
 			if (errno == EAGAIN || errno == EINTR)
 				continue;
+			/* Fall back to read/write on EINVAL or ENOSYS */
+			if (errno == EINVAL || errno == ENOSYS)
+				break;
 			return errno;
 		}
-		return 0;
+		if (off == size)
+			return 0;
 	} while (1);
-#else
-	ssize_t i, j;
+#endif
 
-	i = lseek(slot->cache_fd, slot->keylen + 1, SEEK_SET);
-	if (i != slot->keylen + 1)
+	if (lseek(slot->cache_fd, off, SEEK_SET) != off)
 		return errno;
 
 	do {
-		i = j = xread(slot->cache_fd, slot->buf, sizeof(slot->buf));
-		if (i > 0)
-			j = xwrite(STDOUT_FILENO, slot->buf, i);
-	} while (i > 0 && j == i);
-
-	if (i < 0 || j != i)
-		return errno;
-	else
-		return 0;
-#endif
+		ssize_t ret;
+		ret = xread(slot->cache_fd, slot->buf, sizeof(slot->buf));
+		if (ret < 0)
+			return errno;
+		if (ret == 0)
+			return 0;
+		if (write_in_full(STDOUT_FILENO, slot->buf, ret) < 0)
+			return errno;
+	} while (1);
 }
 
 /* Check if the slot has expired */
-- 
2.35.3