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

[PATCH 2/3] fsck: actually detect bad file modes in trees

From
Jeff King <peff@peff.net>
Date
Aug 10, 2022, 21:02 UTC
Message-ID
<YvQc9egGzwFomQbw@coredump.intra.peff.net>
In-Reply-To
<YvQcNpizy9uOZiAz@coredump.intra.peff.net>

We use the normal tree_desc code to iterate over trees in fsck, meaning we only see the canonicalized modes it returns. And hence we'd never see anything unexpected, since it will coerce literally any garbage into one of our normal and accepted modes.

We can use the new RAW_MODES flag to see the real modes, and then use the existing code to actually analyze them. The existing code is written as allow-known-good, so there's not much point in testing a variety of breakages. The one tested here should be S_IFREG but with nonsense permissions.

Do note that the error-reporting here isn't great. We don't mention the specific bad mode, but just that the tree has one or more broken modes. But when you go to look at it with "git ls-tree", we'll report the canonicalized mode! This isn't ideal, but given that this should come up rarely, and that any number of other tree corruptions might force you into looking at the binary bytes via "cat-file", it's not the end of the world. And it's something we can improve on top later if we choose.

Reported-by: Xavier Morel <xavier.morel@masklinn.net>
Signed-off-by: Jeff King <peff@peff.net>
---
 fsck.c          |  2 +-
 t/t1450-fsck.sh | 14 ++++++++++++++
 2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/fsck.c b/fsck.c
index 5acc982a7c..b3da1d68c0 100644
--- a/fsck.c
+++ b/fsck.c
@@ -578,7 +578,7 @@ static int fsck_tree(const struct object_id *tree_oid,
 	const char *o_name;
 	struct name_stack df_dup_candidates = { NULL };
 
-	if (init_tree_desc_gently(&desc, buffer, size, 0)) {
+	if (init_tree_desc_gently(&desc, buffer, size, TREE_DESC_RAW_MODES)) {
 		retval += report(options, tree_oid, OBJ_TREE,
 				 FSCK_MSG_BAD_TREE,
 				 "cannot be parsed as a tree");
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index ab7f31f1dc..53c2aa10b7 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -364,6 +364,20 @@ test_expect_success 'tree entry with type mismatch' '
 	test_i18ngrep ! "dangling blob" out
 '
 
+test_expect_success 'tree entry with bogus mode' '
+	test_when_finished "remove_object \$blob" &&
+	test_when_finished "remove_object \$tree" &&
+	blob=$(echo blob | git hash-object -w --stdin) &&
+	blob_oct=$(echo $blob | hex2oct) &&
+	tree=$(printf "100000 foo\0${blob_oct}" |
+	       git hash-object -t tree --stdin -w --literally) &&
+	git fsck 2>err &&
+	cat >expect <<-EOF &&
+	warning in tree $tree: badFilemode: contains bad file modes
+	EOF
+	test_cmp expect err
+'
+
 test_expect_success 'tag pointing to nonexistent' '
 	badoid=$(test_oid deadbeef) &&
 	cat >invalid-tag <<-EOF &&
-- 
2.37.1.917.gf0c714241f
Previous: Jeff KingNext: Junio C Hamano
Message 8 of 17 in “fsck: BAD_FILEMODE diagnostic broken / never triggers”
  1. Xavier MorelAug 6, 2022
  2. Jeff KingAug 9, 2022
  3. Jeff KingAug 9, 2022
  4. Xavier MorelAug 10, 2022
  5. Jeff KingAug 10, 2022
  6. 0/3 actually detect bad file modes in fsckJeff King, Aug 10, 2022
  7. 1/3 tree-walk: add a mechanism for getting non-canonicalized modesJeff King, Aug 10, 2022
  8. 2/3 fsck: actually detect bad file modes in treesJeff King, Aug 10, 2022
  9. Junio C HamanoAug 10, 2022
  10. Jeff KingAug 10, 2022
  11. Junio C HamanoAug 10, 2022
  12. 3/3 fsck: downgrade tree badFilemode to "info"Jeff King, Aug 10, 2022
  13. Junio C HamanoAug 10, 2022
  14. Jeff KingAug 11, 2022
  15. Junio C HamanoAug 11, 2022
  16. Ævar Arnfjörð BjarmasonAug 11, 2022
  17. Jeff KingAug 14, 2022

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.