Re: [PATCH] Avoid infinite loop in malformed packfiles
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 24, 2020, 20:38 UTC
- Message-ID
- <xmqq5z974w50.fsf@gitster.c.googlers.com>
- In-Reply-To
- <20200824201208.GA706849@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I think it may be worth making this a configurable value
> (core.maxDeltaDepth or something). Nobody would generally need to tweak
> it, but it would give an escape hatch for getting people out of a broken
> situation ("git -c core.maxDeltaDepth=50000 repack" or similar).... meaning "the pack I have has overlong delta chains to read, and I am running repack to cut these chains down to more manageable level"? Makes sense.
As it may be a bit tricky to figure out where we should read such a configuration for those who are new to our codebase, here is an illustration to give a starting point. Docs and tests are probably needed, too.
cache.h | 1 + config.c | 5 +++++ environment.c | 1 + packfile.c | 6 ++++++ 4 files changed, 13 insertions(+)
diff --git a/cache.h b/cache.h index 0290849c19..b59d43f0ec 100644 --- a/cache.h +++ b/cache.h @@ -919,6 +919,7 @@ extern int minimum_abbrev, default_abbrev; extern int ignore_case; extern int assume_unchanged; extern int prefer_symlink_refs; +extern int max_allowed_delta_depth; extern int warn_ambiguous_refs; extern int warn_on_object_refname_ambiguity; extern const char *apply_default_whitespace; diff --git a/config.c b/config.c index 2b79fe76ad..5f9114f847 100644 --- a/config.c +++ b/config.c @@ -1197,6 +1197,11 @@ static int git_default_core_config(const char *var, const char *value, void *cb) return 0; } + if (!strcmp(var, "core.maxalloweddeltadepth")) { + max_allowed_delta_depth = git_config_int(var, value); + return 0; + } + if (!strcmp(var, "core.logallrefupdates")) { if (value && !strcasecmp(value, "always")) log_all_ref_updates = LOG_REFS_ALWAYS; diff --git a/environment.c b/environment.c index 52e0c979ba..d3f9a10799 100644 --- a/environment.c +++ b/environment.c @@ -27,6 +27,7 @@ int minimum_abbrev = 4, default_abbrev = -1; int ignore_case; int assume_unchanged; int prefer_symlink_refs; +int max_allowed_delta_depth = 10000; int is_bare_repository_cfg = -1; /* unspecified */ int warn_ambiguous_refs = 1; int warn_on_object_refname_ambiguity = 1; diff --git a/packfile.c b/packfile.c index 6ab5233613..2ea24a19dd 100644 --- a/packfile.c +++ b/packfile.c @@ -1715,6 +1715,12 @@ void *unpack_entry(struct repository *r, struct packed_git *p, off_t obj_offset, break; } + if (max_allowed_delta_depth < delta_stack_nr) { + error("overlong delta chain at offset %"PRIuMAX" from %s", + (uintmax_t)curpos, p->pack_name); + goto out; + } + /* push object, proceed to base */ if (delta_stack_nr >= delta_stack_alloc && delta_stack == small_delta_stack) {