[PATCH] git name-rev writes beyond the end of malloc() with large generations

From: Andy Whitcroft <apw@shadowen.org>
Date: May 15, 2007, 16:33 UTC
Message-ID: <2be2ad34be511217dc735a15490f4536@pinky>

When using git name-rev on my kernel tree I triggered a malloc() corruption warning from glibc.

apw@pinky$ git log --pretty=one $N/base.. | git name-rev --stdin *** glibc detected *** malloc(): memory corruption: 0x0bff8950 *** Aborted

This comes from name_rev() which is building the name of the revision in a malloc'd string, which it sprintf's into:

	char *new_name = xmalloc(len + 8);
	[...]
		sprintf(new_name, "%.*s~%d^%d", len, tip_name,
				generation, parent_number);

This allocation is only sufficient if the generation number is less than 5 digits, in my case generation was 13432. In reality parent_number can be up to 16 so that also can require two digits, reducing us to 3 digits before we are at risk of blowing this allocation.

This patch introduces a decimal_length() which approximates the number of digits a type may hold, it produces the following:

Type Longest Value Len Est ---- ------------- --- --- unsigned char 256 3 4 unsigned short 65536 5 6 unsigned long 4294967296 10 11 unsigned long long 18446744073709551616 20 21 char -128 4 4 short -32768 6 6 long -2147483648 11 11 long long -9223372036854775808 20 21

This is then used to size the new_name.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---

	This patch is against current next.  I have confirmed that
	at least GCC can optimise this away to a constant.
---

diff --git a/builtin-name-rev.c b/builtin-name-rev.c
index c022224..ef16385 100644
--- a/builtin-name-rev.c
+++ b/builtin-name-rev.c
@@ -58,7 +58,10 @@ copy_data:
 			parents = parents->next, parent_number++) {
 		if (parent_number > 1) {
 			int len = strlen(tip_name);
-			char *new_name = xmalloc(len + 8);
+			char *new_name = xmalloc(len +
+				1 + decimal_length(generation) +  /* ~<n> */
+				1 + 2 +				  /* ^NN */
+				1);
 
 			if (len > 2 && !strcmp(tip_name + len - 2, "^0"))
 				len -= 2;
diff --git a/git-compat-util.h b/git-compat-util.h
index c08688c..25b8274 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -19,6 +19,9 @@
 #define TYPEOF(x)
 #endif
 
+/* Approximation of the length of the decimal representation of this type. */
+#define decimal_length(x)	((int)(sizeof(x) * 2.56 + 0.5) + 1)
+
 #define MSB(x, bits) ((x) & TYPEOF(x)(~0ULL << (sizeof(x) * 8 - (bits))))
 
 #if !defined(__APPLE__) && !defined(__FreeBSD__)