Re: [PATCH] Don't dereference NULL upon lookup_tree failure.
- From
Jim Meyering <jim@meyering.net>
- Date
- Dec 21, 2007, 22:54 UTC
- Message-ID
- <87hcibveut.fsf@rho.meyering.net>
- In-Reply-To
- <7v4peb4qbj.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> wrote:
Show 37 quoted lines
> Jim Meyering <jim@meyering.net> writes:
>
>> 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