All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Taylor Blau" <me@ttaylorr.com>, "Jeff King" <peff@peff.net>,
	"Derrick Stolee" <stolee@gmail.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH 2/2] pack-objects: fix segfault in --stdin-packs option
Date: Mon, 21 Jun 2021 17:03:38 +0200	[thread overview]
Message-ID: <patch-2.2-a9702132385-20210621T145819Z-avarab@gmail.com> (raw)
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


  parent reply	other threads:[~2021-06-21 15:03 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-21 15:03 [PATCH 0/2] pack-objects: missing tests & --stdin-packs segfault fix Ævar Arnfjörð Bjarmason
2021-06-21 15:03 ` [PATCH 1/2] pack-objects tests: cover blindspots in stdin handling Ævar Arnfjörð Bjarmason
2021-06-21 15:03 ` Ævar Arnfjörð Bjarmason [this message]
2021-06-21 20:33   ` [PATCH 2/2] pack-objects: fix segfault in --stdin-packs option Taylor Blau
2021-06-21 20:34 ` [PATCH 0/2] pack-objects: missing tests & --stdin-packs segfault fix Taylor Blau
2021-07-09 10:13 ` [PATCH v2 " Ævar Arnfjörð Bjarmason
2021-07-09 10:13   ` [PATCH v2 1/2] pack-objects tests: cover blindspots in stdin handling Ævar Arnfjörð Bjarmason
2021-07-09 10:13   ` [PATCH v2 2/2] pack-objects: fix segfault in --stdin-packs option Ævar Arnfjörð Bjarmason
2021-07-19 21:31     ` Taylor Blau
2021-07-20 11:55       ` Ævar Arnfjörð Bjarmason
2021-07-20 16:58         ` Taylor Blau

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=patch-2.2-a9702132385-20210621T145819Z-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=me@ttaylorr.com \
    --cc=peff@peff.net \
    --cc=stolee@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.