From 45d4bdae5906cfe6b7cb1ba1cec82fd80381e07e Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 25 Mar 2013 16:16:50 -0400 Subject: stream_blob_to_fd: detect errors reading from stream We call read_istream, but never check its return value for errors. This can lead to us looping infinitely, as we just keep trying to write "-1" bytes (and we do not notice the error, as we simply check that write_in_full reports the same number of bytes we fed it, which of course is also -1). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- streaming.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'streaming.c') diff --git a/streaming.c b/streaming.c index 4d978e54e4..f4126a7da5 100644 --- a/streaming.c +++ b/streaming.c @@ -514,6 +514,8 @@ int stream_blob_to_fd(int fd, unsigned const char *sha1, struct stream_filter *f ssize_t wrote, holeto; ssize_t readlen = read_istream(st, buf, sizeof(buf)); + if (readlen < 0) + goto close_and_exit; if (!readlen) break; if (can_seek && sizeof(buf) == readlen) { -- cgit v1.2.1 From 42e7e2a53492ed1772b7b5d8b328f8d0a66f8b33 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 25 Mar 2013 16:18:16 -0400 Subject: read_istream_filtered: propagate read error from upstream The filter istream pulls data from an "upstream" stream, running it through a filter function. However, we did not properly notice when the upstream filter yielded an error, and just returned what we had read. Instead, we should propagate the error. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- streaming.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'streaming.c') diff --git a/streaming.c b/streaming.c index f4126a7da5..f4ab12ba42 100644 --- a/streaming.c +++ b/streaming.c @@ -237,7 +237,7 @@ static read_method_decl(filtered) if (!fs->input_finished) { fs->i_end = read_istream(fs->upstream, fs->ibuf, FILTER_BUFFER); if (fs->i_end < 0) - break; + return -1; if (fs->i_end) continue; } -- cgit v1.2.1 From 692f0bc7ae0cb818bf3c757d509773d6f2e48c68 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 25 Mar 2013 16:21:14 -0400 Subject: avoid infinite loop in read_istream_loose The read_istream_loose function loops on inflating a chunk of data from an mmap'd loose object. We end the loop when we run out of space in our output buffer, or if we see a zlib error. We need to treat Z_BUF_ERROR specially, though, as it is not fatal; it is just zlib's way of telling us that we need to either feed it more input or give it more output space. It is perfectly normal for us to hit this when we are at the end of our buffer. However, we may also get Z_BUF_ERROR because we have run out of input. In a well-formed object, this should not happen, because we have fed the whole mmap'd contents to zlib. But if the object is truncated or corrupt, we will loop forever, never giving zlib any more data, but continuing to ask it to inflate. We can fix this by considering it an error when zlib returns Z_BUF_ERROR but we still have output space left (which means it must want more input, which we know is a truncation error). It would not be sufficient to just check whether zlib had consumed all the input at the start of the loop, as it might still want to generate output from what is in its internal state. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- streaming.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'streaming.c') diff --git a/streaming.c b/streaming.c index f4ab12ba42..cabcd9d157 100644 --- a/streaming.c +++ b/streaming.c @@ -309,7 +309,7 @@ static read_method_decl(loose) st->z_state = z_done; break; } - if (status != Z_OK && status != Z_BUF_ERROR) { + if (status != Z_OK && (status != Z_BUF_ERROR || total_read < sz)) { git_inflate_end(&st->z); st->z_state = z_error; return -1; -- cgit v1.2.1