From 3c078b9c860c7c1dbe8782aa1f79877354cbc602 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 23 Aug 2014 01:27:41 -0400 Subject: fast-import: clean up pack_data pointer in end_packfile We have a global pointer pack_data pointing to the current pack we have open. Inside end_packfile we have two new pointers, old_p and new_p. The latter points to pack_data, and the former points to the new "installed" version of the packfile we get when we hand the file off to the regular sha1_file machinery. When then free old_p. Presumably the extra old_p pointer was there so that we could overwrite pack_data with new_p and still free old_p, but we don't do that. We just leave pack_data pointing to bogus memory, and don't overwrite it until we call start_packfile again (if ever). This can cause problems for our die routine, which calls end_packfile to clean things up. If we die at the wrong moment, we can end up looking at invalid memory in pack_data left after the last end_packfile(). Instead, let's make sure we set pack_data to NULL after we free it, and make calling endfile() again with a NULL pack_data a noop (there is nothing to end). We can further make things less confusing by dropping old_p entirely, and moving new_p closer to its point of use. Signed-off-by: Jeff King Reviewed-by: Ronnie Sahlberg Signed-off-by: Junio C Hamano --- fast-import.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/fast-import.c b/fast-import.c index fb4738d373..92b3186326 100644 --- a/fast-import.c +++ b/fast-import.c @@ -945,10 +945,12 @@ static void unkeep_all_packs(void) static void end_packfile(void) { - struct packed_git *old_p = pack_data, *new_p; + if (!pack_data) + return; clear_delta_base_cache(); if (object_count) { + struct packed_git *new_p; unsigned char cur_pack_sha1[20]; char *idx_name; int i; @@ -990,10 +992,11 @@ static void end_packfile(void) pack_id++; } else { - close(old_p->pack_fd); - unlink_or_warn(old_p->pack_name); + close(pack_data->pack_fd); + unlink_or_warn(pack_data->pack_name); } - free(old_p); + free(pack_data); + pack_data = NULL; /* We can't carry a delta across packfiles. */ strbuf_release(&last_blob.data); -- cgit v1.2.3 From c252785982c268e5c969900c677322744d09f52e Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 23 Aug 2014 01:32:37 -0400 Subject: fast-import: fix buffer overflow in dump_tags When creating a new annotated tag, we sprintf the refname into a static-sized buffer. If we have an absurdly long tagname, like: git init repo && cd repo && git commit --allow-empty -m foo && git tag -m message mytag && git fast-export mytag | perl -lpe '/^tag/ and s/mytag/"a" x 8192/e' | git fast-import Reviewed-by: Michael Haggerty Reviewed-by: Ronnie Sahlberg Signed-off-by: Junio C Hamano --- fast-import.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/fast-import.c b/fast-import.c index 92b3186326..6475febe99 100644 --- a/fast-import.c +++ b/fast-import.c @@ -1730,14 +1730,16 @@ static void dump_tags(void) static const char *msg = "fast-import"; struct tag *t; struct ref_lock *lock; - char ref_name[PATH_MAX]; + struct strbuf ref_name = STRBUF_INIT; for (t = first_tag; t; t = t->next_tag) { - sprintf(ref_name, "tags/%s", t->name); - lock = lock_ref_sha1(ref_name, NULL); + strbuf_reset(&ref_name); + strbuf_addf(&ref_name, "tags/%s", t->name); + lock = lock_ref_sha1(ref_name.buf, NULL); if (!lock || write_ref_sha1(lock, t->sha1, msg) < 0) - failure |= error("Unable to update %s", ref_name); + failure |= error("Unable to update %s", ref_name.buf); } + strbuf_release(&ref_name); } static void dump_marks_helper(FILE *f, -- cgit v1.2.3