[PATCH 2/2] pack-objects: fix segfault in --stdin-packs option
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Jun 21, 2021, 15:03 UTC
- Message-ID
- <patch-2.2-a9702132385-20210621T145819Z-avarab@gmail.com>
- In-Reply-To
- <cover-0.2-00000000000-20210621T145819Z-avarab@gmail.com>
Fix a segfault in the --stdin-packs option added in 339bce27f4f (builtin/pack-objects.c: add '--stdin-packs' option, 2021-02-22). The read_packs_list_from_stdin() function didn't check that the lines it was reading were valid packs, and thus when doing the QSORT() with pack_mtime_cmp() we'd have a NULL "util" field.
The logic error was in assuming that we could iterate all packs and annotate the excluded and included packs we got, as opposed to checking the lines we got on stdin. There was a check for excluded packs, but included packs were simply assumed to be valid.
As noted in the test we'll not report the first bad line, but whatever line sorted first according to the string-list.c API. In this case I think that's fine.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- builtin/pack-objects.c | 10 ++++++++++ t/t5300-pack-object.sh | 18 ++++++++++++++++++ 2 files changed, 28 insertions(+)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index de00adbb9e0..65579e09fe0 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -3310,6 +3310,16 @@ static void read_packs_list_from_stdin(void) item->util = p; } + /* + * Arguments we got on stdin may not even be packs. Check that + * to avoid segfaulting later on in e.g. pack_mtime_cmp(). + */ + for_each_string_list_item(item, &include_packs) { + struct packed_git *p = item->util; + if (!p) + die(_("could not find pack '%s'"), item->string); + } + /* * First handle all of the excluded packs, marking them as kept in-core * so that later calls to add_object_entry() discards any objects that diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh index 65e991e3706..330deec656b 100755 --- a/t/t5300-pack-object.sh +++ b/t/t5300-pack-object.sh @@ -119,6 +119,24 @@ test_expect_success 'pack-object <stdin parsing: [|--revs] with --stdin' ' test_cmp err.expect err.actual ' +test_expect_success 'pack-object <stdin parsing: --stdin-packs handles garbage' ' + cat >in <<-EOF && + $(git -C pack-object-stdin rev-parse one) + $(git -C pack-object-stdin rev-parse two) + EOF + + # We actually just report the first bad line in strcmp() + # order, it just so happens that we get the same result under + # SHA-1 and SHA-256 here. It does not really matter that we + # report the first bad item in this obscure case, so this + # oddity of the test is OK. + cat >err.expect <<-EOF && + fatal: could not find pack '"'"'$(git -C pack-object-stdin rev-parse two)'"'"' + EOF + test_must_fail git -C pack-object-stdin pack-objects stdin-with-stdin-option --stdin-packs <in 2>err.actual && + test_cmp err.expect err.actual +' + # usage: check_deltas <stderr_from_pack_objects> <cmp_op> <nr_deltas> # e.g.: check_deltas stderr -gt 0 check_deltas() {
-- 2.32.0.599.g3967b4fa4ac