Re: [PATCH 0/5] add pack index v2 reading capability to git v1.4.4.4
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 17, 2008, 08:01 UTC
- Message-ID
- <7vzlohhrcf.fsf@gitster.siamese.dyndns.org>
- In-Reply-To
- <alpine.LFD.1.10.0807161033170.2835@woody.linux-foundation.org>
Linus Torvalds <torvalds@linux-foundation.org> writes:
Show 16 quoted lines
> On Wed, 16 Jul 2008, Junio C Hamano wrote: >> >> I do not think it should SEGV. The pack-idx signature was chosen rather >> carefully to allow older ones to die gracefully. > > Well, Pasky reported differently. > >> error: non-monotonic index >> error: Could not read 4a588075c54cd5902e5f4d43b9d6b0c31d0f9769 > > Pasky's report was > > error: non-monotonic index > /usr/bin/git-fetch: line 297: 30402 Segmentation fault git-http-fetch -v -a "$head" "$remote/" > > but maybe that was something specific to his case.
It is caused by the http walker not being careful. In v1.4.4.5 http-fetch.c, this code appears unmodified since v1.4.4.4, and an equivalent code is still in http-walker.c in more recent versions:
static int setup_index(struct alt_base *repo, unsigned char *sha1)
{
struct packed_git *new_pack;
if (has_pack_file(sha1))
return 0; /* don't list this as something we can get */ if (fetch_index(repo, sha1))
return -1; new_pack = parse_pack_index(sha1);
new_pack->next = repo->packs;
repo->packs = new_pack;
return 0;
}Nico taught parse_pack_index() what v2 pack idx file looks like, but when the code hits unknown idx file (or a corrupt one), the function signals error by returning NULL; assigning to new_pack->next without checking would segfault.
We would need this fix to futureproof ourselves for pack idx v3 and later, and also for protecting from a corrupt idx file coming over the wire.
---
http-walker.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/http-walker.c b/http-walker.c index 51c18f2..9dc6b27 100644 --- a/http-walker.c +++ b/http-walker.c @@ -442,6 +442,8 @@ static int setup_index(struct walker *walker, struct alt_base *repo, unsigned ch return -1; new_pack = parse_pack_index(sha1); + if (!new_pack) + return -1; /* parse_pack_index() already issued an error message */ new_pack->next = repo->packs; repo->packs = new_pack; return 0;