git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 2/2] fsck: avoid looking at NULL blob->object

From
Jeff King <peff@peff.net>
Date
Jun 9, 2018, 09:31 UTC
Message-ID
<20180609093141.GB8942@sigill.intra.peff.net>
In-Reply-To
<20180609093028.GA8864@sigill.intra.peff.net>

Commit 159e7b080b (fsck: detect gitmodules files, 2018-05-02) taught fsck to look at the content of .gitmodules files. If the object turns out not to be a blob at all, we just complain and punt on checking the content. And since this was such an obvious and trivial code path, I didn't even bother to add a test.

Except it _does_ do one non-trivial thing, which is call the report() function, which wants us to pass a pointer to a "struct object". Which we don't have (we have only a "struct object_id"). So we erroneously pass a NULL object to report(), which gets dereferenced and causes a segfault.

It seems like we could refactor report() to just take the object_id itself. But we pass the object pointer along to a callback function, and indeed this ends up in builtin/fsck.c's objreport() which does want to look at other parts of the object (like the type).

So instead, let's just use lookup_unknown_object() to get the real "struct object", and pass that.

Signed-off-by: Jeff King <peff@peff.net>
---
 fsck.c                     |  3 ++-
 t/t7415-submodule-names.sh | 18 ++++++++++++++++++
 2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/fsck.c b/fsck.c
index bcae2c30e6..48e7e36869 100644
--- a/fsck.c
+++ b/fsck.c
@@ -1036,7 +1036,8 @@ int fsck_finish(struct fsck_options *options)
 
 		blob = lookup_blob(oid);
 		if (!blob) {
-			ret |= report(options, &blob->object,
+			struct object *obj = lookup_unknown_object(oid->hash);
+			ret |= report(options, obj,
 				      FSCK_MSG_GITMODULES_BLOB,
 				      "non-blob found at .gitmodules");
 			continue;
diff --git a/t/t7415-submodule-names.sh b/t/t7415-submodule-names.sh
index e2aae587ae..d7868c11fa 100755
--- a/t/t7415-submodule-names.sh
+++ b/t/t7415-submodule-names.sh
@@ -150,4 +150,22 @@ test_expect_success 'fsck detects symlinked .gitmodules file' '
 	)
 '
 
+test_expect_success 'fsck detects non-blob .gitmodules' '
+	git init non-blob &&
+	(
+		cd non-blob &&
+
+		# As above, make the funny tree directly to avoid index
+		# restrictions.
+		mkdir subdir &&
+		cp ../.gitmodules subdir/file &&
+		git add subdir/file &&
+		git commit -m ok &&
+		tree=$(git ls-tree HEAD | sed s/subdir/.gitmodules/ | git mktree) &&
+
+		test_must_fail git fsck 2>output &&
+		grep gitmodulesBlob output
+	)
+'
+
 test_done
-- 
2.18.0.rc1.446.g4486251e51
Previous: Jeff KingNext: SZEDER Gábor
Message 16 of 17 in “fsck: avoid looking at NULL blob->object”
  1. fsck: avoid looking at NULL blob->objectJeff King, Jun 9, 2018
  2. Eric SunshineJun 9, 2018
  3. Jeff KingJun 9, 2018
  4. Duy NguyenJun 9, 2018
  5. Jeff KingJun 9, 2018
  6. Martin ÅgrenJun 9, 2018
  7. Jeff KingJun 9, 2018
  8. Martin ÅgrenJun 9, 2018
  9. Jeff KingJun 12, 2018
  10. 0/2 .gitmodules fsck cleanupsJeff King, Jun 9, 2018
  11. 1/2 t7415: don't bother creating commit for symlink testJeff King, Jun 9, 2018
  12. SZEDER GáborJun 9, 2018
  13. 0/2 .gitmodules fsck cleanupsJeff King, Jun 11, 2018
  14. 1/2 t7415: don't bother creating commit for symlink testJeff King, Jun 11, 2018
  15. 2/2 fsck: avoid looking at NULL blob->objectJeff King, Jun 11, 2018
  16. 2/2 fsck: avoid looking at NULL blob->objectJeff King, Jun 9, 2018
  17. SZEDER GáborJun 9, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.