git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 4/9] avoid infinite loop in read_istream_loose

From
Jeff King <peff@peff.net>
Date
Mar 25, 2013, 20:21 UTC
Message-ID
<20130325202114.GD16019@sigill.intra.peff.net>
In-Reply-To
<20130325201427.GA15798@sigill.intra.peff.net>

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 <peff@peff.net>
---
The read_istream_pack_non_delta function does not suffer from the same
issue, because it continually feeds more data via use_pack(). Although
it may run into problems if it reads to the very end of a pack. I also
didn't audit the other zlib code paths for similar problems; we may want
to do that.
 streaming.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/streaming.c b/streaming.c
index f4ab12b..cabcd9d 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;
-- 
1.8.2.13.g0f18d3c
Previous: Jeff KingNext: Jeff King
Message 36 of 60 in “propagating repo corruption across clone”
  1. Jeff KingMar 24, 2013
  2. Ævar Arnfjörð BjarmasonMar 24, 2013
  3. Jeff KingMar 24, 2013
  4. Jeff MitchellMar 25, 2013
  5. Jeff KingMar 25, 2013
  6. Duy NguyenMar 25, 2013
  7. Jeff KingMar 25, 2013
  8. Jeff MitchellMar 25, 2013
  9. Jeff KingMar 25, 2013
  10. Jeff MitchellMar 26, 2013
  11. Jeff KingMar 26, 2013
  12. Philip OakleyMar 26, 2013
  13. Jeff KingMar 26, 2013
  14. Rich FrommMar 26, 2013
  15. Jonathan NiederMar 27, 2013
  16. Rich FrommMar 27, 2013
  17. Jeff KingMar 27, 2013
  18. Jeff KingMar 27, 2013
  19. Junio C HamanoMar 27, 2013
  20. Sitaram ChamartyMar 27, 2013
  21. Junio C HamanoMar 27, 2013
  22. Sitaram ChamartyMar 27, 2013
  23. Rich FrommMar 27, 2013
  24. Junio C HamanoMar 27, 2013
  25. Jeff MitchellMar 28, 2013
  26. Jeff MitchellMar 28, 2013
  27. Duy NguyenMar 26, 2013
  28. Ilari LiusvaaraMar 24, 2013
  29. Junio C HamanoMar 25, 2013
  30. Jeff KingMar 25, 2013
  31. 0/9 corrupt object potpourriJeff King, Mar 25, 2013
  32. 1/9 stream_blob_to_fd: detect errors reading from streamJeff King, Mar 25, 2013
  33. Junio C HamanoMar 26, 2013
  34. 2/9 check_sha1_signature: check return value from read_istreamJeff King, Mar 25, 2013
  35. 3/9 read_istream_filtered: propagate read error from upstreamJeff King, Mar 25, 2013
  36. 4/9 avoid infinite loop in read_istream_looseJeff King, Mar 25, 2013
  37. 5/9 add test for streaming corrupt blobsJeff King, Mar 25, 2013
  38. Jonathan NiederMar 25, 2013
  39. Jeff KingMar 25, 2013
  40. Jeff KingMar 27, 2013
  41. Junio C HamanoMar 27, 2013
  42. 6/9 streaming_write_entry: propagate streaming errorsJeff King, Mar 25, 2013
  43. Eric SunshineMar 25, 2013
  44. Jeff KingMar 25, 2013
  45. Jonathan NiederMar 25, 2013
  46. 6/9 streaming_write_entry: propagate streaming errorsJeff King, Mar 25, 2013
  47. Jonathan NiederMar 25, 2013
  48. Junio C HamanoMar 26, 2013
  49. 7/9 add tests for cloning corrupted repositoriesJeff King, Mar 25, 2013
  50. 8/9 clone: die on errors from unpack_treesJeff King, Mar 25, 2013
  51. Junio C HamanoMar 26, 2013
  52. 10/9 clone: leave repo in place after checkout errorsJeff King, Mar 26, 2013
  53. Jonathan NiederMar 26, 2013
  54. Jeff KingMar 27, 2013
  55. 9/9 clone: run check_everything_connectedJeff King, Mar 25, 2013
  56. Duy NguyenMar 26, 2013
  57. Jeff KingMar 26, 2013
  58. Junio C HamanoMar 26, 2013
  59. Duy NguyenMar 28, 2013
  60. Duy NguyenMar 31, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.