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

[PATCH v4 3/5] archive-tar: write extended headers for file sizes >= 8GB

From
Jeff King <peff@peff.net>
Date
Jun 30, 2016, 09:09 UTC
Message-ID
<20160630090916.GC17463@sigill.intra.peff.net>
In-Reply-To
<20160630090614.GA16725@sigill.intra.peff.net>

The ustar format has a fixed-length field for the size of each file entry which is supposed to contain up to 11 bytes of octal-formatted data plus a NUL or space terminator.

These means that the largest size we can represent is 077777777777, or 1 byte short of 8GB. The correct solution for a larger file, according to POSIX.1-2001, is to add an extended pax header, similar to how we handle long filenames. This patch does that, and writes zero for the size field in the ustar header (the last bit is not mentioned by POSIX, but it matches how GNU tar behaves with --format=pax).

This should be a strict improvement over the current behavior, which is to die in xsnprintf with a "BUG". However, there's some interesting history here.

Prior to f2f0267 (archive-tar: use xsnprintf for trivial formatting, 2015-09-24), we silently overflowed the "size" field. The extra bytes ended up in the "mtime" field of the header, which was then immediately written itself, overwriting our extra bytes. What that means depends on how many bytes we wrote.

If the size was 64GB or greater, then we actually overflowed digits into the mtime field, meaning our value was effectively right-shifted by those lost octal digits. And this patch is again a strict improvement over that.

But if the size was between 8GB and 64GB, then our 12-byte field held all of the actual digits, and only our NUL terminator overflowed. According to POSIX, there should be a NUL or space at the end of the field. However, GNU tar seems to be lenient here, and will correctly parse a size up 64GB (minus one) from the field. So sizes in this range might have just worked, depending on the implementation reading the tarfile.

This patch is mostly still an improvement there, as the 8GB limit is specifically mentioned in POSIX as the correct limit. But it's possible that it could be a regression (versus the pre-f2f0267 state) if all of the following are true:

  1. You have a file between 8GB and 64GB.
  2. Your tar implementation _doesn't_ know about pax
     extended headers.
  3. Your tar implementation _does_ parse 12-byte sizes from
     the ustar header without a delimiter.

It's probably not worth worrying about such an obscure set of conditions, but I'm documenting it here just in case.

Helped-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Jeff King <peff@peff.net>
---
 archive-tar.c       | 31 +++++++++++++++++++++++++++++--
 t/t5000-tar-tree.sh |  4 ++--
 2 files changed, 31 insertions(+), 4 deletions(-)
diff --git a/archive-tar.c b/archive-tar.c
index cb99df2..57a1540 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -18,6 +18,13 @@ static int tar_umask = 002;
 static int write_tar_filter_archive(const struct archiver *ar,
 				    struct archiver_args *args);
 
+/*
+ * This is the max value that a ustar size header can specify, as it is fixed
+ * at 11 octal digits. POSIX specifies that we switch to extended headers at
+ * this size.
+ */
+#define USTAR_MAX_SIZE 077777777777UL
+
 /* writes out the whole block, but only if it is full */
 static void write_if_needed(void)
 {
@@ -137,6 +144,20 @@ static void strbuf_append_ext_header(struct strbuf *sb, const char *keyword,
 	strbuf_addch(sb, '\n');
 }
 
+/*
+ * Like strbuf_append_ext_header, but for numeric values.
+ */
+static void strbuf_append_ext_header_uint(struct strbuf *sb,
+					  const char *keyword,
+					  uintmax_t value)
+{
+	char buf[40]; /* big enough for 2^128 in decimal, plus NUL */
+	int len;
+
+	len = xsnprintf(buf, sizeof(buf), "%"PRIuMAX, value);
+	strbuf_append_ext_header(sb, keyword, buf, len);
+}
+
 static unsigned int ustar_header_chksum(const struct ustar_header *header)
 {
 	const unsigned char *p = (const unsigned char *)header;
@@ -208,7 +229,7 @@ static int write_tar_entry(struct archiver_args *args,
 	struct ustar_header header;
 	struct strbuf ext_header = STRBUF_INIT;
 	unsigned int old_mode = mode;
-	unsigned long size;
+	unsigned long size, size_in_header;
 	void *buffer;
 	int err = 0;
 
@@ -267,7 +288,13 @@ static int write_tar_entry(struct archiver_args *args,
 			memcpy(header.linkname, buffer, size);
 	}
 
-	prepare_header(args, &header, mode, size);
+	size_in_header = size;
+	if (S_ISREG(mode) && size > USTAR_MAX_SIZE) {
+		size_in_header = 0;
+		strbuf_append_ext_header_uint(&ext_header, "size", size);
+	}
+
+	prepare_header(args, &header, mode, size_in_header);
 
 	if (ext_header.len > 0) {
 		err = write_extended_header(args, sha1, ext_header.buf,
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index 950bdd3..93c2d34 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -360,7 +360,7 @@ test_expect_success 'set up repository with huge blob' '
 
 # We expect git to die with SIGPIPE here (otherwise we
 # would generate the whole 64GB).
-test_expect_failure 'generate tar with huge size' '
+test_expect_success 'generate tar with huge size' '
 	{
 		git archive HEAD
 		echo $? >exit-code
@@ -369,7 +369,7 @@ test_expect_failure 'generate tar with huge size' '
 	test_cmp expect exit-code
 '
 
-test_expect_failure TAR_HUGE 'system tar can read our huge size' '
+test_expect_success TAR_HUGE 'system tar can read our huge size' '
 	echo 68719476737 >expect &&
 	tar_info huge.tar | cut -d" " -f1 >actual &&
 	test_cmp expect actual
-- 
2.9.0.317.g65b4e7c
Previous: Jeff KingNext: Johannes Sixt
Message 3 of 42 in “friendlier handling of overflows in archive-tar”
  1. 0/5 friendlier handling of overflows in archive-tarJeff King, Jun 30, 2016
  2. 5/5 archive-tar: drop return valueJeff King, Jun 30, 2016
  3. 3/5 archive-tar: write extended headers for file sizes >= 8GBJeff King, Jun 30, 2016
  4. Johannes SixtJul 14, 2016
  5. Junio C HamanoJul 14, 2016
  6. Jeff KingJul 14, 2016
  7. Torsten BögershausenJul 15, 2016
  8. 4/5 archive-tar: write extended headers for far-future mtimeJeff King, Jun 30, 2016
  9. 1/5 t9300: factor out portable "head -c" replacementJeff King, Jun 30, 2016
  10. Eric SunshineJul 1, 2016
  11. Junio C HamanoJul 1, 2016
  12. Jeff KingJul 1, 2016
  13. 6/5 t5000: use test_match_signalJeff King, Jun 30, 2016
  14. 2/5 t5000: test tar files that overflow ustar headersJeff King, Jun 30, 2016
  15. Johannes SchindelinJul 14, 2016
  16. Johannes SixtJul 14, 2016
  17. Junio C HamanoJul 14, 2016
  18. Johannes SixtJul 14, 2016
  19. Jeff KingJul 14, 2016
  20. Junio C HamanoJul 14, 2016
  21. Jeff KingJul 14, 2016
  22. Torsten BögershausenJul 15, 2016
  23. Jeff KingJul 15, 2016
  24. Junio C HamanoJul 14, 2016
  25. Jeff KingJul 14, 2016
  26. Jeff KingJul 14, 2016
  27. Junio C HamanoJul 14, 2016
  28. Junio C HamanoJul 14, 2016
  29. Jeff KingJul 14, 2016
  30. Junio C HamanoJul 14, 2016
  31. Jeff KingJul 14, 2016
  32. Junio C HamanoJul 14, 2016
  33. Jeff KingJul 14, 2016
  34. Junio C HamanoJul 14, 2016
  35. 0/2 ulong may only be 32-bit wideJunio C Hamano, Jul 14, 2016
  36. 2/2 archive-tar: huge offset and future timestamps would not work on 32-bitJunio C Hamano, Jul 14, 2016
  37. Jeff KingJul 14, 2016
  38. Junio C HamanoJul 14, 2016
  39. Duy NguyenJul 16, 2016
  40. 1/2 t0006: skip "far in the future" test when unsigned long is not long enoughJunio C Hamano, Jul 14, 2016
  41. Johannes SchindelinJul 15, 2016
  42. Junio C HamanoJul 15, 2016

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.