Thread (1 message) 1 message, 1 author, 2016-06-15

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:44:01

Jim Meyering [off-list ref] writes:
quoted
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)
Sorry, I asked for a wrong thing.  parse_object() reads and
finds out the type, so type there is presumably the right type
of the object (which is OBJ_TREE).  Then parse_object_buffer()
checks if it has already seen the object of the same SHA-1 and
finds that somebody had earlier told that SHA-1 name is of a
commit object.  Either you found a SHA-1 collision (highly
unlikely) or the earlier caller had lied.  And I think what
really needs to be fixed is that lying caller.  That is not in
the above call chain.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help