[PATCH v2] packfile: Correct zlib buffer handling

Subsystems: the rest · Maintainer: Linus Torvalds

STALE3058d

2 messages, 2 authors, 2018-06-13 · open the first message on its own page

[PATCH v2] packfile: Correct zlib buffer handling

From: Jeremy Linton <hidden>
Date: 2018-06-13 14:22:33

The buffer being passed to zlib includes a null terminator that
git needs to keep in place. unpack_compressed_entry() attempts to
detect the case that the source buffer hasn't been fully consumed
by checking to see if the destination buffer has been over consumed.

This causes a problem, that more recent zlib patches have been
poisoning the unconsumed portions of the buffer which overwrites
the null, while correctly returning length and status.

Let's replace the null at the end of the buffer to assure that
if its been overwritten by zlib it doesn't result in problems for
git.

Signed-off-by: Jeremy Linton <redacted>
---
 packfile.c | 2 ++
 1 file changed, 2 insertions(+)
diff --git a/packfile.c b/packfile.c
index 7c1a2519f..8db5d90ca 100644
--- a/packfile.c
+++ b/packfile.c
@@ -1433,6 +1433,8 @@ static void *unpack_compressed_entry(struct packed_git *p,
 		return NULL;
 	}
 
+	buffer[size] = 0; /* assure that the buffer is still terminated */
+
 	return buffer;
 }
 
-- 
2.13.6

Re: [PATCH v2] packfile: Correct zlib buffer handling

From: Eric Sunshine <hidden>
Date: 2018-06-13 17:21:18

A couple comments if you happen to re-roll...

On Wed, Jun 13, 2018 at 10:22 AM Jeremy Linton [off-list ref] wrote:
The buffer being passed to zlib includes a null terminator that
On this project, the character mnemonic "NUL" is typically used, not
"null" or "NULL" (which is typically reserved for pointers), so:
s/null/NUL/g
quoted hunk
git needs to keep in place. unpack_compressed_entry() attempts to
detect the case that the source buffer hasn't been fully consumed
by checking to see if the destination buffer has been over consumed.

This causes a problem, that more recent zlib patches have been
poisoning the unconsumed portions of the buffer which overwrites
the null, while correctly returning length and status.

Let's replace the null at the end of the buffer to assure that
if its been overwritten by zlib it doesn't result in problems for
git.

Signed-off-by: Jeremy Linton <redacted>
---
diff --git a/packfile.c b/packfile.c
@@ -1433,6 +1433,8 @@ static void *unpack_compressed_entry(struct packed_git *p,
+       buffer[size] = 0; /* assure that the buffer is still terminated */
I think we normally use '\0' for NUL on this project rather than simply 0.

The comment is also effectively pure noise since it merely repeats
what the code already states clearly (especially when the code says
"buffer[size] = '\0';"), so dropping the comment altogether would be
reasonable.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help