Re: [PATCH] Don't dereference NULL upon lookup_tree failure.

Junio C Hamano [off-list ref] wrote:

Jim Meyering [off-list ref] writes:

quoted

When running on an x86_64 system (either debian unstable or rawhide)
I see only this:

  0 blocks
  error: Object 0d57588da39d10795486bd5451bc2660832228e6 is a commit, not a tree
  fatal: The remote end hung up unexpectedly
...

diff --git a/object.c b/object.c
index 16793d9..eb59550 100644
--- a/object.c
+++ b/object.c

@@ -142,10 +142,14 @@ struct object *parse_object_buffer(const unsigned char *sha1, enum object_type t
 		obj = &blob->object;
 	} else if (type == OBJ_TREE) {
 		struct tree *tree = lookup_tree(sha1);
-		obj = &tree->object;
-		if (!tree->object.parsed) {
-			parse_tree_buffer(tree, buffer, size);
-			eaten = 1;
+		if (!tree)
+		    obj = NULL;
+		else {
+		    obj = &tree->object;
+		    if (!tree->object.parsed) {
+			    parse_tree_buffer(tree, buffer, size);
+			    eaten = 1;
+		    }
 		}
 	} else if (type == OBJ_COMMIT) {
 		struct commit *commit = lookup_commit(sha1);

While this change may be a prudent safeguard, there is something
else going on.  Can you provide the callchain that led to the
parse_object_buffer() that gave SHA1 of a commit object with
type set to OBJ_TREE?  Which caller does that bogus combination?

Sure.
Here's valgrind output from running this (from my reproducer):

    valgrind --trace-children=yes git clone . k
---------------
  error: Object 0d57588da39d10795486bd5451bc2660832228e6 is a commit, not a tree
  ==9483== Invalid read of size 1
  ==9483==    at 0x405C27: parse_object_buffer (object.c:146)
  ==9483==    by 0x405CE4: parse_object (object.c:187)
  ==9483==    by 0x403185: send_ref (upload-pack.c:561)
  ==9483==    by 0x408EEF: do_for_each_ref (refs.c:546)
  ==9483==    by 0x4036EC: main (upload-pack.c:587)
  ==9483==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
  ==9483==
  ==9483== Process terminating with default action of signal 11 (SIGSEGV)
  ==9483==  Access not within mapped region at address 0x0
  ==9483==    at 0x405C27: parse_object_buffer (object.c:146)
  ==9483==    by 0x405CE4: parse_object (object.c:187)
  ==9483==    by 0x403185: send_ref (upload-pack.c:561)
  ==9483==    by 0x408EEF: do_for_each_ref (refs.c:546)
  ==9483==    by 0x4036EC: main (upload-pack.c:587)
  ...
  fatal: The remote end hung up unexpectedly